Merge pull request #911 from rliebz/fix/infinite-parse

Fix infinite loop when parsing invalid flags for apps with short option handling

command_test.go

@@ -70,8 +70,13 @@ func TestParseAndRunShortOpts(t *testing.T) {
 		{[]string{"foo", "test", "-af"}, nil, []string{}},
 		{[]string{"foo", "test", "-cf"}, nil, []string{}},
 		{[]string{"foo", "test", "-acf"}, nil, []string{}},
+		{[]string{"foo", "test", "--acf"}, errors.New("flag provided but not defined: -acf"), nil},
 		{[]string{"foo", "test", "-invalid"}, errors.New("flag provided but not defined: -invalid"), nil},
+		{[]string{"foo", "test", "-acf", "-invalid"}, errors.New("flag provided but not defined: -invalid"), nil},
+		{[]string{"foo", "test", "--invalid"}, errors.New("flag provided but not defined: -invalid"), nil},
+		{[]string{"foo", "test", "-acf", "--invalid"}, errors.New("flag provided but not defined: -invalid"), nil},
 		{[]string{"foo", "test", "-acf", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}},
+		{[]string{"foo", "test", "-acf", "arg1", "--invalid"}, nil, []string{"arg1", "--invalid"}},
 		{[]string{"foo", "test", "-acfi", "not-arg", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}},
 		{[]string{"foo", "test", "-i", "ivalue"}, nil, []string{}},
 		{[]string{"foo", "test", "-i", "ivalue", "arg1"}, nil, []string{"arg1"}},

parse.go

@@ -22,28 +22,35 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error {
 		}
 
 		errStr := err.Error()
-		trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: ")
+		trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: -")
 		if errStr == trimmed {
 			return err
 		}
 
 		// regenerate the initial args with the split short opts
-		newArgs := []string{}
+		argsWereSplit := false
 		for i, arg := range args {
-			if arg != trimmed {
-				newArgs = append(newArgs, arg)
+			// skip args that are not part of the error message
+			if name := strings.TrimLeft(arg, "-"); name != trimmed {
 				continue
 			}
 
-			shortOpts := splitShortOptions(set, trimmed)
+			// if we can't split, the error was accurate
+			shortOpts := splitShortOptions(set, arg)
 			if len(shortOpts) == 1 {
 				return err
 			}
 
-			// add each short option and all remaining arguments
-			newArgs = append(newArgs, shortOpts...)
-			newArgs = append(newArgs, args[i+1:]...)
-			args = newArgs
+			// swap current argument with the split version
+			args = append(args[:i], append(shortOpts, args[i+1:]...)...)
+			argsWereSplit = true
+			break
+		}
+
+		// This should be an impossible to reach code path, but in case the arg
+		// splitting failed to happen, this will prevent infinite loops
+		if !argsWereSplit {
+			return err
 		}
 
 		// Since custom parsing failed, replace the flag set before retrying