diff --git a/runner/filteroperator.go b/runner/filteroperator.go index 69f93cca..8e896249 100644 --- a/runner/filteroperator.go +++ b/runner/filteroperator.go @@ -34,7 +34,14 @@ func (f FilterOperator) Parse(flagValue string) (string, time.Duration, error) { timeVal := strings.Trim(splittedFlagValue[1], " ") value, err = time.ParseDuration(timeVal) if err != nil && strings.Contains(err.Error(), "missing unit") { - value, _ = time.ParseDuration(fmt.Sprintf("%ss", timeVal)) + // A bare number is read as seconds. This retry used to drop its + // error, so a number too large to hold as a duration left value + // at zero and reported nothing, and the filter then matched + // every host instead of rejecting the flag. + value, err = time.ParseDuration(fmt.Sprintf("%ss", timeVal)) + if err != nil { + return operator, value, fmt.Errorf("invalid value provided for %s", f.flag) + } } else if err != nil { return operator, value, fmt.Errorf("invalid value provided for %s", f.flag) } diff --git a/runner/filteroperator_test.go b/runner/filteroperator_test.go new file mode 100644 index 00000000..ced8a4b6 --- /dev/null +++ b/runner/filteroperator_test.go @@ -0,0 +1,62 @@ +package runner + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestFilterOperatorParse(t *testing.T) { + f := FilterOperator{flag: "-mrt, -match-response-time"} + + t.Run("valid", func(t *testing.T) { + for _, tc := range []struct { + in string + operator string + value time.Duration + }{ + {in: "<1s", operator: "<", value: time.Second}, + {in: ">1s", operator: ">", value: time.Second}, + {in: "=1s", operator: "=", value: time.Second}, + {in: "<=1s", operator: "<=", value: time.Second}, + {in: ">=1s", operator: ">=", value: time.Second}, + {in: "!=1s", operator: "!=", value: time.Second}, + {in: "<100ms", operator: "<", value: 100 * time.Millisecond}, + // A bare number is read as seconds. + {in: ">=5", operator: ">=", value: 5 * time.Second}, + // Surrounding spaces are trimmed. + {in: ">= 5s ", operator: ">=", value: 5 * time.Second}, + } { + t.Run(tc.in, func(t *testing.T) { + operator, value, err := f.Parse(tc.in) + require.NoError(t, err) + require.Equal(t, tc.operator, operator) + require.Equal(t, tc.value, value) + }) + } + }) + + t.Run("rejected", func(t *testing.T) { + for _, in := range []string{ + // No operator at all. + "1s", + "", + // An operator with nothing after it. + ">=", + // Not a number and not a duration. + ">=abc", + // A bare number too large to hold as a duration once seconds are + // added. This used to come back as 0 with no error, which turned + // -mrt into a filter that matched every host. + ">=10000000000", + ">=99999999999999999999", + } { + t.Run(in, func(t *testing.T) { + _, value, err := f.Parse(in) + require.Error(t, err, "expected %q to be rejected", in) + require.Zero(t, value) + }) + } + }) +}