Skip to content

Commit

Permalink
Fix normalization check for Rlocation path
Browse files Browse the repository at this point in the history
The check incorrectly discarded paths such as `foo/..bar`.

Also improve error messages.
  • Loading branch information
fmeum committed Dec 4, 2022
1 parent 2238018 commit 30497cf
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
19 changes: 13 additions & 6 deletions go/runfiles/runfiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ func (r *Runfiles) Rlocation(path string) (string, error) {
if path == "" {
return "", errors.New("runfiles: path may not be empty")
}
if !isNormalizedPath(path) {
return "", fmt.Errorf("runfiles: path %q is not normalized", path)
if err := isNormalizedPath(path); err != nil {
return "", err
}

// See https://github.com/bazelbuild/bazel/commit/b961b0ad6cc2578b98d0a307581e23e73392ad02
Expand All @@ -137,10 +137,17 @@ func (r *Runfiles) Rlocation(path string) (string, error) {
return p, nil
}

func isNormalizedPath(s string) bool {
return !strings.HasPrefix(s, "../") && !strings.Contains(s, "/..") &&
!strings.HasPrefix(s, "./") && !strings.HasSuffix(s, "/.") &&
!strings.Contains(s, "/./") && !strings.Contains(s, "//")
func isNormalizedPath(s string) error {
if strings.HasPrefix(s, "../") || strings.Contains(s, "/../") || strings.HasSuffix(s, "/..") {
return fmt.Errorf(`runfiles: path %q must not contain ".." segments`, s)
}
if strings.HasPrefix(s, "./") || strings.Contains(s, "/./") || strings.HasSuffix(s, "/.") {
return fmt.Errorf(`runfiles: path %q must not contain "." segments`, s)
}
if strings.Contains(s, "//") {
return fmt.Errorf(`runfiles: path %q must not contain "//"`, s)
}
return nil
}

// Env returns additional environmental variables to pass to subprocesses.
Expand Down
7 changes: 7 additions & 0 deletions tests/runfiles/runfiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ func TestPath_errors(t *testing.T) {
}
})
}
for _, s := range []string{"foo/..bar", "foo/.bar"} {
t.Run(s, func(t *testing.T) {
if _, err := r.Rlocation(s); err != nil && !os.IsNotExist(err.(*runfiles.Error).Err) {
t.Errorf("got %q, want none or 'file not found' error", err)
}
})
}
}

func TestRunfiles_zero(t *testing.T) {
Expand Down

0 comments on commit 30497cf

Please sign in to comment.