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

Jump to bottom

syscall: sort environment passed to CreateProcess / CreateProcessAsUser #29530

Open
petemoore opened this issue Jan 3, 2019 · 6 comments
Open

syscall: sort environment passed to CreateProcess / CreateProcessAsUser #29530

petemoore opened this issue Jan 3, 2019 · 6 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@petemoore
Copy link

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
Copy link
Author

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 dominikh added this to the Go1.12 milestone Jan 3, 2019
@dominikh dominikh added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 3, 2019
@dominikh
Copy link
Member

dominikh commented Jan 3, 2019

Evil machine overlords etc etc…

@dominikh dominikh changed the title MSDN docs state that environment blocks passed to CreateProcess / CreateProcessAsUser should be sorted syscall: sort environment passed to CreateProcess / CreateProcessAsUser Jan 3, 2019
@alexbrainman
Copy link
Member

This SGTM.

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

Alex

@andybons andybons modified the milestones: Go1.12, Go1.13 Feb 12, 2019
@gopherbot
Copy link

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

@odeke-em odeke-em self-assigned this May 1, 2019
@bradfitz
Copy link
Contributor

bradfitz commented May 6, 2019

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

@odeke-em
Copy link
Member

odeke-em commented May 6, 2019

SGTM!

@odeke-em odeke-em modified the milestones: Go1.13, Go1.14 May 6, 2019
@odeke-em odeke-em added the early-in-cycle A change that should be done early in the 3 month dev cycle. label May 6, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
8 participants
@bradfitz @dominikh @rsc @petemoore @andybons @odeke-em @gopherbot @alexbrainman