syscall: sort Windows env block in StartProcess

Fixes #29530

Change-Id: Ia28c78274b9288bfa5de9ccb142a452d291a5b66
Reviewed-on: https://go-review.googlesource.com/c/go/+/694435
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go
index 3ba2fbe0ec3b6808c2449875ad1a6bd1d55e950d..96089fcd93a63478dab7c821773b23ef1bb2a1ff 100644 (file)
--- a/src/syscall/exec_windows.go
+++ b/src/syscall/exec_windows.go
@@ -9,6 +9,7 @@ package syscall
 import (
        "internal/bytealg"
        "runtime"
+       "slices"
        "sync"
        "unicode/utf16"
        "unsafe"
@@ -113,6 +114,49 @@ func makeCmdLine(args []string) string {
        return string(b)
 }
 
+func envSorted(envv []string) []string {
+       if len(envv) < 2 {
+               return envv
+       }
+
+       lowerKeyCache := map[string][]byte{} // lowercased keys to avoid recomputing them in sort
+       lowerKey := func(kv string) []byte {
+               eq := bytealg.IndexByteString(kv, '=')
+               if eq < 0 {
+                       return nil
+               }
+               k := kv[:eq]
+               v, ok := lowerKeyCache[k]
+               if !ok {
+                       v = []byte(k)
+                       for i, b := range v {
+                               // We only normalize ASCII for now.
+                               // In practice, all environment variables are ASCII, and the
+                               // syscall package can't import "unicode" anyway.
+                               // Also, per https://nullprogram.com/blog/2023/08/23/ the
+                               // sorting of environment variables doesn't really matter.
+                               // TODO(bradfitz): use RtlCompareUnicodeString instead,
+                               // per that blog post? For now, ASCII is good enough.
+                               if 'a' <= b && b <= 'z' {
+                                       v[i] -= 'a' - 'A'
+                               }
+                       }
+                       lowerKeyCache[k] = v
+               }
+               return v
+       }
+
+       cmpEnv := func(a, b string) int {
+               return bytealg.Compare(lowerKey(a), lowerKey(b))
+       }
+
+       if !slices.IsSortedFunc(envv, cmpEnv) {
+               envv = slices.Clone(envv)
+               slices.SortFunc(envv, cmpEnv)
+       }
+       return envv
+}
+
 // createEnvBlock converts an array of environment strings into
 // the representation required by CreateProcess: a sequence of NUL
 // terminated strings followed by a nil.
@@ -122,6 +166,12 @@ func createEnvBlock(envv []string) ([]uint16, error) {
        if len(envv) == 0 {
                return utf16.Encode([]rune("\x00\x00")), nil
        }
+
+       // https://learn.microsoft.com/en-us/windows/win32/procthread/changing-environment-variables
+       // says that: "All strings in the environment block must be sorted
+       // alphabetically by name."
+       envv = envSorted(envv)
+
        var length int
        for _, s := range envv {
                if bytealg.IndexByteString(s, 0) != -1 {

diff --git a/src/syscall/exec_windows_test.go b/src/syscall/exec_windows_test.go
index 90a13af8d1bdbbef2c75d7f1bbf9c563ebd096ff..60a995e4e74eff1abab8cddf551b1a0d10b7fe9b 100644 (file)
--- a/src/syscall/exec_windows_test.go
+++ b/src/syscall/exec_windows_test.go
@@ -10,6 +10,7 @@ import (
        "os"
        "os/exec"
        "path/filepath"
+       "slices"
        "syscall"
        "testing"
        "time"
@@ -48,6 +49,37 @@ func TestEscapeArg(t *testing.T) {
        }
 }
 
+func TestEnvBlockSorted(t *testing.T) {
+       tests := []struct {
+               env  []string
+               want []string
+       }{
+               {},
+               {
+                       env:  []string{"A=1"},
+                       want: []string{"A=1"},
+               },
+               {
+                       env:  []string{"A=1", "B=2", "C=3"},
+                       want: []string{"A=1", "B=2", "C=3"},
+               },
+               {
+                       env:  []string{"C=3", "B=2", "A=1"},
+                       want: []string{"A=1", "B=2", "C=3"},
+               },
+               {
+                       env:  []string{"c=3", "B=2", "a=1"},
+                       want: []string{"a=1", "B=2", "c=3"},
+               },
+       }
+       for _, tt := range tests {
+               got := syscall.EnvSorted(tt.env)
+               if !slices.Equal(got, tt.want) {
+                       t.Errorf("EnvSorted(%q) = %q, want %q", tt.env, got, tt.want)
+               }
+       }
+}
+
 func TestChangingProcessParent(t *testing.T) {
        if os.Getenv("GO_WANT_HELPER_PROCESS") == "parent" {
                // in parent process

diff --git a/src/syscall/export_windows_test.go b/src/syscall/export_windows_test.go
index eccf1bccace4d9585a452504455638c5fa203f42..a816d7760b74a857729c0a5b645f672088005b7b 100644 (file)
--- a/src/syscall/export_windows_test.go
+++ b/src/syscall/export_windows_test.go
@@ -12,3 +12,5 @@ const PROC_THREAD_ATTRIBUTE_HANDLE_LIST = _PROC_THREAD_ATTRIBUTE_HANDLE_LIST
 
 var EncodeWTF16 = encodeWTF16
 var DecodeWTF16 = decodeWTF16
+
+var EnvSorted = envSorted