From 3e824412d981085e256e97bbd80c2cd801a5cf1e Mon Sep 17 00:00:00 2001 From: Thomas Gardner Date: Fri, 1 Apr 2016 21:42:55 +1000 Subject: [PATCH] commands/cli: fix parsing of incorrect permutations parseOpts now does some preliminary path screening to prevent command sequences like `ipfs cat` from succeeding. The tests affected by this have been slightly altered, but should be restored once parseOpts is decoupled from path analysis. Command suggestion printing has also been factored into a single function. Fixes: #2501 License: MIT Signed-off-by: Thomas Gardner --- commands/cli/cmd_suggestion.go | 14 ++++++++++++++ commands/cli/parse.go | 16 +++++++--------- commands/cli/parse_test.go | 32 ++++++++++++++++++-------------- core/commands/root.go | 5 ----- 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/commands/cli/cmd_suggestion.go b/commands/cli/cmd_suggestion.go index fa03f6586..2cd699b25 100644 --- a/commands/cli/cmd_suggestion.go +++ b/commands/cli/cmd_suggestion.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "sort" "strings" @@ -69,3 +70,16 @@ func suggestUnknownCmd(args []string, root *cmds.Command) []string { } return sFinal } + +func printSuggestions(inputs []string, root *cmds.Command) (err error) { + + suggestions := suggestUnknownCmd(inputs, root) + if len(suggestions) > 1 { + err = fmt.Errorf("Unknown Command \"%s\"\n\nDid you mean any of these?\n\n\t%s", inputs[0], strings.Join(suggestions, "\n\t")) + } else if len(suggestions) > 0 { + err = fmt.Errorf("Unknown Command \"%s\"\n\nDid you mean this?\n\n\t%s", inputs[0], suggestions[0]) + } else { + err = fmt.Errorf("Unknown Command \"%s\"\n", inputs[0]) + } + return +} diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 0e22d8f0f..2ac455f75 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -227,6 +227,11 @@ func parseOpts(args []string, root *cmds.Command) ( } } else { stringVals = append(stringVals, arg) + if len(path) == 0 { + // found a typo or early argument + err = printSuggestions(stringVals, root) + return + } } } } @@ -268,15 +273,8 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi // and the last arg definition is not variadic (or there are no definitions), return an error notVariadic := len(argDefs) == 0 || !argDefs[len(argDefs)-1].Variadic if notVariadic && len(inputs) > len(argDefs) { - suggestions := suggestUnknownCmd(inputs, root) - - if len(suggestions) > 1 { - return nil, nil, fmt.Errorf("Unknown Command \"%s\"\n\nDid you mean any of these?\n\n\t%s", inputs[0], strings.Join(suggestions, "\n\t")) - } else if len(suggestions) > 0 { - return nil, nil, fmt.Errorf("Unknown Command \"%s\"\n\nDid you mean this?\n\n\t%s", inputs[0], suggestions[0]) - } else { - return nil, nil, fmt.Errorf("Unknown Command \"%s\"\n", inputs[0]) - } + err := printSuggestions(inputs, root) + return nil, nil, err } stringArgs := make([]string, 0, numInputs) diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 54f60bf69..d092d2f90 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -78,6 +78,9 @@ func TestOptionParsing(t *testing.T) { } testHelper := func(args string, expectedOpts kvs, expectedWords words, expectErr bool) { + var opts map[string]interface{} + var input []string + _, opts, input, _, err := parseOpts(strings.Split(args, " "), cmd) if expectErr { if err == nil { @@ -99,9 +102,8 @@ func TestOptionParsing(t *testing.T) { testHelper(args, expectedOpts, expectedWords, false) } - test("-", kvs{}, words{"-"}) + test("test -", kvs{}, words{"-"}) testFail("-b -b") - test("beep boop", kvs{}, words{"beep", "boop"}) test("test beep boop", kvs{}, words{"beep", "boop"}) testFail("-s") test("-s foo", kvs{"s": "foo"}, words{}) @@ -110,25 +112,27 @@ func TestOptionParsing(t *testing.T) { test("-b", kvs{"b": true}, words{}) test("-bs foo", kvs{"b": true, "s": "foo"}, words{}) test("-sb", kvs{"s": "b"}, words{}) - test("-b foo", kvs{"b": true}, words{"foo"}) - test("--bool foo", kvs{"bool": true}, words{"foo"}) + test("-b test foo", kvs{"b": true}, words{"foo"}) + test("--bool test foo", kvs{"bool": true}, words{"foo"}) testFail("--bool=foo") testFail("--string") test("--string foo", kvs{"string": "foo"}, words{}) test("--string=foo", kvs{"string": "foo"}, words{}) test("-- -b", kvs{}, words{"-b"}) - test("foo -b", kvs{"b": true}, words{"foo"}) + test("test foo -b", kvs{"b": true}, words{"foo"}) test("-b=false", kvs{"b": false}, words{}) test("-b=true", kvs{"b": true}, words{}) - test("-b=false foo", kvs{"b": false}, words{"foo"}) - test("-b=true foo", kvs{"b": true}, words{"foo"}) - test("--bool=true foo", kvs{"bool": true}, words{"foo"}) - test("--bool=false foo", kvs{"bool": false}, words{"foo"}) - test("-b=FaLsE foo", kvs{"b": false}, words{"foo"}) - test("-b=TrUe foo", kvs{"b": true}, words{"foo"}) - test("-b true", kvs{"b": true}, words{"true"}) - test("-b false", kvs{"b": true}, words{"false"}) - test("-b --string foo bar", kvs{"b": true, "string": "foo"}, words{"bar"}) + test("-b=false test foo", kvs{"b": false}, words{"foo"}) + test("-b=true test foo", kvs{"b": true}, words{"foo"}) + test("--bool=true test foo", kvs{"bool": true}, words{"foo"}) + test("--bool=false test foo", kvs{"bool": false}, words{"foo"}) + test("-b test true", kvs{"b": true}, words{"true"}) + test("-b test false", kvs{"b": true}, words{"false"}) + test("-b=FaLsE test foo", kvs{"b": false}, words{"foo"}) + test("-b=TrUe test foo", kvs{"b": true}, words{"foo"}) + test("-b test true", kvs{"b": true}, words{"true"}) + test("-b test false", kvs{"b": true}, words{"false"}) + test("-b --string foo test bar", kvs{"b": true, "string": "foo"}, words{"bar"}) test("-b=false --string bar", kvs{"b": false, "string": "bar"}, words{}) } diff --git a/core/commands/root.go b/core/commands/root.go index bd3392966..355312df4 100644 --- a/core/commands/root.go +++ b/core/commands/root.go @@ -13,11 +13,6 @@ import ( var log = logging.Logger("core/commands") -type TestOutput struct { - Foo string - Bar int -} - const ( ApiOption = "api" )