Skip to content

syscall/exec_windows.go: appendEscapeArg does not escape all necessary characters #68313

Closed as not planned
@Naatan

Description

@Naatan

Go version

go version go1.22.4 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Nathan\AppData\Local\go-build
set GOENV=C:\Users\Nathan\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Nathan\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Nathan\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:/Program Files/Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.4
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=C:\Users\Nathan\Downloads\battest\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\Nathan\AppData\Local\Temp\go-build2629603369=/tmp/go-build -gno-record-gcc-switches

What did you do?

I'm trying to run a go executable with problematic arguments, eg.

go run test.go "a<b" "hello world"

Under the hood this will forward the arguments to a file called test.bat, and print the arguments.

This is what the files look like:

test.go:

package main

import (
	"os"
	"os/exec"
)

func main() {
	cmd := exec.Command("C:\\Windows\\System32\\cmd.exe", append([]string{"/C", ".\\test.bat"}, os.Args[1:]...)...)
	cmd.Dir = "C:\\Users\\Nathan\\Downloads\\battest"
	cmd.Stdout = os.Stdout
	cmd.Stderr = os.Stderr
	err := cmd.Run()
	if err != nil {
		panic(err)
	}
}

test.bat:

@echo %*

What did you see happen?

$ go run test_issue.go "a<b" "hello world"
The system cannot find the file specified.
panic: exit status 1

What did you expect to see?

$ go run test.go "a<b" "hello world"
"a<b" "hello world"

Through debugging I found that the issue is down to the appendEscapeArg function in exec_windows.go under the syscall package. This function will add quotes around arguments that have spaces or tabs, but it doesn't look for any other characters that need escaping.

I noticed you can override the commandLine that syscall is served and bypass this function, and so I copied and tweaked the relevant code to also escape arguments that contain <, or >. Now it works as expected.

I'd open a PR, but I don't know enough about windows, particularly; are there other characters that need escaping?

Here's my fix for what it's worth:

package main

import (
	"os"
	"os/exec"
	"syscall"
)

func main() {
	cmd := exec.Command("C:\\Windows\\System32\\cmd.exe", append([]string{"/C", ".\\test.bat"}, os.Args[1:]...)...)
	cmd.SysProcAttr = &syscall.SysProcAttr{CmdLine: makeCmdLine(cmd.Args)}
	cmd.Stdout = os.Stdout
	cmd.Stderr = os.Stderr
	err := cmd.Run()
	if err != nil {
		panic(err)
	}
}

// makeCmdLine builds a command line out of args by escaping "special"
// characters and joining the arguments with spaces.
func makeCmdLine(args []string) string {
	var b []byte
	for _, v := range args {
		if len(b) > 0 {
			b = append(b, ' ')
		}
		b = appendEscapeArg(b, v)
	}
	return string(b)
}

// appendEscapeArg escapes the string s, as per escapeArg,
// appends the result to b, and returns the updated slice.
func appendEscapeArg(b []byte, s string) []byte {
	if len(s) == 0 {
		return append(b, `""`...)
	}

	needsBackslash := false
	needsQuotes := false
	for i := 0; i < len(s); i++ {
		switch s[i] {
		case '"', '\\':
			needsBackslash = true
		case ' ', '\t', '<', '>':
			needsQuotes = true
		}
	}

	if !needsBackslash && !needsQuotes {
		// No special handling required; normal case.
		return append(b, s...)
	}
	if !needsBackslash {
		// hasSpace is true, so we need to quote the string.
		b = append(b, '"')
		b = append(b, s...)
		return append(b, '"')
	}

	if needsQuotes {
		b = append(b, '"')
	}
	slashes := 0
	for i := 0; i < len(s); i++ {
		c := s[i]
		switch c {
		default:
			slashes = 0
		case '\\':
			slashes++
		case '"':
			for ; slashes > 0; slashes-- {
				b = append(b, '\\')
			}
			b = append(b, '\\')
		}
		b = append(b, c)
	}
	if needsQuotes {
		for ; slashes > 0; slashes-- {
			b = append(b, '\\')
		}
		b = append(b, '"')
	}

	return b
}

The above is identical to the native implementation, except that it renames hasSpace to needsQuotes and adds , '<', '>' to the needsQuotes switch case.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions