net/http: prevent redirect loop in serveFile if "/" is a normal file

When FileServer(Dir("file")) is used where "file" is a normal file and
not a directory, the server enters a redirect loop.

The usage of a file inplace of a directory path is not documented in
http.Dir and it could be considered undefined behavior.

This CL updates serveFile to check if we are trying to traverse a normal
file instead of a directory and return an error, preventing the redirect
loop.

Fixes #63769

Change-Id: I81e289444e7d0bd72189c2e7b763f5540333e2d0
GitHub-Last-Rev: 754c9a1167
GitHub-Pull-Request: golang/go#63860
Reviewed-on: https://go-review.googlesource.com/c/go/+/538719
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Commit-Queue: Bryan Mills <bcmills@google.com>

src/net/http/fs.go

@@ -28,7 +28,7 @@ import (
 // specific directory tree.
 //
 // While the [FileSystem.Open] method takes '/'-separated paths, a Dir's string
-// value is a filename on the native file system, not a URL, so it is separated
+// value is a directory path on the native file system, not a URL, so it is separated
 // by [filepath.Separator], which isn't necessarily '/'.
 //
 // Note that Dir could expose sensitive files and directories. Dir will follow
@@ -665,11 +665,16 @@ func serveFile(w ResponseWriter, r *Request, fs FileSystem, name string, redirec
 				localRedirect(w, r, path.Base(url)+"/")
 				return
 			}
-		} else {
-			if url[len(url)-1] == '/' {
-				localRedirect(w, r, "../"+path.Base(url))
+		} else if url[len(url)-1] == '/' {
+			base := path.Base(url)
+			if base == "/" || base == "." {
+				// The FileSystem maps a path like "/" or "/./" to a file instead of a directory.
+				msg := "http: attempting to traverse a non-directory"
+				Error(w, msg, StatusInternalServerError)
 				return
 			}
+			localRedirect(w, r, "../"+base)
+			return
 		}
 	}
 

src/net/http/fs_test.go

@@ -1668,3 +1668,29 @@ func (grw gzipResponseWriter) Flush() {
 		fw.Flush()
 	}
 }
+
+// Issue 63769
+func TestFileServerDirWithRootFile(t *testing.T) { run(t, testFileServerDirWithRootFile) }
+func testFileServerDirWithRootFile(t *testing.T, mode testMode) {
+	testDirFile := func(t *testing.T, h Handler) {
+		ts := newClientServerTest(t, mode, h).ts
+		defer ts.Close()
+
+		res, err := ts.Client().Get(ts.URL)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if g, w := res.StatusCode, StatusInternalServerError; g != w {
+			t.Errorf("StatusCode mismatch: got %d, want: %d", g, w)
+		}
+		res.Body.Close()
+	}
+
+	t.Run("FileServer", func(t *testing.T) {
+		testDirFile(t, FileServer(Dir("testdata/index.html")))
+	})
+
+	t.Run("FileServerFS", func(t *testing.T) {
+		testDirFile(t, FileServerFS(os.DirFS("testdata/index.html")))
+	})
+}