Skip to content

Commit

Permalink
os/exec: add GODEBUG setting to opt out of ErrDot changes
Browse files Browse the repository at this point in the history
The changes are likely to break users, and we need
to make it easy to unbreak without code changes.

For golang#43724.
Fixes golang#53962.

Change-Id: I105c5d6c801d354467e0cefd268189c18846858e
Reviewed-on: https://go-review.googlesource.com/c/go/+/419794
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
  • Loading branch information
rsc authored and jproberts committed Aug 10, 2022
1 parent 90063b1 commit 47eb687
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 40 deletions.
8 changes: 5 additions & 3 deletions src/go/build/deps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,11 @@ var depsRules = `
os/signal, STR
< path/filepath
< io/ioutil, os/exec;
< io/ioutil;
os < internal/godebug;
path/filepath, internal/godebug < os/exec;
io/ioutil, os/exec, os/signal
< OS;
Expand All @@ -187,8 +191,6 @@ var depsRules = `
OS
< golang.org/x/sys/cpu;
os < internal/godebug;
# FMT is OS (which includes string routines) plus reflect and fmt.
# It does not include package log, which should be avoided in core packages.
strconv, unicode
Expand Down
86 changes: 52 additions & 34 deletions src/os/exec/dot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,40 +56,58 @@ func TestLookPath(t *testing.T) {

// Add "." to PATH so that exec.LookPath looks in the current directory on all systems.
// And try to trick it with "../testdir" too.
for _, dir := range []string{".", "../testdir"} {
t.Run(pathVar+"="+dir, func(t *testing.T) {
t.Setenv(pathVar, dir+string(filepath.ListSeparator)+origPath)
good := dir + "/execabs-test"
if found, err := LookPath(good); err != nil || !strings.HasPrefix(found, good) {
t.Fatalf(`LookPath(%#q) = %#q, %v, want "%s...", nil`, good, found, err, good)
}
if runtime.GOOS == "windows" {
good = dir + `\execabs-test`
if found, err := LookPath(good); err != nil || !strings.HasPrefix(found, good) {
t.Fatalf(`LookPath(%#q) = %#q, %v, want "%s...", nil`, good, found, err, good)
}
}

if _, err := LookPath("execabs-test"); err == nil {
t.Fatalf("LookPath didn't fail when finding a non-relative path")
} else if !errors.Is(err, ErrDot) {
t.Fatalf("LookPath returned unexpected error: want Is ErrDot, got %q", err)
}

cmd := Command("execabs-test")
if cmd.Err == nil {
t.Fatalf("Command didn't fail when finding a non-relative path")
} else if !errors.Is(cmd.Err, ErrDot) {
t.Fatalf("Command returned unexpected error: want Is ErrDot, got %q", cmd.Err)
}
cmd.Err = nil

// Clearing cmd.Err should let the execution proceed,
// and it should fail because it's not a valid binary.
if err := cmd.Run(); err == nil {
t.Fatalf("Run did not fail: expected exec error")
} else if errors.Is(err, ErrDot) {
t.Fatalf("Run returned unexpected error ErrDot: want error like ENOEXEC: %q", err)
for _, errdot := range []string{"1", "0"} {
t.Run("GODEBUG=execerrdot="+errdot, func(t *testing.T) {
t.Setenv("GODEBUG", "execerrdot="+errdot)
for _, dir := range []string{".", "../testdir"} {
t.Run(pathVar+"="+dir, func(t *testing.T) {
t.Setenv(pathVar, dir+string(filepath.ListSeparator)+origPath)
good := dir + "/execabs-test"
if found, err := LookPath(good); err != nil || !strings.HasPrefix(found, good) {
t.Fatalf(`LookPath(%#q) = %#q, %v, want "%s...", nil`, good, found, err, good)
}
if runtime.GOOS == "windows" {
good = dir + `\execabs-test`
if found, err := LookPath(good); err != nil || !strings.HasPrefix(found, good) {
t.Fatalf(`LookPath(%#q) = %#q, %v, want "%s...", nil`, good, found, err, good)
}
}

_, err := LookPath("execabs-test")
if errdot == "1" {
if err == nil {
t.Fatalf("LookPath didn't fail when finding a non-relative path")
} else if !errors.Is(err, ErrDot) {
t.Fatalf("LookPath returned unexpected error: want Is ErrDot, got %q", err)
}
} else {
if err != nil {
t.Fatalf("LookPath failed unexpectedly: %v", err)
}
}

cmd := Command("execabs-test")
if errdot == "1" {
if cmd.Err == nil {
t.Fatalf("Command didn't fail when finding a non-relative path")
} else if !errors.Is(cmd.Err, ErrDot) {
t.Fatalf("Command returned unexpected error: want Is ErrDot, got %q", cmd.Err)
}
cmd.Err = nil
} else {
if cmd.Err != nil {
t.Fatalf("Command failed unexpectedly: %v", err)
}
}

// Clearing cmd.Err should let the execution proceed,
// and it should fail because it's not a valid binary.
if err := cmd.Run(); err == nil {
t.Fatalf("Run did not fail: expected exec error")
} else if errors.Is(err, ErrDot) {
t.Fatalf("Run returned unexpected error ErrDot: want error like ENOEXEC: %q", err)
}
})
}
})
}
Expand Down
5 changes: 5 additions & 0 deletions src/os/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@
// log.Fatal(err)
// }
//
// Setting the environment variable GODEBUG=execerrdot=0
// disables generation of ErrDot entirely, temporarily restoring the pre-Go 1.19
// behavior for programs that are unable to apply more targeted fixes.
// A future version of Go may remove support for this variable.
//
// Before adding such overrides, make sure you understand the
// security implications of doing so.
// See https://go.dev/blog/path-security for more information.
Expand Down
3 changes: 2 additions & 1 deletion src/os/exec/lp_plan9.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package exec

import (
"errors"
"internal/godebug"
"io/fs"
"os"
"path/filepath"
Expand Down Expand Up @@ -53,7 +54,7 @@ func LookPath(file string) (string, error) {
for _, dir := range filepath.SplitList(path) {
path := filepath.Join(dir, file)
if err := findExecutable(path); err == nil {
if !filepath.IsAbs(path) {
if !filepath.IsAbs(path) && godebug.Get("execerrdot") != "0" {
return path, &Error{file, ErrDot}
}
return path, nil
Expand Down
3 changes: 2 additions & 1 deletion src/os/exec/lp_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package exec

import (
"errors"
"internal/godebug"
"io/fs"
"os"
"path/filepath"
Expand Down Expand Up @@ -56,7 +57,7 @@ func LookPath(file string) (string, error) {
}
path := filepath.Join(dir, file)
if err := findExecutable(path); err == nil {
if !filepath.IsAbs(path) {
if !filepath.IsAbs(path) && godebug.Get("execerrdot") != "0" {
return path, &Error{file, ErrDot}
}
return path, nil
Expand Down
6 changes: 5 additions & 1 deletion src/os/exec/lp_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package exec

import (
"errors"
"internal/godebug"
"io/fs"
"os"
"path/filepath"
Expand Down Expand Up @@ -102,6 +103,9 @@ func LookPath(file string) (string, error) {
)
if _, found := syscall.Getenv("NoDefaultCurrentDirectoryInExePath"); !found {
if f, err := findExecutable(filepath.Join(".", file), exts); err == nil {
if godebug.Get("execerrdot") == "0" {
return f, nil
}
dotf, dotErr = f, &Error{file, ErrDot}
}
}
Expand All @@ -124,7 +128,7 @@ func LookPath(file string) (string, error) {
}
}

if !filepath.IsAbs(f) {
if !filepath.IsAbs(f) && godebug.Get("execerrdot") != "0" {
return f, &Error{file, ErrDot}
}
return f, nil
Expand Down

0 comments on commit 47eb687

Please sign in to comment.