Skip to content

net/http/httputil: ReverseProxy calls WriteHeader multiple times on a single response #70237

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

Closed
aakog opened this issue Nov 7, 2024 · 3 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@aakog
Copy link

aakog commented Nov 7, 2024

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

$ go version
go version go1.23.2 darwin/arm64

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='arm64'
GOBIN=''
GOCACHE='/Users/aaron/Library/Caches/go-build'
GOENV='/Users/aaron/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/aaron/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/aaron/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.2/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/aaron/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/aaron/kognitos/brain/aiproxy/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/gv/b9j8dl_j2gz5d_6n94m3qmj80000gn/T/go-build520885361=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOROOT/bin/go version: go version go1.23.2 darwin/arm64
GOROOT/bin/go tool compile -V: compile version go1.23.2
uname -v: Darwin Kernel Version 24.0.0: Mon Aug 12 20:52:12 PDT 2024; root:xnu-11215.1.10~2/RELEASE_ARM64_T6020
ProductName:		macOS
ProductVersion:		15.0
BuildVersion:		24A335
lldb --version: lldb-1600.0.36.3
Apple Swift version 6.0 (swiftlang-6.0.0.9.10 clang-1600.0.26.2)

What did you do?

https://go.dev/play/p/P_8ZOOmh4UV

What did you expect to see?

response.WriteHeader should be called exactly once with 502 as the status code.

What did you see instead?

ReverseProxy calls response.WriteHeader multiple times on one response.

http: superfluous response.WriteHeader call from net/http/httputil.(*ReverseProxy).defaultErrorHandler (reverseproxy.go:308)
@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 7, 2024
@aakog
Copy link
Author

aakog commented Nov 13, 2024

Some more context here: I'm using httputil.ReverseProxy and I want to record a metric once per request which tracks the result of the request. ReverseProxy.ErrorHandler seems like a reasonable place to add such a metric for the case when the request fails. However, it's not clear from the docs if ErrorHandler could be called more than once for a single request.

Looking at the code, it turns out that ErrorHandler will be called multiple times in some cases. One such case is here https://go.dev/play/p/P_8ZOOmh4UV

On the other hand, ReverseProxy.defaultErrorHandler should not be called more than once per request. It calls http.ResponseWriter.WriteHeader which requires "at most one 2xx-5xx header".

Ideally the ReverseProxy interface would be clarified so that it is simple to track how many requests failed.

@seankhliao seankhliao changed the title net/http/httputil.ReverseProxy calls WriteHeader multiple times on a single response net/http/httputil: ReverseProxy calls WriteHeader multiple times on a single response Nov 13, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/627635 mentions this issue: net/http/httputil: return after handling error

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 18, 2024
@dmitshur dmitshur added this to the Go1.24 milestone Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants