From 296d0812b091d9cb1847381cb88ed8936d7c35ec Mon Sep 17 00:00:00 2001 From: Mauri de Souza Meneguzzo Date: Wed, 10 Jan 2024 13:33:40 +0000 Subject: [PATCH] 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: 754c9a1167916b5a8c3c827391d7e4a2ff3bc44d GitHub-Pull-Request: golang/go#63860 Reviewed-on: https://go-review.googlesource.com/c/go/+/538719 Reviewed-by: Damien Neil Reviewed-by: Emmanuel Odeke LUCI-TryBot-Result: Go LUCI Reviewed-by: Bryan Mills Auto-Submit: Bryan Mills Commit-Queue: Bryan Mills --- src/net/http/fs.go | 13 +++++++++---- src/net/http/fs_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/net/http/fs.go b/src/net/http/fs.go index 977c3a766e..45cf16eed1 100644 --- a/src/net/http/fs.go +++ b/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 } } diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go index 383d27df9b..70a4b8982f 100644 --- a/src/net/http/fs_test.go +++ b/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"))) + }) +} -- 2.48.1