Repository: rsteube/carapace
Pull request #871 (2023-07-31 22:30:03)

bash: use double quotes
TODO: breaks on `example multiparts --colon first:"third with space:"<TAB>`

needs to calculate wordbreakprefix in carapace-shlex

TODO: check `$(malicious command)` and such are correctly escaped


Patch:
diff --git a/example/cmd/multiparts.go b/example/cmd/multiparts.go
index cf7fc89..aa13e23 100644
--- a/example/cmd/multiparts.go
+++ b/example/cmd/multiparts.go
@@ -27,6 +27,7 @@ func init() {
 	multipartsCmd.Flags().String("none-two", "", "multiparts without divider limited to 2")
 	multipartsCmd.Flags().String("none-three", "", "multiparts without divider limited to 3")
 	multipartsCmd.Flags().String("slash", "", "multiparts with / as divider")
+	multipartsCmd.Flags().String("space", "", "multiparts with space as divider")
 
 	rootCmd.AddCommand(multipartsCmd)
 
@@ -84,6 +85,12 @@ func init() {
 			}
 		}),
 		"slash": actionMultipartsTest("/"),
+		"space": carapace.ActionValues(
+			"one",
+			"two",
+			"three",
+			"four",
+		).UniqueList(" "),
 	})
 
 	carapace.Gen(multipartsCmd).PositionalCompletion(
diff --git a/example/cmd/special.go b/example/cmd/special.go
index 8b5610b..779988a 100644
--- a/example/cmd/special.go
+++ b/example/cmd/special.go
@@ -30,8 +30,7 @@ func init() {
 	})
 
 	carapace.Gen(specialCmd).PositionalCompletion(
-		carapace.ActionValues("positional1", "p1", "positional1 with space"),
-		carapace.ActionValues("positional2", "p2", "positional2 with space"),
+		carapace.ActionValues(`p1 & < > ' " { } $ # | ? ( ) ;  [ ] * \ `+"`", "positional1"),
 	)
 
 }
diff --git a/internal/shell/bash/action.go b/internal/shell/bash/action.go
index 9607a60..90c96a0 100644
--- a/internal/shell/bash/action.go
+++ b/internal/shell/bash/action.go
@@ -14,28 +14,15 @@ var sanitizer = strings.NewReplacer(
 	"\t", ``,
 )
 
