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>

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 {

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

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