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/36635Reviewed-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>

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
 }

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