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

Work around Windows bug where arguments are not properly quoted #3389

Merged
merged 9 commits into from
Jul 8, 2024
3 changes: 1 addition & 2 deletions cmd/state-exec/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ package main
import (
"fmt"
"os"
"os/exec"
)

func runCmd(meta *executorMeta) (int, error) {
userArgs := os.Args[1:]
cmd := exec.Command(meta.MatchingBin, userArgs...)
cmd := Command(meta.MatchingBin, userArgs...)
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
Expand Down
10 changes: 10 additions & 0 deletions cmd/state-exec/cmd_lin_mac.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//go:build !windows
// +build !windows

package main

import "os/exec"

func Command(name string, arg ...string) *exec.Cmd {
return exec.Command(name, arg...)
}
99 changes: 99 additions & 0 deletions cmd/state-exec/cmd_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package main

import (
"os/exec"
"path/filepath"
"strings"
"syscall"

"github.com/ActiveState/cli/cmd/state-exec/internal/logr"
)

func Command(name string, arg ...string) *exec.Cmd {
cmd := exec.Command(name, arg...)

exeName := filepath.Base(strings.ToLower(name))
if exeName == "cmd" || strings.HasSuffix(exeName, ".bat") || strings.HasSuffix(exeName, ".cmd") {
// Go currently does not escape arguments properly on Windows, it account for spaces and tab characters, but not
// other characters that need escaping such as `<` and `>`.
// This can be dropped once we update to a Go version that fixes this bug: https://github.com/golang/go/issues/68313
cmd.SysProcAttr = &syscall.SysProcAttr{CmdLine: makeCmdLine(cmd.Args)}
logr.Debug("processed command line: %s", cmd.SysProcAttr.CmdLine)
}

return cmd
}

// makeCmdLine builds a command line out of args by escaping "special"
// characters and joining the arguments with spaces.
// Based on syscall\exec_windows.go
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.
// Based on syscall\exec_windows.go
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
// Based on https://github.com/sebres/PoC/blob/master/SB-0D-001-win-exec/SOLUTION.md#definition
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
}
8 changes: 4 additions & 4 deletions internal/osutils/exeutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func ExecSimple(bin string, args []string, env []string) (string, string, error)

