net/http: improve handling of errors in Dir.Open

The current implementation fails to produce an "IsNotExist" error on some
platforms (unix) for certain situations where it would be expected. This causes
downstream consumers, like FileServer, to emit 500 errors instead of a 404 for
some non-existant paths on certain platforms but not others.

As an example, os.Open("/index.html/foo") on a unix-type system will return
syscall.ENOTDIR, which os.IsNotExist cannot return true for (because the
error code is ambiguous without context). On windows, this same example
would result in os.IsNotExist returning true -- since the returned error is
specific.

This change alters Dir.Open to look up the tree for an "IsPermission" or
"IsNotExist" error to return, or a non-directory, returning os.ErrNotExist in
the last case. For all other error scenarios, the original error is returned.
This ensures that downstream code, like FileServer, receive errors that behave
the same across all platforms.

Fixes #18984

Change-Id: Id7d16591c24cd96afddb6d8ae135ac78da42ed37
Reviewed-on: https://go-review.googlesource.com/36635
Reviewed-by: Yasuhiro MATSUMOTO <mattn.jp@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/fs.go b/src/net/http/fs.go
index 51b6b1d32f4e1daf73469639fd8bf2d482acbd26..773e74d536d68d2f7da47fad4394ae24b540dd49 100644 (file)
--- a/src/net/http/fs.go
+++ b/src/net/http/fs.go
@@ -33,6 +33,27 @@ import (
 // An empty Dir is treated as ".".
 type Dir string
 
+// mapDirOpenError 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 os.ErrNotExist. See Issue 18984.
+func mapDirOpenError(originalErr error, name string) error {
+       if os.IsNotExist(originalErr) || os.IsPermission(originalErr) {
+               return originalErr
+       }
+
+       parts := strings.Split(name, string(filepath.Separator))
+       for i := range parts {
+               fi, err := os.Stat(strings.Join(parts[:i+1], string(filepath.Separator)))
+               if err != nil {
+                       return originalErr
+               }
+               if !fi.IsDir() {
+                       return os.ErrNotExist
+               }
+       }
+       return originalErr
+}
+
 func (d Dir) Open(name string) (File, error) {
        if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) {
                return nil, errors.New("http: invalid character in file path")
@@ -41,9 +62,10 @@ func (d Dir) Open(name string) (File, error) {
        if dir == "" {
                dir = "."
        }
-       f, err := os.Open(filepath.Join(dir, filepath.FromSlash(path.Clean("/"+name))))
+       fullName := filepath.Join(dir, filepath.FromSlash(path.Clean("/"+name)))
+       f, err := os.Open(fullName)
        if err != nil {
-               return nil, err
+               return nil, mapDirOpenError(err, fullName)
        }
        return f, nil
 }

diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go
index 17a0e4a9af85668c003f92d15923dfd82f723ca2..8ff2faf9b90f081a53a0de4b36d8870eb3ccf54b 100644 (file)
--- a/src/net/http/fs_test.go
+++ b/src/net/http/fs_test.go
@@ -1161,6 +1161,39 @@ func TestLinuxSendfileChild(*testing.T) {
        }
 }
 
+// Issue 18984: 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")))
+       defer ts.Close()
+
+       res, err := Get(ts.URL + "/index.html/not-a-file")
+       if err != nil {
+               t.Fatal(err)
+       }
+       res.Body.Close()
+       if res.StatusCode != 404 {
+               t.Errorf("StatusCode = %v; want 404", res.StatusCode)
+       }
+
+       dir := Dir("testdata")
+       _, err = dir.Open("/index.html/not-a-file")
+       if err == nil {
+               t.Fatal("err == nil; want != nil")
+       }
+       if !os.IsNotExist(err) {
+               t.Errorf("err = %v; os.IsNotExist(err) = %v; want true", err, os.IsNotExist(err))
+       }
+
+       _, err = dir.Open("/index.html/not-a-dir/not-a-file")
+       if err == nil {
+               t.Fatal("err == nil; want != nil")
+       }
+       if !os.IsNotExist(err) {
+               t.Errorf("err = %v; os.IsNotExist(err) = %v; want true", err, os.IsNotExist(err))
+       }
+}
+
 func TestFileServerCleanPath(t *testing.T) {
        tests := []struct {
                path     string