Skip to content

Commit 44cf595

Browse files
committed
path/filepath: return special error from EvalSymlinks
CL 155597 attempted to fix #29372. But it failed to make all new test cases pass. Also CL 155597 broke some existing code (see #29449 for details). Make small adjustment to CL 155597 that fixes both #29372 and #29449. Suggested by Ian. Updates #29372 Fixes #29449 Change-Id: I9777a615514d3f152af5acb65fb1239e696607b6 Reviewed-on: https://go-review.googlesource.com/c/156398 Run-TryBot: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
1 parent 94d9a20 commit 44cf595

File tree

5 files changed

+54
-41
lines changed

5 files changed

+54
-41
lines changed

src/path/filepath/path_test.go

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,27 +1382,18 @@ func TestIssue29372(t *testing.T) {
13821382
path := f.Name()
13831383
defer os.Remove(path)
13841384

1385-
isWin := runtime.GOOS == "windows"
13861385
pathSeparator := string(filepath.Separator)
1387-
tests := []struct {
1388-
path string
1389-
skip bool
1390-
}{
1391-
{path + strings.Repeat(pathSeparator, 1), false},
1392-
{path + strings.Repeat(pathSeparator, 2), false},
1393-
{path + strings.Repeat(pathSeparator, 1) + ".", false},
1394-
{path + strings.Repeat(pathSeparator, 2) + ".", false},
1395-
// windows.GetFinalPathNameByHandle return the directory part with trailing dot dot
1396-
// C:\path\to\existing_dir\existing_file\.. returns C:\path\to\existing_dir
1397-
{path + strings.Repeat(pathSeparator, 1) + "..", isWin},
1398-
{path + strings.Repeat(pathSeparator, 2) + "..", isWin},
1386+
tests := []string{
1387+
path + strings.Repeat(pathSeparator, 1),
1388+
path + strings.Repeat(pathSeparator, 2),
1389+
path + strings.Repeat(pathSeparator, 1) + ".",
1390+
path + strings.Repeat(pathSeparator, 2) + ".",
1391+
path + strings.Repeat(pathSeparator, 1) + "..",
1392+
path + strings.Repeat(pathSeparator, 2) + "..",
13991393
}
14001394

14011395
for i, test := range tests {
1402-
if test.skip {
1403-
continue
1404-
}
1405-
_, err = filepath.EvalSymlinks(test.path)
1396+
_, err = filepath.EvalSymlinks(test)
14061397
if err != syscall.ENOTDIR {
14071398
t.Fatalf("test#%d: want %q, got %q", i, syscall.ENOTDIR, err)
14081399
}

src/path/filepath/path_windows_test.go

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -536,17 +536,39 @@ func TestNTNamespaceSymlink(t *testing.T) {
536536
}
537537
target := strings.Trim(string(output), " \n\r")
538538

539-
link := filepath.Join(tmpdir, "link")
540-
output, err = exec.Command("cmd", "/c", "mklink", "/J", link, target).CombinedOutput()
539+
dirlink := filepath.Join(tmpdir, "dirlink")
540+
output, err = exec.Command("cmd", "/c", "mklink", "/J", dirlink, target).CombinedOutput()
541541
if err != nil {
542-
t.Fatalf("failed to run mklink %v %v: %v %q", link, target, err, output)
542+
t.Fatalf("failed to run mklink %v %v: %v %q", dirlink, target, err, output)
543543
}
544544

545-
got, err := filepath.EvalSymlinks(link)
545+
got, err := filepath.EvalSymlinks(dirlink)
546546
if err != nil {
547547
t.Fatal(err)
548548
}
549549
if want := vol + `\`; got != want {
550-
t.Errorf(`EvalSymlinks(%q): got %q, want %q`, link, got, want)
550+
t.Errorf(`EvalSymlinks(%q): got %q, want %q`, dirlink, got, want)
551+
}
552+
553+
file := filepath.Join(tmpdir, "file")
554+
err = ioutil.WriteFile(file, []byte(""), 0666)
555+
if err != nil {
556+
t.Fatal(err)
557+
}
558+
559+
target += file[len(filepath.VolumeName(file)):]
560+
561+
filelink := filepath.Join(tmpdir, "filelink")
562+
output, err = exec.Command("cmd", "/c", "mklink", filelink, target).CombinedOutput()
563+
if err != nil {
564+
t.Fatalf("failed to run mklink %v %v: %v %q", filelink, target, err, output)
565+
}
566+
567+
got, err = filepath.EvalSymlinks(filelink)
568+
if err != nil {
569+
t.Fatal(err)
570+
}
571+
if want := file; got != want {
572+
t.Errorf(`EvalSymlinks(%q): got %q, want %q`, filelink, got, want)
551573
}
552574
}

src/path/filepath/symlink.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"errors"
99
"os"
1010
"runtime"
11-
"syscall"
1211
)
1312

1413
func walkSymlinks(path string) (string, error) {
@@ -79,7 +78,7 @@ func walkSymlinks(path string) (string, error) {
7978

8079
if fi.Mode()&os.ModeSymlink == 0 {
8180
if !fi.Mode().IsDir() && end < len(path) {
82-
return "", syscall.ENOTDIR
81+
return "", slashAfterFilePathError
8382
}
8483
continue
8584
}

src/path/filepath/symlink_unix.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@
22

33
package filepath
44

5+
import (
6+
"syscall"
7+
)
8+
9+
// walkSymlinks returns slashAfterFilePathError error for paths like
10+
// //path/to/existing_file/ and /path/to/existing_file/. and /path/to/existing_file/..
11+
12+
var slashAfterFilePathError = syscall.ENOTDIR
13+
514
func evalSymlinks(path string) (string, error) {
615
return walkSymlinks(path)
716
}

src/path/filepath/symlink_windows.go

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -159,18 +159,6 @@ func evalSymlinksUsingGetFinalPathNameByHandle(path string) (string, error) {
159159
return "", errors.New("GetFinalPathNameByHandle returned unexpected path=" + s)
160160
}
161161

162-
func symlinkOrDir(path string) (string, error) {
163-
fi, err := os.Lstat(path)
164-
if err != nil {
165-
return "", err
166-
}
167-
168-
if fi.Mode()&os.ModeSymlink == 0 && !fi.Mode().IsDir() {
169-
return "", syscall.ENOTDIR
170-
}
171-
return path, nil
172-
}
173-
174162
func samefile(path1, path2 string) bool {
175163
fi1, err := os.Lstat(path1)
176164
if err != nil {
@@ -183,16 +171,20 @@ func samefile(path1, path2 string) bool {
183171
return os.SameFile(fi1, fi2)
184172
}
185173

174+
// walkSymlinks returns slashAfterFilePathError error for paths like
175+
// //path/to/existing_file/ and /path/to/existing_file/. and /path/to/existing_file/..
176+
177+
var slashAfterFilePathError = errors.New("attempting to walk past file path.")
178+
186179
func evalSymlinks(path string) (string, error) {
187180
newpath, err := walkSymlinks(path)
181+
if err == slashAfterFilePathError {
182+
return "", syscall.ENOTDIR
183+
}
188184
if err != nil {
189185
newpath2, err2 := evalSymlinksUsingGetFinalPathNameByHandle(path)
190186
if err2 == nil {
191-
normPath, toNormErr := toNorm(newpath2, normBase)
192-
if toNormErr != nil {
193-
return "", toNormErr
194-
}
195-
return symlinkOrDir(normPath)
187+
return toNorm(newpath2, normBase)
196188
}
197189
return "", err
198190
}

0 commit comments

Comments
 (0)