net/http: map FS Open errors just like Dir

When an http.FileServer is given a path like file1/file2 where file1
exists but file2 does not, the proper HTTP status should be
NotFound. Some OSes return a "not a directory" error instead, so this
must be mapped to NotFound.

That mapping was already being done for the Dir FileSystem
implementation, as discussed in #18984. But it wasn't for the
FS implementation.

This CL does the same mapping for FS, by generalizing the function
that did it for Dir.

Fixes #49552

Change-Id: I61d6aa8ef101158e9674707d44e653f5dedbd040
Reviewed-on: https://go-review.googlesource.com/c/go/+/376874
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/net/http/fs.go b/src/net/http/fs.go
index 19b2894bf2ff388cbc0de14c38e26c4b342d2b4c..6caee9ed931e871337a56687c380bd8dc700c415 100644 (file)
--- a/src/net/http/fs.go
+++ b/src/net/http/fs.go
@@ -42,20 +42,20 @@ import (
 // An empty Dir is treated as ".".
 type Dir string
 
-// mapDirOpenError maps the provided non-nil error from opening name
+// mapOpenError maps the provided non-nil error from opening name
 // to a possibly better non-nil error. In particular, it turns OS-specific errors
-// about opening files in non-directories into fs.ErrNotExist. See Issue 18984.
-func mapDirOpenError(originalErr error, name string) error {
+// about opening files in non-directories into fs.ErrNotExist. See Issues 18984 and 49552.
+func mapOpenError(originalErr error, name string, sep rune, stat func(string) (fs.FileInfo, error)) error {
        if errors.Is(originalErr, fs.ErrNotExist) || errors.Is(originalErr, fs.ErrPermission) {
                return originalErr
        }
 
-       parts := strings.Split(name, string(filepath.Separator))
+       parts := strings.Split(name, string(sep))
        for i := range parts {
                if parts[i] == "" {
                        continue
                }
-               fi, err := os.Stat(strings.Join(parts[:i+1], string(filepath.Separator)))
+               fi, err := stat(strings.Join(parts[:i+1], string(sep)))
                if err != nil {
                        return originalErr
                }
@@ -79,7 +79,7 @@ func (d Dir) Open(name string) (File, error) {
        fullName := filepath.Join(dir, filepath.FromSlash(path.Clean("/"+name)))
        f, err := os.Open(fullName)
        if err != nil {
-               return nil, mapDirOpenError(err, fullName)
+               return nil, mapOpenError(err, fullName, filepath.Separator, os.Stat)
        }
        return f, nil
 }
@@ -759,7 +759,9 @@ func (f ioFS) Open(name string) (File, error) {
        }
        file, err := f.fsys.Open(name)
        if err != nil {
-               return nil, err
+               return nil, mapOpenError(err, name, '/', func(path string) (fs.FileInfo, error) {
+                       return fs.Stat(f.fsys, path)
+               })
        }
        return ioFile{file}, nil
 }

diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go
index 4b01cce72dbf413b93b5c6839ded32e5e47a774a..d627dfd4be965287cb1d929ba8dc5621c02525cc 100644 (file)
--- a/src/net/http/fs_test.go
+++ b/src/net/http/fs_test.go
@@ -1244,10 +1244,19 @@ func TestLinuxSendfileChild(*testing.T) {
        }
 }
 
-// Issue 18984: tests that requests for paths beyond files return not-found errors
+// Issues 18984, 49552: tests that requests for paths beyond files return not-found errors
 func TestFileServerNotDirError(t *testing.T) {
        defer afterTest(t)
-       ts := httptest.NewServer(FileServer(Dir("testdata")))
+       t.Run("Dir", func(t *testing.T) {
+               testFileServerNotDirError(t, func(path string) FileSystem { return Dir(path) })
+       })
+       t.Run("FS", func(t *testing.T) {
+               testFileServerNotDirError(t, func(path string) FileSystem { return FS(os.DirFS(path)) })
+       })
+}
+
+func testFileServerNotDirError(t *testing.T, newfs func(string) FileSystem) {
+       ts := httptest.NewServer(FileServer(newfs("testdata")))
        defer ts.Close()
 
        res, err := Get(ts.URL + "/index.html/not-a-file")
@@ -1259,9 +1268,9 @@ func TestFileServerNotDirError(t *testing.T) {
                t.Errorf("StatusCode = %v; want 404", res.StatusCode)
        }
 
-       test := func(name string, dir Dir) {
+       test := func(name string, fsys FileSystem) {
                t.Run(name, func(t *testing.T) {
-                       _, err = dir.Open("/index.html/not-a-file")
+                       _, err = fsys.Open("/index.html/not-a-file")
                        if err == nil {
                                t.Fatal("err == nil; want != nil")
                        }
@@ -1270,7 +1279,7 @@ func TestFileServerNotDirError(t *testing.T) {
                                        errors.Is(err, fs.ErrNotExist))
                        }
 
-                       _, err = dir.Open("/index.html/not-a-dir/not-a-file")
+                       _, err = fsys.Open("/index.html/not-a-dir/not-a-file")
                        if err == nil {
                                t.Fatal("err == nil; want != nil")
                        }
@@ -1286,8 +1295,8 @@ func TestFileServerNotDirError(t *testing.T) {
                t.Fatal("get abs path:", err)
        }
 
-       test("RelativePath", Dir("testdata"))
-       test("AbsolutePath", Dir(absPath))
+       test("RelativePath", newfs("testdata"))
+       test("AbsolutePath", newfs(absPath))
 }
 
 func TestFileServerCleanPath(t *testing.T) {