golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.98k stars 17.67k forks source link

x/net/webdav: `WriteHeader` may be called twice when webdav processes methods such as `PROPFIND` #66085

Open y805939188 opened 8 months ago

y805939188 commented 8 months ago

Go version

go version go1.21.6 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/myself/Library/Caches/go-build'
GOENV='/Users/myself/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/myself/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/myself/go'
GOPRIVATE=''
GOPROXY='https://goproxy.cn,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.6'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/myself/Desktop/webdav/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/7k/jrqj5sv161n2sy70g9st76_40000gn/T/go-build1299288959=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I tried to modify http request method from HEAD to PROPFIND so that I can mock the PROPFIND request. As follows:

func ServeHTTP(w http.ResponseWriter, r *http.Request) {
    if r.Method == "HEAD" {
        r.Method = "PROPFIND"
        if r.Header.Get("Depth") == "" {
            r.Header.Add("Depth", "0")
        }
    }
    handler := &webdav.Handler{
        FileSystem:  myFileSystem,
                 LockSystem: webdav.NewMemLS(),
    }
    handler.ServeHTTP(w, r)
}

I realize that it is not a right approach after testing. But I got a confusing error message http: superfluous response.WriteHeader call from github.com/hacdias/webdav/v4/lib.responseWriterNoBody.WriteHeader (webdav.go:222):

image

And my webdav client received a 207 response:

image

What did you see happen?

I got a confusing error message http: superfluous response.WriteHeader call from github.com/hacdias/webdav/v4/lib.responseWriterNoBody.WriteHeader (webdav.go:222) and 207 response. I tried to debug the code, I found that I would get an short write error in this handlePropfind function

image



But before triggering this error, the status code of 207 had already been written into the response header:

image



So in the outermost function used to handle PROPFIND or PROPPATCH methods, when a status with err was returned, the WriteHeader was called again, resulting in the client receive a 207 response and the server side get an superfluous response.WriteHeader call error.

image

What did you expect to see?

I have given up using head to simulate propfind, but I hope that when processing the PROPFIND or PROPPATCH methods, if the server side encounters an error, it can return 500 instead of 207

mknyszek commented 8 months ago

@y805939188 In the future, please post text instead of images. The images are less accessible and more difficult to play around with (copy and paste somewhere else, for example).

Also, I'm just triaging and not an expert here, but it seems like this might be a bug in github.com/hacdias/webdav? What makes you think this is a bug in the Go standard library? Or is it the error message that's the problem?

Thanks.

CC @neild

y805939188 commented 8 months ago

@y805939188 In the future, please post text instead of images. The images are less accessible and more difficult to play around with (copy and paste somewhere else, for example).

Also, I'm just triaging and not an expert here, but it seems like this might be a bug in github.com/hacdias/webdav? What makes you think this is a bug in the Go standard library? Or is it the error message that's the problem?

Thanks.

CC @neild

Hello! I will try to use detailed text and images to describe the problem. I think it may be an issue in Go's net/webdav package. There is a function named ServeHTTP in net/webdav/webdav.go that handles the methods PROPFIND and PROPPATCH with the functions status, err = h.handlePropfind(w, r) and status, err = h.handleProppatch(w, r): image

There is a code path of ServerHTTP -> handlePropfind -> walkFn -> mw.write, the function mw.write calls the w.writeHeader to write the 207 response code and returns w.enc.Encode(r) in x/net/webdav/xml.go, the w.enc.Encoder(r) function returns an error type: img_v3_028o_976429b3-e059-48c0-8d34-e657947d238g And the error from w.enc.Encoder(r) will be returned to the ServerHTTP function from h.handlePropfind(w, r) But in the ServeHTTP function, if an error type is returned from handlePropfind, ServeHTTP will call WriteHeader to write a 500 code into response header: img_v3_028o_76efcf28-626b-405c-b148-6e2d0cf77d4g But in the mw.write function, the HTTP status code 207 has been written into the response header. As a result, the WebDAV client will receive a response with the 207 status code, even though an error related to a superfluous response.WriteHeader call occurred on the server side. In summary, when the x/net/webdav/webdav.go file is handling PROPFIND or PROPPATCH methods, if an error occurs on the server side, w.WriteHeader may be triggered twice.