-var quoter = strings.NewReplacer(
-	// seems readline provides quotation only for the filename completion (which would add suffixes) so do that here
-	`&`, `\&`,
-	`<`, `\<`,
-	`>`, `\>`,
-	"`", "\\`",
-	`'`, `\'`,
+var valueReplacer = strings.NewReplacer(
+	`\`, `\\`,
 	`"`, `\"`,
-	`{`, `\{`,
-	`}`, `\}`,
 	`$`, `\$`,
-	`#`, `\#`,
-	`|`, `\|`,
-	`?`, `\?`,
-	`(`, `\(`,
-	`)`, `\)`,
-	`;`, `\;`,
-	` `, `\ `,
-	`[`, `\[`,
-	`]`, `\]`,
-	`*`, `\*`,
-	`\`, `\\`,
+	"`", "\\`",
+)
+
+var displayReplacer = strings.NewReplacer(
+	`${`, `\\\${`,
 )
 
 func commonPrefix(a, b string) string {
@@ -72,16 +59,17 @@ func commonValuePrefix(values ...common.RawValue) (prefix string) {
 func ActionRawValues(currentWord string, meta common.Meta, values common.RawValues) string {
 	lastSegment := currentWord // last segment of currentWord split by COMP_WORDBREAKS
 
-	for valueIndex, value := range values {
-		// TODO optimize
-		if wordbreaks, ok := os.LookupEnv("COMP_WORDBREAKS"); ok {
-			wordbreaks = strings.Replace(wordbreaks, " ", "", -1)
-			if index := strings.LastIndexAny(currentWord, wordbreaks); index != -1 {
-				values[valueIndex].Value = strings.TrimPrefix(value.Value, currentWord[:index+1])
-				lastSegment = currentWord[index+1:]
-			}
-		}
-	}
+	// TODO handle wordbreaks correctly with carapace-shlex
+	// for valueIndex, value := range values {
+	// TODO optimize
+	// if wordbreaks, ok := os.LookupEnv("COMP_WORDBREAKS"); ok {
+	// 	wordbreaks = strings.Replace(wordbreaks, " ", "", -1)
+	// 	if index := strings.LastIndexAny(currentWord, wordbreaks); index != -1 {
+	// 		values[valueIndex].Value = strings.TrimPrefix(value.Value, currentWord[:index+1])
+	// 		lastSegment = currentWord[index+1:]
+	// 	}
+	// }
+	// }
 
 	if len(values) > 1 && commonDisplayPrefix(values...) != "" {
 		// When all display values have the same prefix bash will insert is as partial completion (which skips prefixes/formatting).
@@ -95,15 +83,27 @@ func ActionRawValues(currentWord string, meta common.Meta, values common.RawValu
 		meta.Nospace.Add('*')
 	}
 
+	nospace := true
 	vals := make([]string, len(values))
 	for index, val := range values {
 		if len(values) == 1 {
-			vals[index] = quoter.Replace(sanitizer.Replace(val.Value))
 			if !meta.Nospace.Matches(val.Value) {
-				vals[index] = val.Value + " "
+				nospace = false
 			}
 
+			vals[index] = sanitizer.Replace(val.Value)
+			if requiresQuoting(vals[index]) {
+				vals[index] = valueReplacer.Replace(vals[index])
+				switch {
+				case strings.HasPrefix(vals[index], "~"): // assume homedir expansion
+					vals[index] = fmt.Sprintf(`~"%v"`, strings.TrimPrefix(vals[index], "~"))
+				default:
+					vals[index] = fmt.Sprintf(`"%v"`, vals[index])
+				}
+			}
 		} else {
+			val.Display = displayReplacer.Replace(val.Display)
+			val.Description = displayReplacer.Replace(val.Description)
 			if val.Description != "" {
 				vals[index] = fmt.Sprintf("%v (%v)", val.Display, sanitizer.Replace(val.TrimmedDescription()))
 			} else {
@@ -111,5 +111,13 @@ func ActionRawValues(currentWord string, meta common.Meta, values common.RawValu
 			}
 		}
 	}
-	return strings.Join(vals, "\n")
+	return fmt.Sprintf("%v\001%v", nospace, strings.Join(vals, "\n"))
+}
+
+func requiresQuoting(s string) bool {
+	chars := " \t\r\n`" + `[]{}()<>;|$&:*#`
+	chars += os.Getenv("COMP_WORDBREAKS")
+	chars += `\`
+	return strings.ContainsAny(s, chars)
+
 }
diff --git a/internal/shell/bash/snippet.go b/internal/shell/bash/snippet.go
index 8b1021f..806ec2f 100644
--- a/internal/shell/bash/snippet.go
+++ b/internal/shell/bash/snippet.go
@@ -15,16 +15,27 @@ _%v_completion() {
   export COMP_WORDBREAKS
   export COMP_LINE
 
-  local compline="${COMP_LINE:0:${COMP_POINT}}"
+  local nospace data compline="${COMP_LINE:0:${COMP_POINT}}"
+
+  if echo ${compline}"''" | xargs echo 2>/dev/null > /dev/null; then
+  	data=$(echo ${compline}"''" | xargs %v _carapace bash)
+  elif echo ${compline} | sed "s/\$/'/" | xargs echo 2>/dev/null > /dev/null; then
+  	data=$(echo ${compline} | sed "s/\$/'/" | xargs %v _carapace bash)
+  else
+  	data=$(echo ${compline} | sed 's/$/"/' | xargs %v _carapace bash)
+  fi
+
+  IFS=$'\001' read -r -d '' nospace data <<<"${data}"
+  mapfile -t COMPREPLY < <(echo "${data}")
+  unset COMPREPLY[-1]
+
+  [ "${nospace}" = true ] && compopt -o nospace
   local IFS=$'\n'
-  mapfile -t COMPREPLY < <(echo "$compline" | sed -e "s/ \$/ ''/" -e 's/"/\"/g' | xargs %v _carapace bash)
   [[ "${COMPREPLY[*]}" == "" ]] && COMPREPLY=() # fix for mapfile creating a non-empty array from empty command output
-
-  compopt -o nospace
 }
 
-complete -F _%v_completion %v
-`, cmd.Name(), uid.Executable(), cmd.Name(), cmd.Name())
+complete -o noquote -F _%v_completion %v
+`, cmd.Name(), uid.Executable(), uid.Executable(), uid.Executable(), cmd.Name(), cmd.Name())
 
 	return result
 }


Test patch:
diff --git a/example/cmd/_test/bash-ble.sh b/example/cmd/_test/bash-ble.sh
index a41976c..ba7c57b 100644
--- a/example/cmd/_test/bash-ble.sh
+++ b/example/cmd/_test/bash-ble.sh
@@ -3,15 +3,26 @@ _example_completion() {
   export COMP_WORDBREAKS
   export COMP_LINE
 
-  local compline="${COMP_LINE:0:${COMP_POINT}}"
+  local nospace data compline="${COMP_LINE:0:${COMP_POINT}}"
+
+  if echo ${compline}"''" | xargs echo 2>/dev/null > /dev/null; then
+  	data=$(echo ${compline}"''" | xargs example _carapace bash)
+  elif echo ${compline} | sed "s/\$/'/" | xargs echo 2>/dev/null > /dev/null; then
+  	data=$(echo ${compline} | sed "s/\$/'/" | xargs example _carapace bash)
+  else
+  	data=$(echo ${compline} | sed 's/$/"/' | xargs example _carapace bash)
+  fi
+
+  IFS=$'\001' read -r -d '' nospace data <<<"${data}"
+  mapfile -t COMPREPLY < <(echo "${data}")
+  unset COMPREPLY[-1]
+
+  [ "${nospace}" = true ] && compopt -o nospace
   local IFS=$'\n'
-  mapfile -t COMPREPLY < <(echo "$compline" | sed -e "s/ \$/ ''/" -e 's/"/\"/g' | xargs example _carapace bash)
   [[ "${COMPREPLY[*]}" == "" ]] && COMPREPLY=() # fix for mapfile creating a non-empty array from empty command output
-
-  compopt -o nospace
 }
 
-
+complete -o noquote -F _example_completion example
 
 _example_completion_ble() {
   if [[ ${BLE_ATTACHED-} ]]; then
diff --git a/example/cmd/_test/bash.sh b/example/cmd/_test/bash.sh
index 4c9f338..9591d72 100644
--- a/example/cmd/_test/bash.sh
+++ b/example/cmd/_test/bash.sh
@@ -3,13 +3,24 @@ _example_completion() {
   export COMP_WORDBREAKS
   export COMP_LINE
 
-  local compline="${COMP_LINE:0:${COMP_POINT}}"
+  local nospace data compline="${COMP_LINE:0:${COMP_POINT}}"
+
+  if echo ${compline}"''" | xargs echo 2>/dev/null > /dev/null; then
+  	data=$(echo ${compline}"''" | xargs example _carapace bash)
+  elif echo ${compline} | sed "s/\$/'/" | xargs echo 2>/dev/null > /dev/null; then
+  	data=$(echo ${compline} | sed "s/\$/'/" | xargs example _carapace bash)
+  else
+  	data=$(echo ${compline} | sed 's/$/"/' | xargs example _carapace bash)
+  fi
+
+  IFS=$'\001' read -r -d '' nospace data <<<"${data}"
+  mapfile -t COMPREPLY < <(echo "${data}")
+  unset COMPREPLY[-1]
+
+  [ "${nospace}" = true ] && compopt -o nospace
   local IFS=$'\n'
-  mapfile -t COMPREPLY < <(echo "$compline" | sed -e "s/ \$/ ''/" -e 's/"/\"/g' | xargs example _carapace bash)
   [[ "${COMPREPLY[*]}" == "" ]] && COMPREPLY=() # fix for mapfile creating a non-empty array from empty command output
-
-  compopt -o nospace
 }
 
-complete -F _example_completion example
+complete -o noquote -F _example_completion example
 
diff --git a/example/cmd/_test_files/files_linux.go b/example/cmd/_test_files/files_linux.go
index e051b3f..f5e5640 100644
--- a/example/cmd/_test_files/files_linux.go
+++ b/example/cmd/_test_files/files_linux.go
@@ -2,27 +2,37 @@ package testfiles
 
 //go:generate touch -- -_minus_prefix.txt
 //go:generate touch -- ampersand_&.txt
-//go:generate touch -- angle-brackets_<.txt
-//go:generate touch -- angle-brackets_<>.txt
-//go:generate touch -- backslash2_\n.txt
-//go:generate touch -- backslash3_\t.txt
-//go:generate touch -- backslash_\.txt
-//go:generate touch -- backtick_`.txt
-//go:generate touch -- both_quotes_'".txt
+//go:generate touch -- angle-bracket_left_<.txt
+//go:generate touch -- angle-bracket_right_>.txt
+//go:generate touch -- angle-bracket_both_<>.txt
+//go:generate touch -- backslash_linebreak_\n.txt
+//go:generate touch -- backslash_tab_\t.txt
+//go:generate touch -- backslash_single_\.txt
+//go:generate touch -- backslash_double_\\.txt
+//go:generate touch -- backtick_single_`.txt
+//go:generate touch -- backtick_double_``.txt
 //go:generate touch -- colon_:.txt
-//go:generate touch -- curly-bracket_{.txt
-//go:generate touch -- curly-brackets_{}.txt
-//go:generate touch -- dollar_$.txt
-//go:generate touch -- double-quote_".txt
+//go:generate touch -- curly-bracket_left_{.txt
+//go:generate touch -- curly-bracket_right_}.txt
+//go:generate touch -- curly-bracket_both_{}.txt
+//go:generate touch -- dollar_single_$.txt
+//go:generate touch -- dollar_round_$().txt
+//go:generate touch -- "dollar_curly_${}.txt"
+//go:generate touch -- quote_single_'.txt
+//go:generate touch -- quote_double_".txt
+//go:generate touch -- quote_both_'".txt
 //go:generate touch -- hash_#.txt
 //go:generate touch -- pipe_|.txt
 //go:generate touch -- question_?.txt
-//go:generate touch -- round-bracket_(.txt
-//go:generate touch -- round-brackets_().txt
+//go:generate touch -- round-bracket_left_(.txt
+//go:generate touch -- round-bracket_right_).txt
+//go:generate touch -- round-bracket_both_().txt
 //go:generate touch -- semicolon_;.txt
 //go:generate touch -- single-quote_'.txt
 //go:generate touch -- "space_ .txt"
-//go:generate touch -- square-bracket_[.txt
-//go:generate touch -- square-brackets_[].txt
+//go:generate touch -- square-bracket_left_[.txt
+//go:generate touch -- square-bracket_right_].txt
+//go:generate touch -- square-bracket_both_[].txt
 //go:generate touch -- "star_*.txt"
+//go:generate touch -- "star_*_match.txt"
 //go:generate touch -- ~_tilde_prefix.txt