func ExecSimpleFromDir(dir, bin string, args []string, env []string) (string, string, error) {
logging.Debug("ExecSimpleFromDir: dir: %s, bin: %s, args: %#v, env: %#v", dir, bin, args, env)
c := exec.Command(bin, args...)
c := Command(bin, args...)
if dir != "" {
c.Dir = dir
}
Expand All @@ -117,7 +117,7 @@ func ExecSimpleFromDir(dir, bin string, args []string, env []string) (string, st

// Execute will run the given command and with optional settings for the exec.Cmd struct
func Execute(command string, arg []string, optSetter func(cmd *exec.Cmd) error) (int, *exec.Cmd, error) {
cmd := exec.Command(command, arg...)
cmd := Command(command, arg...)
logging.Debug("Executing command: %s, with args: %s", cmd, arg)
if optSetter != nil {
if err := optSetter(cmd); err != nil {
Expand Down Expand Up @@ -145,7 +145,7 @@ func ExecuteAndPipeStd(command string, arg []string, env []string) (int, *exec.C
// ExecuteAndForget will run the given command in the background, returning immediately.
func ExecuteAndForget(command string, args []string, opts ...func(cmd *exec.Cmd) error) (*os.Process, error) {
logging.Debug("Executing: %s %v", command, args)
cmd := exec.Command(command, args...)
cmd := Command(command, args...)

for _, optSetter := range opts {
if err := optSetter(cmd); err != nil {
Expand All @@ -172,7 +172,7 @@ func ExecuteAndForget(command string, args []string, opts ...func(cmd *exec.Cmd)
func ExecuteInBackground(command string, args []string, opts ...func(cmd *exec.Cmd) error) (*exec.Cmd, *bytes.Buffer, *bytes.Buffer, error) {
logging.Debug("Executing: %s %v", command, args)

cmd := exec.Command(command, args...)
cmd := Command(command, args...)
var stdoutBuf, stderrBuf bytes.Buffer

for _, optSetter := range opts {
Expand Down
6 changes: 6 additions & 0 deletions internal/osutils/exeutils_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@

package osutils

import "os/exec"

const ExeExtension = ""

var exts = []string{}

func Command(name string, arg ...string) *exec.Cmd {
return exec.Command(name, arg...)
}
91 changes: 91 additions & 0 deletions internal/osutils/exeutils_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package osutils

import (
"os"
"os/exec"
"path/filepath"
"strings"
"syscall"

"github.com/thoas/go-funk"
)
Expand All @@ -15,3 +18,91 @@ func init() {
PATHEXT := os.Getenv("PATHEXT")
exts = funk.Uniq(funk.Map(strings.Split(PATHEXT, string(os.PathListSeparator)), strings.ToLower).([]string)).([]string)
}

func Command(name string, arg ...string) *exec.Cmd {
cmd := exec.Command(name, arg...)

exeName := filepath.Base(strings.ToLower(name))
if exeName == "cmd" || strings.HasSuffix(exeName, ".bat") || strings.HasSuffix(exeName, ".cmd") {
// Go currently does not escape arguments properly on Windows, it account for spaces and tab characters, but not
// other characters that need escaping such as `<` and `>`.
// This can be dropped once we update to a Go version that fixes this bug: https://github.com/golang/go/issues/68313
cmd.SysProcAttr = &syscall.SysProcAttr{CmdLine: makeCmdLine(cmd.Args)}
}

return cmd
}

// makeCmdLine builds a command line out of args by escaping "special"
// characters and joining the arguments with spaces.
// Based on syscall\exec_windows.go
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.
// Based on syscall\exec_windows.go
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
// Based on https://github.com/sebres/PoC/blob/master/SB-0D-001-win-exec/SOLUTION.md#definition
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
}
12 changes: 6 additions & 6 deletions internal/testhelpers/e2e/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,24 @@ import (
"fmt"
"io/fs"
"os"
"os/exec"
"path/filepath"
"regexp"
"runtime"
"strings"
"testing"
"time"

"github.com/ActiveState/cli/internal/subshell"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/platform/model/buildplanner"
"github.com/ActiveState/cli/pkg/projectfile"
"github.com/ActiveState/termtest"
"github.com/go-openapi/strfmt"
"github.com/google/uuid"
"github.com/phayes/permbits"
"github.com/stretchr/testify/require"

"github.com/ActiveState/cli/internal/subshell"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/platform/model/buildplanner"
"github.com/ActiveState/cli/pkg/projectfile"

"github.com/ActiveState/cli/internal/condition"
"github.com/ActiveState/cli/internal/config"
"github.com/ActiveState/cli/internal/constants"
Expand Down Expand Up @@ -298,7 +298,7 @@ func (s *Session) SpawnCmdWithOpts(exe string, optSetters ...SpawnOptSetter) *Sp
args = spawnOpts.Args
}

cmd := exec.Command(shell, args...)
cmd := osutils.Command(shell, args...)

cmd.Env = spawnOpts.Env
if spawnOpts.Dir != "" {
Expand Down
4 changes: 4 additions & 0 deletions pkg/platform/runtime/executors/executors.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func (es *Executors) ExecutorSrc() (string, error) {
return installation.ExecutorExec()
}

func (es *Executors) SetExecutorSrc(path string) {
es.altExecSrcPath = path
}

func (es *Executors) Apply(sockPath string, targeter Targeter, env map[string]string, exes envdef.ExecutablePaths) error {
logging.Debug("Creating executors at %s, exes: %v", es.executorPath, exes)

Expand Down
Loading
Loading