net/http: http.FileServer goes into infinite redirect loop when root is a file

[meblum]

What version of Go are you using (go version)?

go1.21.3 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\MeirBloomenfeld\AppData\Local\go-build
set GOENV=C:\Users\MeirBloomenfeld\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\MeirBloomenfeld\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\MeirBloomenfeld\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.21.3
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=C:\Users\MeirBloomenfeld\Downloads\test\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\MEIRBL~1\AppData\Local\Temp\go-build4162283884=/tmp/go-build -gno-record-gcc-switches

What did you do?

func main() {
	// myFile is a file, not a directory
	http.ListenAndServe(":8080", http.FileServer(http.Dir("myFile")))
}

What did you expect to see?

a request to http://localhost:8080 returns an error response, or the content of "myFile"

What did you see instead?

server responds with 301 status code to redirect to "..//" which causes an infinite redirect loop.

[seankhliao]

cc @neild

[meblum]

The problem is caused by the fact that the FileServer implementation will redirect /a/ to /a when a is a not a directory, but does not handle the case where the request is to / and the root is a file.

This should be trivial to fix, I would be more than happy to submit a pull request for this issue.

[meblum]

Note, the current implementation of ServeFileFS will return the file when using it as root.

[mauri870]

The http.FileSystem interface predates fs.FS and might not be 100% compatible in some cases, that is why ServeFileFS works as expected, been a newer api.

Having this routine enter an infinite redirect loop is not cool, and appears to be a bug that should be addressed properly.

[mauri870]

It is unclear to me what should happen if http.Dir is invoked with a normal file as argument, and I could not find any tests or documentation around it to back it up. All the occurrences of http.Dir in the codebase use a directory as parameter.

Is that right to assume it is undefined behavior calling http.FileServer(http.Dir("myFile"))?

I'll send a CL shortly that allows a normal file to be served as well as fixing the redirect loop, but I'm not sure if that is something it should support, or if we should error out at some point saying that we need a real directory.

Either way I think we should fix the redirect loop since it affects 1.20, 1.21 and tip.

[gopherbot]

Change https://go.dev/cl/538719 mentions this issue: net/http: prevent redirect loop in serveFile if "/" is a normal file

[meblum]

The reason why ServeFileFS doesn't have this redirect loop issue, is because like ServeFile, it doesn't redirect requests at all (index.html being the exception), it just returns the content found at path. I don't think it was a deliberate api design decision not to error when using a file instead of a directory.

For FileServer, it might make sense to return an error instead of the file, since we expect a file (other than index.html) to be served only from a request not ending with a slash.
In other words, I believe this will be the only case of a file other than index.html being served for a path ending in a slash.

Does it matter? Not sure.

[mauri870]

@neild @bcmills I'm not quite sure what should be the path forward to fix this but I would like to merge the fix before the freeze. I believe we have two choices:

1. We make Dir handle files and document that it can do that, and end up with a constructor like Dir("file.txt") which looks quite odd but it works and fixes the redirect loop.
2. We call Stat inside (Dir).Open and fail if the provided file is not a directory, and we document that as well. That should fix the redirect loop too.

I'm inclined to go with option 2, what do you guys think?

[bcmills]

I prefer option (2) from the perspective of doing what it says on the tin, but I will defer to Damien if he has a preference.

[odeke-em]

Thank you @mauri870 for the CL, I've commented on your CL per https://go-review.googlesource.com/c/go/+/538719/comments/dfdfd7d9_f04c9321 about strictly ensuring that we only perform point 2 and that we preserve the old behavior as is, following through with https://www.hyrumslaw.com/ and only stat-ing to ensure that we have purely directories.

[mauri870]

@odeke-em Thanks for the feedback, I addressed your comments in the CL.