cmd/go: build panics when passed `="` as build path

noerw commented 2 years ago

What version of Go are you using (`go version`)?

$ go version
1.17.1 linux/amd64

Does this issue reproduce with the latest release?

yes (1.17.2)

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/asdf/.cache/go-build"
GOENV="/home/asdf/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/norwin/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/norwin/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/asdf/asdf/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3283573966=/tmp/go-build -gno-record-gcc-switches"

What did you do?

run either of these commands in any go project dir

go build '' -ldflags='-X "main.Tags="'
go install '' -ldflags='-X "main.Tags="'

What did you expect to see?

The compilation should succeed, as it does when removing the ''.

-go build '' -ldflags='-X "main.Tags="'
+go build -ldflags='-X "main.Tags="'

What did you see instead?

panic: path "-ldflags=-X \"main.Tags=\"" not in error "invalid import path \"-ldflags=-X \\\"main.Tags=\\\"\"" [recovered]
    panic: path "-ldflags=-X \"main.Tags=\"" not in error "invalid import path \"-ldflags=-X \\\"main.Tags=\\\"\""

goroutine 1 [running]:
cmd/go/internal/load.(*preload).flush(0xc0001f62c0)
    /usr/lib/go/src/cmd/go/internal/load/pkg.go:1041 +0x78
panic({0x8f9d60, 0xc000684a60})
    /usr/lib/go/src/runtime/panic.go:1038 +0x215
cmd/go/internal/load.ImportErrorf({0x7ffca7ec715d, 0x18}, {0x99f456, 0x18}, {0xc00021f6b0, 0xc00021fb58, 0x0})
    /usr/lib/go/src/cmd/go/internal/load/pkg.go:500 +0x19b
cmd/go/internal/load.(*Package).load(0xc0006dc000, {0xa70560, 0xc000122000}, {0x34, 0xc0, 0x2}, {0x7ffca7ec715d, 0x0}, 0xc00021fb58, {0x0, ...}, ...)
    /usr/lib/go/src/cmd/go/internal/load/pkg.go:1846 +0x17dc
cmd/go/internal/load.loadImport({0xa70560, 0xc000122000}, {0x60, 0x5, 0xa7}, 0xc0001f62c0, {0x7ffca7ec715d, 0x18}, {0xc00002c034, 0x24}, ...)
    /usr/lib/go/src/cmd/go/internal/load/pkg.go:718 +0x4dd
cmd/go/internal/load.PackagesAndErrors({0xa70560, 0xc000122000}, {0x2d, 0x0, 0x0}, {0xc000134030, 0x2, 0x2})
    /usr/lib/go/src/cmd/go/internal/load/pkg.go:2468 +0x809
cmd/go/internal/work.runBuild({0xa70560, 0xc000122000}, 0xc000150420, {0xc000134030, 0x2, 0x2})
    /usr/lib/go/src/cmd/go/internal/work/build.go:371 +0xa5
main.invoke(0xd804e0, {0xc000134010, 0x4, 0x4})
    /usr/lib/go/src/cmd/go/main.go:216 +0x2f6
main.main()
    /usr/lib/go/src/cmd/go/main.go:173 +0x78e

seankhliao commented 2 years ago

Note the invocation won't succeed even if the panic is fixed, '' passes an argument of an empty string and go requires flags to come before other arguments.

More minimal panic:

$ go build '="'
panic: path "=\"" not in error "invalid import path \"=\\\"\"" [recovered]
    panic: path "=\"" not in error "invalid import path \"=\\\"\""

cc @bcmills @matloob

SilverRainZ commented 2 years ago

Hello, I am trying to fix this.

Once double quoted string appears in the import path, build panics. for example: go build '"a"' also crash:

``` $ go build '"a"' panic: path "\"a\"" not in error "invalid import path \"\\\"a\\\"\"" [recovered] panic: path "\"a\"" not in error "invalid import path \"\\\"a\\\"\"" goroutine 1 [running]: cmd/go/internal/load.(*preload).flush(0xc0001196c0) /usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:1041 +0x78 panic({0x14f7e80, 0xc000119770}) /usr/local/Cellar/go/1.17/libexec/src/runtime/panic.go:1038 +0x215 cmd/go/internal/load.ImportErrorf({0x7ffeefbff889, 0x3}, {0x159e971, 0x3}, {0xc0001d36b0, 0xc0001d3b58, 0x0}) /usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:500 +0x19b cmd/go/internal/load.(*Package).load(0xc000184b00, {0x166f9c0, 0xc000122000}, {0x24, 0x80, 0x2}, {0x7ffeefbff889, 0x0}, 0xc0001d3b58, {0x0, ...}, ...) /usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:1846 +0x17dc cmd/go/internal/load.loadImport({0x166f9c0, 0xc000122000}, {0xc0, 0xf9, 0x66}, 0xc0001196c0, {0x7ffeefbff889, 0x3}, {0xc000028024, 0x17}, ...) /usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:718 +0x4dd cmd/go/internal/load.PackagesAndErrors({0x166f9c0, 0xc000122000}, {0x2d, 0x0, 0x0}, {0xc00011c1a0, 0x1, 0x1}) /usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:2468 +0x809 cmd/go/internal/work.runBuild({0x166f9c0, 0xc000122000}, 0xc000134480, {0xc00011c1a0, 0x1, 0x1}) /usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/work/build.go:371 +0xa5 main.invoke(0x1988580, {0xc00011c190, 0x2, 0x2}) /usr/local/Cellar/go/1.17/libexec/src/cmd/go/main.go:216 +0x2f6 main.main() /usr/local/Cellar/go/1.17/libexec/src/cmd/go/main.go:173 +0x78e ```

$ go version
go version go1.17 darwin/amd64

In "cmd/go/internal/load/pkg.go", ImportErrorf checks whether the path (importError.importPath) is contained in err (importError.err):

// func ImportErrorf
    if errStr := err.Error(); !strings.Contains(errStr, path) {
        panic(fmt.Sprintf("path %q not in error %q", path, errStr))
    }

err is created by stmt like fmt.Errorf("invalid import path %q", importPath), so the import path in error message is escaped. If import path contians any char that can be escaped by %q, strings.Contains returns false.

IMO, this check is quite unreasonable, do we really need it？

gopherbot commented 2 years ago

Change https://golang.org/cl/358815 mentions this issue: cmd/go: make escape consistent in ImportErrorf

gopherbot commented 2 years ago

Change https://golang.org/cl/372398 mentions this issue: cmd/go/internal/load: prevent calling ImportErrorf when the err is *module.InvalidPathError

srinivas-pokala commented 1 year ago

@bcmills , Is the above CL(https://golang.org/cl/372398) working well for the Issue or else is still there is someother changes required for this.

gopherbot commented 1 year ago

Change https://go.dev/cl/452715 mentions this issue: cmd/go/internal/load: bypass calling ImportErrorf function

golang / go