unquote: Fix possible out of bounds access in splitWord() under Windows#1
unquote: Fix possible out of bounds access in splitWord() under Windows#1JoeKar wants to merge 1 commit intozyedidia:masterfrom JoeKar:fix/crash-windows-unquote
splitWord() under Windows#1Conversation
|
The commit with the functional change (https://github.com/JoeKar/go-shellquote/commit/51031e9ea27db3f7abbb545385be313a14939f05) is now cherry-picked into https://github.com/micro-editor/go-shellquote/tree/micro as micro-editor@feb6c39. Now only the dependency must be changed in micro. |
| } | ||
| buf.WriteString(string(escapeChar)) | ||
| input = cur | ||
| goto raw |
There was a problem hiding this comment.
Just for the record: I've been thinking that since essentially what we are doing here is making shellquote.Split() treat \ as a normal, non-special character in all cases (i.e. never escape with \) on Windows, it could be better to do that in a straightforward way, like:
diff --git a/unquote.go b/unquote.go
index c52f0c5..d5644d5 100644
--- a/unquote.go
+++ b/unquote.go
@@ -41,7 +41,7 @@ func Split(input string) (words []string, err error) {
if strings.ContainsRune(splitChars, c) {
input = input[l:]
continue
- } else if c == escapeChar {
+ } else if c == escapeChar && os.PathSeparator != escapeChar {
// Look ahead for escaped newline so we can skip over it
next := input[l:]
if len(next) == 0 {
@@ -82,13 +82,8 @@ raw:
buf.WriteString(input[0 : len(input)-len(cur)-l])
input = cur
goto double
- } else if c == escapeChar {
+ } else if c == escapeChar && os.PathSeparator != escapeChar {
buf.WriteString(input[0 : len(input)-len(cur)-l])
- if os.PathSeparator == escapeChar {
- buf.WriteString(string(escapeChar))
- input = cur
- goto raw
- }
input = cur
goto escape
} else if strings.ContainsRune(splitChars, c) {But that is troublesome, since we also use shellquote.Join() (in RunCmd()) which inserts \ to escape special characters, and then we call shellquote.Split() again for this escaped string. So with my above code, run "'" or run '"' won't work anymore on Windows.
So this PR's trick seems to be the best we can do for now.
| if os.PathSeparator == escapeChar { | ||
| next := rune(cur[0]) | ||
| switch next { | ||
| case singleChar, doubleChar, escapeChar, 'n': |
There was a problem hiding this comment.
Just for the record: so we sacrifice the possibility to use \ to escape single or double quotes on Windows. For example: open a new unnamed file, press Ctrl-S to save it, and in the Filename: prompt, type \". Result: without this PR the file is successfully saved as ", while with this PR we get Error parsing arguments: Unterminated double-quoted string.
I think it's ok, and it's actually better if \ behaves consistently, i.e. never escapes on Windows. And the user can still escape quotes in other ways: "'" and '"'.
Fixes micro-editor/micro#2666