rogpeppe / go-internal

Selected Go-internal packages factored out from the standard library
BSD 3-Clause "New" or "Revised" License
823 stars 67 forks source link

testscript: invalid escape sequence when parsing argument to stdout #255

Open myitcv opened 3 weeks ago

myitcv commented 3 weeks ago

As of c8567cf , the following (which can be used as a testscript test for testscript itself) fails when it should succeed:

env BLAH='\'
exec echo ..${BLAH}LICENSE
stdout ..${BLAH}LICENSE

The output is:

--- FAIL: TestScripts (0.00s)
    --- FAIL: TestScripts/stdout (0.00s)
        testscript.go:558: > env BLAH='\'
            > exec echo ..${BLAH}LICENSE
            [stdout]
            ..\LICENSE
            > stdout ..${BLAH}LICENSE
            FAIL: testdata/stdout.txt:3: error parsing regexp: invalid escape sequence: `\L`

FAIL
FAIL    github.com/rogpeppe/go-internal/testscript      1.857s

Upstream Go succeeds with this test (as of https://github.com/golang/go/commit/9b43bfbc51c469ec13fca24960834a75b2bf66eb).

rogpeppe commented 2 weeks ago

For the record, the way that the Go implementation does this is by associating each testscript command with some metadata. This metadata includes information about which arguments are regular expressions.

Here is where that information is provided for the stdout command, for example: https://github.com/golang/go/blob/c83b1a7013784098c2061ae7be832b2ab7241424/src/cmd/go/internal/script/cmds.go#L982

This information is needed because the commands themselves are not responsible for expanding variable references, so cannot know which parts of an argument have come from an environment variable (and hence need to be quoted) and which have not.

In order to fix this, ISTM we would need a differently typed map for commands. Currently the map is of type map[string]func(ts *TestScript, neg bool, args []string) but that does not provide any space for added metadata.

Note that also, this would technically be a breaking change. Currently the following test passes, but would fail if variable references were to be automatically escaped:

env BLAH='(hello|goodbye)'
exec echo hello
stdout $BLAH

For now, there is a workaround: testscript provides the ability to escape regexp characters by using the @R modifier. This variant of the original example test passes, for instance:

env BLAH='\'
exec echo ..${BLAH}LICENSE
stdout ..${BLAH@R}LICENSE