Browse Source

properly quote the path to garble in -toolexec

If we don't quote it, paths containing spaces or quote characters will
fail. For instance, the added test without the fix fails:

        > env NAME='with spaces'
        > mkdir $NAME
        > cp $EXEC_PATH $NAME/garble$exe
        > exec $NAME/garble$exe build main.go
        [stderr]
        go tool compile: fork/exec $WORK/with: no such file or directory
        exit status 1

Luckily, the fix is easy: we bundle Go's cmd/internal/quoted package,
which implements a QuotedJoin API for this very purpose.

Fixes #544.
Daniel Martí 6 months ago
parent
commit
f37561589b
4 changed files with 47 additions and 1 deletions
  1. 2 0
      CHANGELOG.md
  2. 7 1
      main.go
  3. 6 0
      main_test.go
  4. 32 0
      testdata/scripts/goenv.txt

+ 2 - 0
CHANGELOG.md

@@ -9,6 +9,7 @@ Noteworthy changes include:
 * Initial support for obfuscating generic code - [#414]
 * Remove unused imports in `-literals` more reliably - [#481]
 * Support obfuscating package paths ending with `.go` - [#539]
+* Support installing garble in paths containing spaces - [#544]
 * Avoid a panic when obfuscating variadic functions - [#524]
 * Avoid a "refusing to list package" panic in `garble test` - [#522]
 * Some module builds are now used as regression tests - [#240]
@@ -132,6 +133,7 @@ Known bugs:
 [#522]: https://github.com/burrowers/garble/issues/522
 [#524]: https://github.com/burrowers/garble/issues/524
 [#539]: https://github.com/burrowers/garble/issues/539
+[#544]: https://github.com/burrowers/garble/issues/544
 
 [v0.6.0]: https://github.com/burrowers/garble/releases/tag/v0.6.0
 [#449]: https://github.com/burrowers/garble/issues/449

+ 7 - 1
main.go

@@ -542,7 +542,13 @@ This command wraps "go %s". Below is its help:
 	// This way, all garble processes see the same flag values.
 	var toolexecFlag strings.Builder
 	toolexecFlag.WriteString("-toolexec=")
-	toolexecFlag.WriteString(cache.ExecPath)
+	quotedExecPath, err := cmdgoQuotedJoin([]string{cache.ExecPath})
+	if err != nil {
+		// Can only happen if the absolute path to the garble binary contains
+		// both single and double quotes. Seems extremely unlikely.
+		return nil, err
+	}
+	toolexecFlag.WriteString(quotedExecPath)
 	appendFlags(&toolexecFlag, false)
 	goArgs = append(goArgs, toolexecFlag.String())
 

+ 6 - 0
main_test.go

@@ -53,6 +53,11 @@ var update = flag.Bool("u", false, "update testscript output files")
 func TestScripts(t *testing.T) {
 	t.Parallel()
 
+	execPath, err := os.Executable()
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	p := testscript.Params{
 		Dir: filepath.Join("testdata", "scripts"),
 		Setup: func(env *testscript.Env) error {
@@ -77,6 +82,7 @@ func TestScripts(t *testing.T) {
 				"GOGC=off",
 
 				"gofullversion="+runtime.Version(),
+				"EXEC_PATH="+execPath,
 			)
 
 			if os.Getenv("TESTSCRIPT_COVER_DIR") != "" {

+ 32 - 0
testdata/scripts/goenv.txt

@@ -0,0 +1,32 @@
+# We need to properly quote the path to garble for toolexec.
+# If we don't, characters like spaces or quotes will result in errors.
+# EXEC_PATH is the test binary's os.Executable.
+# Copying it to a path with basename "garble" makes testscript run our main func.
+# Note that double quotes are not allowed in Windows filenames.
+env NAME='with spaces'
+mkdir $NAME
+cp $EXEC_PATH $NAME/garble$exe
+exec $NAME/garble$exe build main.go
+
+[!windows] env NAME='with"double"quotes'
+[!windows] mkdir $NAME
+[!windows] cp $EXEC_PATH $NAME/garble$exe
+[!windows] exec $NAME/garble$exe build main.go
+
+env NAME='with''single''quotes'
+mkdir $NAME
+cp $EXEC_PATH $NAME/garble$exe
+exec $NAME/garble$exe build main.go
+
+[!windows] env NAME='with"both''quotes'
+[!windows] mkdir $NAME
+[!windows] cp $EXEC_PATH $NAME/garble$exe
+[!windows] ! exec $NAME/garble$exe build main.go
+[!windows] stderr 'cannot be quoted'
+
+-- main.go --
+package main
+
+func main() {
+	println("hello world")
+}