From 9fb4a39cb2ff0070e1cf18e6d654f269bbf87d60 Mon Sep 17 00:00:00 2001 From: "Paul S. Schweigert" Date: Wed, 4 Aug 2021 14:42:43 -0400 Subject: [PATCH 1/4] fixing bug when looking up plugins on path with slash in argument --- pkg/kn/plugin/manager.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index f9b9e28446..abf644336a 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -401,6 +401,10 @@ func findMostSpecificPluginInPath(dir string, parts []string, lookupInPath bool) var nameParts []string var commandParts []string for _, p := range parts[0:i] { + // skip arguments with slashes + if strings.Contains(p, "/") || strings.Contains(p, "\\") { + continue + } // Subcommands with "-" are translated to "_" // (e.g. a command "kn log-all" is translated to a plugin "kn-log_all") nameParts = append(nameParts, convertDashToUnderscore(p)) From 350f63929b6bfd05112e2f9f20cf0eef5efac74c Mon Sep 17 00:00:00 2001 From: "Paul S. Schweigert" Date: Wed, 4 Aug 2021 18:12:53 -0400 Subject: [PATCH 2/4] test for dropping slash substrings --- pkg/kn/plugin/manager_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pkg/kn/plugin/manager_test.go b/pkg/kn/plugin/manager_test.go index 5761463131..104c87ab72 100644 --- a/pkg/kn/plugin/manager_test.go +++ b/pkg/kn/plugin/manager_test.go @@ -301,6 +301,33 @@ func TestPluginList(t *testing.T) { } } +func TestNoSlashInPlugin(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + + // Prepare PATH + tmpPathDir, cleanupFunc := preparePathDirectory(t) + defer cleanupFunc() + + fmt.Println(tmpPathDir) + + createTestPluginInDirectory(t, "kn-path-test", tmpPathDir) + pluginCommands := []string{"path", "test", "/withslash"} + + // args with slash are not returned + ctx.pluginManager.lookupInPath = true + plugin, err := ctx.pluginManager.FindPlugin(pluginCommands) + assert.NilError(t, err) + assert.Assert(t, plugin != nil) + desc, err := plugin.Description() + name := plugin.Name() + fmt.Println(plugin.Name()) + assert.NilError(t, err) + assert.Assert(t, desc != "") + assert.Equal(t, plugin.Path(), filepath.Join(tmpPathDir, "kn-path-test")) + assert.Assert(t, !strings.Contains(name, "/")) +} + // ==================================================================== // Private From 46c7bc4fe894d9abf8756cb1c2d3df89aa91a7eb Mon Sep 17 00:00:00 2001 From: "Paul S. Schweigert" Date: Thu, 5 Aug 2021 10:23:40 -0400 Subject: [PATCH 3/4] use path separator --- pkg/kn/plugin/manager.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index abf644336a..c5f6c3d0b1 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -401,9 +401,10 @@ func findMostSpecificPluginInPath(dir string, parts []string, lookupInPath bool) var nameParts []string var commandParts []string for _, p := range parts[0:i] { - // skip arguments with slashes - if strings.Contains(p, "/") || strings.Contains(p, "\\") { - continue + // for arguments that contain the path separator, + // stop the loop once the separator appears + if strings.Contains(p, string(os.PathSeparator)) { + break } // Subcommands with "-" are translated to "_" // (e.g. a command "kn log-all" is translated to a plugin "kn-log_all") From 5d1e08da0663287781abd2fcb11a98b00857fc5d Mon Sep 17 00:00:00 2001 From: "Paul S. Schweigert" Date: Fri, 6 Aug 2021 10:53:21 -0400 Subject: [PATCH 4/4] more tests, using pathseparator in tests --- pkg/kn/plugin/manager_test.go | 51 ++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/pkg/kn/plugin/manager_test.go b/pkg/kn/plugin/manager_test.go index 104c87ab72..55bf1edce8 100644 --- a/pkg/kn/plugin/manager_test.go +++ b/pkg/kn/plugin/manager_test.go @@ -309,23 +309,44 @@ func TestNoSlashInPlugin(t *testing.T) { tmpPathDir, cleanupFunc := preparePathDirectory(t) defer cleanupFunc() - fmt.Println(tmpPathDir) + var middleSlashPlugin string + var middleSlashArg string + ctx.pluginManager.lookupInPath = true - createTestPluginInDirectory(t, "kn-path-test", tmpPathDir) - pluginCommands := []string{"path", "test", "/withslash"} + // Windows does not allow slashes in filenames, so testing for a slash + // in the middle of an argument will have different results depending on OS + if os.PathSeparator == '/' { + middleSlashPlugin = "kn-slash-test-with\\slash" + middleSlashArg = "with\\slash" + } else { + middleSlashPlugin = "kn-slash-test" + middleSlashArg = "with/slash" + } - // args with slash are not returned - ctx.pluginManager.lookupInPath = true - plugin, err := ctx.pluginManager.FindPlugin(pluginCommands) - assert.NilError(t, err) - assert.Assert(t, plugin != nil) - desc, err := plugin.Description() - name := plugin.Name() - fmt.Println(plugin.Name()) - assert.NilError(t, err) - assert.Assert(t, desc != "") - assert.Equal(t, plugin.Path(), filepath.Join(tmpPathDir, "kn-path-test")) - assert.Assert(t, !strings.Contains(name, "/")) + createTestPluginInDirectory(t, "kn-slash-test", tmpPathDir) + createTestPluginInDirectory(t, middleSlashPlugin, tmpPathDir) + + data := []struct { + parts []string + name string + }{ + {[]string{"slash", "test", string(os.PathSeparator) + "withslash"}, "kn-slash-test"}, + {[]string{"slash", "test", string(os.PathSeparator) + "withslash", "extraarg"}, "kn-slash-test"}, + {[]string{"slash", "test", "with" + string(os.PathSeparator) + "slash", "extraarg"}, "kn-slash-test"}, + {[]string{"slash", "test", middleSlashArg}, middleSlashPlugin}, + } + + for _, d := range data { + plugin, err := ctx.pluginManager.FindPlugin(d.parts) + assert.NilError(t, err) + assert.Assert(t, plugin != nil) + desc, err := plugin.Description() + name := plugin.Name() + assert.NilError(t, err) + assert.Assert(t, desc != "") + assert.Equal(t, plugin.Path(), filepath.Join(tmpPathDir, d.name)) + assert.Assert(t, !strings.Contains(name, string(os.PathSeparator))) + } } // ====================================================================