mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
7.1k stars 336 forks source link

expand: parameter expansions with spaces in the middle of words are mishandled #886

Closed dkegel-fastly closed 2 years ago

dkegel-fastly commented 2 years ago

It feels like gosh trims leading and trailing spaces from variables:

$ cat x.sh
#!/bin/sh
FOO=" Q "
echo a${FOO}b
export FOO
env | grep FOO | sed 's/$/$/'
$ sh x.sh
a Q b
FOO= Q $
$ go run main.go x.sh
aQb
FOO= Q $

I would have expected gosh x.sh and sh x.sh to produce identical output, but gosh outputs "aQb" while sh outputs "a Q b".

Looks like the variable has the spaces, but they are trimmed during variable expansion?

dkegel-fastly commented 2 years ago

The problem can be reproduced with this unit test case in interp/interp_test.go: [incorrect proposed test case deleted]

dkegel-fastly commented 2 years ago

Not all expansions have the problem -- only unquoted ones that aren't on the right hand side of an assignment?

$ go run cmd/gosh/main.go -c 'FOO=" Q "; echo a${FOO}b'
aQb
$ go run cmd/gosh/main.go -c 'FOO=" Q "; echo a"${FOO}"b'
a Q b
$ go run cmd/gosh/main.go -c 'FOO=" Q "; BAR=a${FOO}b; echo $BAR'
a Q b
dkegel-fastly commented 2 years ago

Shortest repro, perhaps:

$ export FOO=" Q "
$ go run cmd/gosh/main.go -c 'echo x${FOO}y'
xQy
dkegel-fastly commented 2 years ago

So this rescues that one test case:

--- a/expand/expand.go
+++ b/expand/expand.go
@@ -617,7 +618,8 @@ func (cfg *Config) wordFields(wps []syntax.WordPart) ([][]fieldPart, error) {
                        if err != nil {
                                return nil, err
                        }
-                       splitAdd(val)
+                       curField = append(curField, fieldPart{val: val})
+                       //splitAdd(val)
                case *syntax.CmdSubst:
                        val, err := cfg.cmdSubst(x)
                        if err != nil {

It also breaks lots of other stuff.

mvdan commented 2 years ago

Your original bug report is valid - you correctly show that our interpreter prints something different than Bash. That does sound like a bug.

However, note that your second reproducer is invalid, as currently our interpreter outputs the same as Bash - note the lack of leading or trailing whitespace:

$ echo $0
/bin/bash
$ a=" foo_interp_missing "; echo $a
foo_interp_missing

I believe the bug exists when an unquoted parameter expansion exists in the middle of a word, like foo${bar}baz, and its value contains whitespace. Your attempted fix is in the right part of the codebase, though the correct fix might be more subtle than that :)

dkegel-fastly commented 2 years ago

I wouldn't call it an attempted fix, more like a depth charge :-)