go: net/http: FileServer no longer serves content for POST
What version of Go are you using (go version)?
$ go version 1.20.2 darwin/amd64
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (go env)?
go env Output
$ go env GO111MODULE="" GOARCH="amd64" GOBIN="" GOCACHE="/Users/beaubrueggemann/Library/Caches/go-build" GOENV="/Users/beaubrueggemann/Library/Application Support/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/beaubrueggemann/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/beaubrueggemann/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/go" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64" GOVCS="" GOVERSION="go1.19.4" GCCGO="gccgo" GOAMD64="v1" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/dev/null" GOWORK="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/47/c0lsmsxn3_z86w657l519d6m0000gn/T/go-build1135191649=/tmp/go-build -gno-record-gcc-switches -fno-common"
What did you do?
I updated Go from version 1.19.4 to version 1.20.2 and ran my server.
Here’s an abbreviated version of the source, that shows the problem:
package main
import (
"log"
"net/http"
)
const (
PostPath = "/postpage.html"
PostName = "name"
)
var FileServer = http.FileServer(http.Dir("/site"))
func MyHandler(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == PostPath {
// Handle post data.
value := r.PostFormValue(PostName)
// Server does something with value here...
log.Println(value)
}
// Serve page from filesystem
FileServer.ServeHTTP(w, r)
}
func main() {
log.Fatal(http.ListenAndServe(":8080", http.HandlerFunc(MyHandler)))
}
Then I tried a POST request to the “localhost:8080/postpage.html” URL.
What did you expect to see?
I expected to receive a status 200, served with a body matching the file on my filesystem at /site/postpage.html
What did you see instead?
I received a 405 status (method not allowed), with a body consisting of “read-only”
I dug into the problem, and it results from commit 413554.
This change breaks the compatibility promise. I was surprised there was no discussion of compatibility in the commit review or the corresponding issue (53501).
It is useful to be able to wrap an http.FileServer with an http.Handler that handles POST data, etc., then forwards to http.FileServer to serve a static response body. But now this doesn’t work anymore.
About this issue
- Original URL
- State: closed
- Created a year ago
- Reactions: 1
- Comments: 15 (10 by maintainers)
Commits related to this issue
- Revert "net/http: FileServer method check + minimal OPTIONS implementation" This reverts https://go.dev/cl/413554 Reason for revert: Backwards-incompatible change in behavior. For #53501 For #59375... — committed to golang/go by neild a year ago
- [release-branch.go1.20] Revert "net/http: FileServer method check + minimal OPTIONS implementation" This reverts https://go.dev/cl/413554 Reason for revert: Backwards-incompatible change in behavior... — committed to golang/go by neild a year ago
- [release-branch.go1.20] Revert "net/http: FileServer method check + minimal OPTIONS implementation" This reverts https://go.dev/cl/413554 Reason for revert: Backwards-incompatible change in behavior... — committed to tailscale/go by neild a year ago
- [release-branch.go1.20] Revert "net/http: FileServer method check + minimal OPTIONS implementation" This reverts https://go.dev/cl/413554 Reason for revert: Backwards-incompatible change in behavior... — committed to tailscale/go by neild a year ago
The CL was adding support for OPTIONS, so it had to do something with methods generally.
There was no discussion of compatibility because I don’t think any of us realized there was a compatibility problem. That is, I don’t think we realized people would actually expect ‘DELETE /foo’ to serve the content of foo, nor that code would be sending DELETE /foo to get the content. For DELETE I still think it’s probably more correct on balance to reject.
POST is trickier: the verb POST by itself really doesn’t imply mutation the way DELETE does. POST is in a grey area between GET and DELETE, on the fence if you will pardon the pun.
I think it would be reasonable for compatibility to restore support for POST in the next point release, although perhaps others disagree.
@pascaldekloe I don’t think that’s entirely fair, and thank you for filing #59470, which I’ve turned into a proposal. The original CL did not take compatibility into account, so it is entirely fair to roll it back in Go 1.20 and revisit with more consideration for Go 1.21.
@beaubrueggemann You may wish to follow or contribute to #59470 as well.
The documentation for type http.Handler says not to modify the request:
Also, really the point is that compatibility was broken. Go has a compatibility promise—is this still taken seriously? This promise was a major factor in my choosing to work with Go.
For many years http.FileServer has ignored the HTTP method (POST, DELETE, etc.), and just served files regardless. Suddenly this behavior has changed, and it serves errors for some methods.
This broke my server and wasted some of my time. I’m bothered that the change was made with absolutely no discussion about the compatibility breaking. I thought the compatibility promise would prevent that.
While you could argue that the new method-filtering behavior is a better design for http.FileServer, I think you could also make a good argument for the original design. But it shouldn’t matter—the ship sailed long ago, and the original design was chosen.
The “compatibility promise” is about documented behaviour @beaubrueggemann. Otherwise you’d never be able to fix a bug, as any correction breaks the “original behaviour”.
Protecting creative use can not outweight all websites broken without the method check in place.
Static content can only be served by read operations. When POST-ing a body, then the status code must match the processing of such request body. For example, an HTTP 404 means that the request was not executed, while the hack in this issue will execute, regardless of the response.
Change https://go.dev/cl/482635 mentions this issue:
Revert "net/http: FileServer method check + minimal OPTIONS implementation"If we were designing FileServer today, I think we might want it to only respond to GET requests. But I agree that this ship has sailed, and we should restore the original behavior.