Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: commandLineToArgv() does not correctly parse argv[0] in Windows #50028

Open
tfogo opened this issue Dec 7, 2021 · 2 comments
Open

os: commandLineToArgv() does not correctly parse argv[0] in Windows #50028

tfogo opened this issue Dec 7, 2021 · 2 comments

Comments

@tfogo
Copy link

@tfogo tfogo commented Dec 7, 2021

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

$ go version
go version go1.17.4 windows/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\tim\AppData\Local\go-build
set GOENV=C:\Users\tim\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\tim\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\tim\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.17.4
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\tim\AppData\Local\Temp\go-build2702872218=/tmp/go-build -gno-record-gcc-switches

What did you do?

When running a go program in Windows cmd with a path such as this:

C:\"Program Files"\main.exe

os.Args will contain two arguments: C:"Program and Files\main.exe.

Here is an example that uses the definition of commandLineToArgv() from os/exec_windows.go: https://go.dev/play/p/VgBbsDdApUg

What did you expect to see?

os.Args should contain a single arg: C:\Program Files\main.exe.

This is not happening because we are not giving special treatment to the first argument. As specified in the documentation for Parsing C++ command-line arguments:

The first argument (argv[0]) is treated specially. It represents the program name. Because it must be a valid pathname, parts surrounded by double quote marks (") are allowed. The double quote marks aren't included in the argv[0] output. The parts surrounded by double quote marks prevent interpretation of a space or tab character as the end of the argument. The later rules in this list don't apply.

In this case, (\") is interpreted as a literal double quote mark (") and the space in Program Files is interpreted as the end of an argument. This is incorrect for the first argument.

David Deley's command line parsing reference also corroborates that argument 0 should be parsed specially. (Although admittedly it's not very clear - the flowchart skips this step for example.)

As far as I can see, when we implemented commandLineToArgv() in #15588, we used the C++ documentation and David Deley's page as reference. However, we did not correctly implement the algorithm specified in these references. So I believe this behavior is unintended. This problem has existed since we fixed #15588.

What did you see instead?

os.Args contains two arguments since the first arg is not parsed correctly: C:"Program and Files\main.exe

Please let me know if my diagnosis is correct. If so, I would be happy to submit a patch for this.

@toothrot
Copy link
Contributor

@toothrot toothrot commented Dec 8, 2021

/cc @bufflig

@toothrot toothrot added this to the Backlog milestone Dec 8, 2021
@iwdgo
Copy link
Contributor

@iwdgo iwdgo commented Dec 18, 2021

#17149 contains a detailed discussion of parsing when using CMD as mentioned in the example. It also suggests a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants