shell completion: fix problems with container path completion

When you try to complete a path which exists and it is a file the
completion logic did not check the parent dir for other matching file
names. To fix that we have to check if the current completion is not a
dir and use the parent dir in this case.

See the updated test for an example why this is required.

Also make sure directories are correctly completed, the shell always
adds the "/" as suffix to signal the user that this path is a directory.
In this case we do not want to automatically add a space. When the path
is a regular file we want the space after the suggestion since there is
nothing more to complete.

This better matches the normal default shell completion.

The test were changed to not assume any particular ordering since this
is irrelevant for the shell completion script and there is no guarantee
about the ordering.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

2 files changed, 60 insertions, 20 deletions

diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go
index 5eef5f982..07dcc4e6a 100644
--- a/cmd/podman/common/completion.go
+++ b/cmd/podman/common/completion.go
@@ -284,7 +284,16 @@ func getNetworks(cmd *cobra.Command, toComplete string, cType completeType) ([]s
 	return suggestions, cobra.ShellCompDirectiveNoFileComp
 }
 
-func getPathCompletion(root string, toComplete string) []string {
+func fdIsNotDir(f *os.File) bool {
+	stat, err := f.Stat()
+	if err != nil {
+		cobra.CompErrorln(err.Error())
+		return true
+	}
+	return !stat.IsDir()
+}
+
+func getPathCompletion(root string, toComplete string) ([]string, cobra.ShellCompDirective) {
 	if toComplete == "" {
 		toComplete = "/"
 	}
@@ -292,41 +301,61 @@ func getPathCompletion(root string, toComplete string) []string {
 	userpath, err := securejoin.SecureJoin(root, toComplete)
 	if err != nil {
 		cobra.CompErrorln(err.Error())
-		return nil
+		return nil, cobra.ShellCompDirectiveDefault
 	}
 	var base string
 	f, err := os.Open(userpath)
-	if err != nil {
+	// when error or file is not dir get the parent path to stat
+	if err != nil || fdIsNotDir(f) {
 		// Do not use path.Dir() since this cleans the paths which
 		// then no longer matches the user input.
 		userpath, base = path.Split(userpath)
 		toComplete, _ = path.Split(toComplete)
 		f, err = os.Open(userpath)
 		if err != nil {
-			return nil
+			return nil, cobra.ShellCompDirectiveDefault
 		}
 	}
-	stat, err := f.Stat()
-	if err != nil {
-		cobra.CompErrorln(err.Error())
-		return nil
-	}
-	if !stat.IsDir() {
+
+	if fdIsNotDir(f) {
 		// nothing to complete since it is no dir
-		return nil
+		return nil, cobra.ShellCompDirectiveDefault
 	}
+
 	entries, err := f.ReadDir(-1)
 	if err != nil {
 		cobra.CompErrorln(err.Error())
-		return nil
+		return nil, cobra.ShellCompDirectiveDefault
+	}
+	if len(entries) == 0 {
+		// path is empty dir, just add the trailing slash and no space
+		if !strings.HasSuffix(toComplete, "/") {
+			toComplete += "/"
+		}
+		return []string{toComplete}, cobra.ShellCompDirectiveDefault | cobra.ShellCompDirectiveNoSpace
 	}
 	completions := make([]string, 0, len(entries))
+	count := 0
 	for _, e := range entries {
 		if strings.HasPrefix(e.Name(), base) {
-			completions = append(completions, simplePathJoinUnix(toComplete, e.Name()))
+			suf := ""
+			// When the entry is an directory we add the "/" as suffix and do not want to add space
+			// to match normal shell completion behavior.
+			// Just inc counter again to fake more than one entry in this case and thus get no space.
+			if e.IsDir() {
+				suf = "/"
+				count++
+			}
+			completions = append(completions, simplePathJoinUnix(toComplete, e.Name()+suf))
+			count++
 		}
 	}
-	return completions
+	directive := cobra.ShellCompDirectiveDefault
+	if count > 1 {
+		// when we have more than one match we do not want to add a space after the completion
+		directive |= cobra.ShellCompDirectiveNoSpace
+	}
+	return completions, directive
 }
 
 // simplePathJoinUnix joins to path components by adding a slash only if p1 doesn't end with one.
@@ -605,7 +634,7 @@ func AutocompleteCreateRun(cmd *cobra.Command, args []string, toComplete string)
 	// So this uses ShellCompDirectiveDefault to also still provide normal shell
 	// completion in case no path matches. This is useful if someone tries to get
 	// completion for paths that are not available in the image, e.g. /proc/...
-	return getPathCompletion(resp[0].Path, toComplete), cobra.ShellCompDirectiveDefault | cobra.ShellCompDirectiveNoSpace
+	return getPathCompletion(resp[0].Path, toComplete)
 }
 
 // AutocompleteRegistries - Autocomplete registries.
@@ -676,7 +705,8 @@ func AutocompleteCpCommand(cmd *cobra.Command, args []string, toComplete string)
 			if len(resp) != 1 {
 				return nil, cobra.ShellCompDirectiveDefault
 			}
-			return prefixSlice(toComplete[:i+1], getPathCompletion(resp[0].Path, toComplete[i+1:])), cobra.ShellCompDirectiveDefault | cobra.ShellCompDirectiveNoSpace
+			comps, directive := getPathCompletion(resp[0].Path, toComplete[i+1:])
+			return prefixSlice(toComplete[:i+1], comps), directive
 		}
 		// Suggest containers when they match the input otherwise normal shell completion is used
 		containers, _ := getContainers(cmd, toComplete, completeDefault)
diff --git a/test/system/600-completion.bats b/test/system/600-completion.bats
index 2de9b1ae1..cb4a2c5f8 100644
--- a/test/system/600-completion.bats
+++ b/test/system/600-completion.bats
@@ -341,7 +341,9 @@ function _check_no_suggestions() {
     skip_if_remote "mounting via remote does not work"
     for cmd in create run; do
         run_completion $cmd $IMAGE ""
-        assert "$output" =~ ".*^/etc\$.*^/home\$.*^/root\$.*" "root directories suggested (cmd: podman $cmd)"
+        assert "$output" =~ ".*^/etc/\$.*" "etc directory suggested (cmd: podman $cmd)"
+        assert "$output" =~ ".*^/home/\$.*" "home directory suggested (cmd: podman $cmd)"
+        assert "$output" =~ ".*^/root/\$.*" "root directory suggested (cmd: podman $cmd)"
 
         # check completion for subdirectory
         run_completion $cmd $IMAGE "/etc"
@@ -354,23 +356,31 @@ function _check_no_suggestions() {
         # check completion with relative path components
         # It is important the we will still use the image root and not escape to the host
         run_completion $cmd $IMAGE "../../"
-        assert "$output" =~ ".*^../../etc\$.*^../../home\$.*" "relative root directories suggested (cmd: podman $cmd ../../)"
+        assert "$output" =~ ".*^../../etc/\$.*" "relative etc directory suggested (cmd: podman $cmd ../../)"
+        assert "$output" =~ ".*^../../home/\$.*" "relative home directory suggested (cmd: podman $cmd ../../)"
     done
 
     random_name=$(random_string 30)
     random_file=$(random_string 30)
-    run_podman run --name $random_name $IMAGE touch /tmp/$random_file
+    run_podman run --name $random_name $IMAGE sh -c "touch /tmp/$random_file && touch /tmp/${random_file}2 && mkdir /emptydir"
 
     # check completion for podman cp
     run_completion cp ""
     assert "$output" =~ ".*^$random_name\:\$.*" "podman cp suggest container names"
 
     run_completion cp "$random_name:"
-    assert "$output" =~ ".*^$random_name\:/etc\$.*" "podman cp suggest paths in container"
+    assert "$output" =~ ".*^$random_name\:/etc/\$.*" "podman cp suggest paths in container"
 
     run_completion cp "$random_name:/tmp"
     assert "$output" =~ ".*^$random_name\:/tmp/$random_file\$.*" "podman cp suggest custom file in container"
 
+    run_completion cp "$random_name:/tmp/$random_file"
+    assert "$output" =~ ".*^$random_name\:/tmp/$random_file\$.*" "podman cp suggest /tmp/$random_file file in container"
+    assert "$output" =~ ".*^$random_name\:/tmp/${random_file}2\$.*" "podman cp suggest /tmp/${random_file}2 file in container"
+
+    run_completion cp "$random_name:/emptydir"
+    assert "$output" =~ ".*^$random_name\:/emptydir/\$.*ShellCompDirectiveNoSpace" "podman cp suggest empty dir with no space directive (:2)"
+
     # cleanup container
     run_podman rm $random_name
 }