golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.91k stars 17.52k forks source link

syscall: sort environment passed to CreateProcess / CreateProcessAsUser #29530

Open petemoore opened 5 years ago

petemoore commented 5 years ago

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

go version go1.11.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

go envOutput
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/pmoore/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pmoore/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.2/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v9/mll6p_rj5h94dt_m5m8j0f9c0000gn/T/go-build797508603=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I looked at https://github.com/golang/go/blob/go1.11.4/src/syscall/exec_windows.go#L97-L122

What did you expect to see?

I expected to see the env vars sorted alphabetically by name, with case-insensitive sort, Unicode order, without regard to locale.

What did you see instead?

The code does not sort the environment variable entries. The MSDN docs state:

All strings in the environment block must be sorted alphabetically by name. The sort is case-insensitive, Unicode order, without regard to locale. Because the equal sign is a separator, it must not be used in the name of an environment variable.

Note, this hasn't caused me any problems - but it seems like this could cause problems with any Windows kernel functions that expect the env to be sorted.

petemoore commented 5 years ago

This isn't actually a documentation bug (I see this issue just received the Documentation label) but a potential golang standard library bug for Windows platform.

dominikh commented 5 years ago

Evil machine overlords etc etc…

alexbrainman commented 5 years ago

This SGTM.

I don't see the downside of this change. But others might disagree.

Alex

gopherbot commented 5 years ago

Change https://golang.org/cl/160828 mentions this issue: syscall: perform environment variable sort for createEnvBlock

bradfitz commented 5 years ago

If this isn't causing any known problems, I think we should wait until Go 1.14 (and early-in-cycle).

odeke-em commented 5 years ago

SGTM!