Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
y805939188 opened this issue Mar 4, 2024 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@y805939188
Copy link

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 mknyszek changed the title When webdav processes methods such as PROPFIND, it is possible that WriteHeader may be called twice net/http: WriteHeader may be called twice when webdav processes methods such as PROPFIND Mar 4, 2024
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 4, 2024
@mknyszek mknyszek added this to the Backlog milestone Mar 4, 2024
@mknyszek
Copy link
Contributor

mknyszek commented Mar 4, 2024

@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

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 4, 2024
@seankhliao seankhliao changed the title net/http: WriteHeader may be called twice when webdav processes methods such as PROPFIND x/net/webdav: WriteHeader may be called twice when webdav processes methods such as PROPFIND Mar 4, 2024
@y805939188
Copy link
Author

@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.

@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants