[Console] Escape completion suggestions with special chars #43664
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stof
reviewed
Oct 22, 2021
| $includesUnsafeChars = false; | ||
| $suggestions = array_map(function ($value) use (&$includesUnsafeChars) { | ||
| $newValue = str_replace('\\', '\\\\', $value); | ||
| $newValue = str_replace(' ', '\ ', $value); |
There was a problem hiding this comment.
shouldn't this do the replacement in $newValue ? Otherwise, the previous line is useless code.
There was a problem hiding this comment.
btw, you could do it as a single str_replace using arrays as arguments
GromNaN
reviewed
Oct 22, 2021
| COMPREPLY=($(compgen -W "$sfcomplete" -- "$cur")) | ||
| # $sfcomplete is escaped already, escape the input as well | ||
| local escapedcur=${cur//\\/\\\\} | ||
| COMPREPLY=($(compgen -W "${sfcomplete}" -- "${escapedcur//\'/\\\'}")) |
There was a problem hiding this comment.
I was working on this issue and found that printf "%q" "$cur" escapes for using as shell input.
%q causes printf to output the corresponding argument in a format that can be reused as shell input.
My version: #43665
|
Closing in favor of #43665 |
fabpot
added a commit
that referenced
this pull request
Oct 30, 2021
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Console] Fix backslash escaping in bash completion | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #43664 | License | MIT | Doc PR | - Fully tested code with #43598 - `printf` options `%b` and `%q` are handy to escape string using in shell input. - Backslashes must to be escaped (doubled) in normal input (`bin/console debug:form App\\Form\\CommentType`). Otherwise they are dropped when PHP receive the argument (`App\Form\CommentType` become `AppFormCommentType` in `$SERVER['argv']`). This is not necessary for quoted arguments (`bin/console debug:form "App\Form\CommentType"` is OK). In the context of a command stored in a variable, like in the bash script, escaping is not interpreted: we must replace `\\` by `\` using `printf '%b'`. - Completion does not process escaping. If `App\\Form\\` is typed, the suggestions must contain a `\\` to match. Since these values are provided to shell, double backslash must be escaped: `\` becomes `\\\\` in suggestion output. - I choose to detect when quotes are typed to allow them in completion and add quotes to suggestions. - Suggestion with spaces are still not properly working. https://user-images.githubusercontent.com/400034/138536102-e282ee75-56c8-414b-8b16-7c3c290276d7.mov Commits ------- e9e0c07 [Console] Fix backslash escaping in bash completion
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes shell completion whenever a special char (backslash and whitespace at the moment) are present. In these cases, all suggestions are put in quotes (to avoid double-double escaping) and the special chars are escaped.
This bug was found in #43598