code review 7305053: os/exec: LookPath on Unix shouldn't look in cwd when PA...

os/exec: LookPath on Unix shouldn't look in cwd when PATH is empty

[speter, 2013-02-07 12:33:24]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[dave_cheney.net, 2013-02-07 12:35:27]

Wow, nice catch.
On 7 Feb 2013 23:33, <speter.go1@gmail.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> os/exec: LookPath on Unix shouldn't look in cwd when PATH is empty
>
> Please review this at
https://codereview.appspot.**com/7305053/<https://codereview.appspot.com/7305...
>
> Affected files:
>   M src/pkg/os/exec/lp_unix.go
>   A src/pkg/os/exec/lp_unix_test.**go
>
>
> Index: src/pkg/os/exec/lp_unix.go
> ==============================**==============================**=======
> --- a/src/pkg/os/exec/lp_unix.go
> +++ b/src/pkg/os/exec/lp_unix.go
> @@ -42,6 +42,9 @@
>                 return "", &Error{file, err}
>         }
>         pathenv := os.Getenv("PATH")
> +       if pathenv == "" {
> +               return "", &Error{file, ErrNotFound}
> +       }
>         for _, dir := range strings.Split(pathenv, ":") {
>                 if dir == "" {
>                         // Unix shell semantics: path element "" means "."
> Index: src/pkg/os/exec/lp_unix_test.**go
> ==============================**==============================**=======
> new file mode 100644
> --- /dev/null
> +++ b/src/pkg/os/exec/lp_unix_**test.go
> @@ -0,0 +1,52 @@
> +// Copyright 2013 The Go Authors.  All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +// +build darwin freebsd linux netbsd openbsd
> +
> +package exec
> +
> +import (
> +       "io/ioutil"
> +       "os"
> +       "testing"
> +)
> +
> +func TestLookPathUnixEmptyPath(t *testing.T) {
> +       tmp, err := ioutil.TempDir("", "TestLookPathUnixEmptyPath")
> +       if err != nil {
> +               t.Fatal("TempDir failed: ", err)
> +       }
> +       defer os.RemoveAll(tmp)
> +       wd, err := os.Getwd()
> +       if err != nil {
> +               t.Fatal("Getwd failed: ", err)
> +       }
> +       err = os.Chdir(tmp)
> +       if err != nil {
> +               t.Fatal("Chdir failed: ", err)
> +       }
> +       defer os.Chdir(wd)
> +
> +       f, err := os.OpenFile("exec_me", os.O_CREATE|os.O_EXCL, 0700)
> +       if err != nil {
> +               t.Fatal("OpenFile failed: ", err)
> +       }
> +       defer f.Close()
> +
> +       pathenv := os.Getenv("PATH")
> +       defer os.Setenv("PATH", pathenv)
> +
> +       err = os.Setenv("PATH", "")
> +       if err != nil {
> +               t.Fatal("Setenv failed: ", err)
> +       }
> +
> +       path, err := LookPath("exec_me")
> +       if err == nil {
> +               t.Fatal("LookPath found exec_me in empty $PATH")
> +       }
> +       if path != "" {
> +               t.Fatalf("LookPath path == %q when err != nil", path)
> +       }
> +}
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[bradfitz, 2013-02-07 14:36:57]

LGTM

On Thu, Feb 7, 2013 at 4:33 AM, <speter.go1@gmail.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> os/exec: LookPath on Unix shouldn't look in cwd when PATH is empty
>
> Please review this at
https://codereview.appspot.**com/7305053/<https://codereview.appspot.com/7305...
>
> Affected files:
>   M src/pkg/os/exec/lp_unix.go
>   A src/pkg/os/exec/lp_unix_test.**go
>
>
> Index: src/pkg/os/exec/lp_unix.go
> ==============================**==============================**=======
> --- a/src/pkg/os/exec/lp_unix.go
> +++ b/src/pkg/os/exec/lp_unix.go
> @@ -42,6 +42,9 @@
>                 return "", &Error{file, err}
>         }
>         pathenv := os.Getenv("PATH")
> +       if pathenv == "" {
> +               return "", &Error{file, ErrNotFound}
> +       }
>         for _, dir := range strings.Split(pathenv, ":") {
>                 if dir == "" {
>                         // Unix shell semantics: path element "" means "."
> Index: src/pkg/os/exec/lp_unix_test.**go
> ==============================**==============================**=======
> new file mode 100644
> --- /dev/null
> +++ b/src/pkg/os/exec/lp_unix_**test.go
> @@ -0,0 +1,52 @@
> +// Copyright 2013 The Go Authors.  All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +// +build darwin freebsd linux netbsd openbsd
> +
> +package exec
> +
> +import (
> +       "io/ioutil"
> +       "os"
> +       "testing"
> +)
> +
> +func TestLookPathUnixEmptyPath(t *testing.T) {
> +       tmp, err := ioutil.TempDir("", "TestLookPathUnixEmptyPath")
> +       if err != nil {
> +               t.Fatal("TempDir failed: ", err)
> +       }
> +       defer os.RemoveAll(tmp)
> +       wd, err := os.Getwd()
> +       if err != nil {
> +               t.Fatal("Getwd failed: ", err)
> +       }
> +       err = os.Chdir(tmp)
> +       if err != nil {
> +               t.Fatal("Chdir failed: ", err)
> +       }
> +       defer os.Chdir(wd)
> +
> +       f, err := os.OpenFile("exec_me", os.O_CREATE|os.O_EXCL, 0700)
> +       if err != nil {
> +               t.Fatal("OpenFile failed: ", err)
> +       }
> +       defer f.Close()
> +
> +       pathenv := os.Getenv("PATH")
> +       defer os.Setenv("PATH", pathenv)
> +
> +       err = os.Setenv("PATH", "")
> +       if err != nil {
> +               t.Fatal("Setenv failed: ", err)
> +       }
> +
> +       path, err := LookPath("exec_me")
> +       if err == nil {
> +               t.Fatal("LookPath found exec_me in empty $PATH")
> +       }
> +       if path != "" {
> +               t.Fatalf("LookPath path == %q when err != nil", path)
> +       }
> +}
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[bradfitz, 2013-02-07 14:41:40]

*** Submitted as https://code.google.com/p/go/source/detail?r=f3ab80f38f60 ***

os/exec: LookPath on Unix shouldn't look in cwd when PATH is empty

R=golang-dev, dave, bradfitz
CC=golang-dev
https://codereview.appspot.com/7305053

Committer: Brad Fitzpatrick <bradfitz@golang.org>