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

Jump to bottom

net/http/httputil: ReverseProxy.ServeHTTP regression #33142

Closed
dsnet opened this issue Jul 17, 2019 · 1 comment
Closed

net/http/httputil: ReverseProxy.ServeHTTP regression #33142

dsnet opened this issue Jul 17, 2019 · 1 comment
Labels
FrozenDueToAge release-blocker
Milestone
Go1.13

Comments

@dsnet
Copy link
Member

dsnet commented Jul 17, 2019

In Go1.12, the logic in the reverse proxy did the following:

go/src/net/http/httputil/reverseproxy.go

Lines 209 to 214 in f194c9f

outreq := req.WithContext(ctx) // includes shallow copies of maps, but okay
if req.ContentLength == 0 {
outreq.Body = nil // Issue 16036: nil Body for http.Transport retries
}
outreq.Header = cloneHeader(req.Header)

which calls the helper function cloneHeader, which always allocates the headers:

go/src/net/http/httputil/reverseproxy.go

Lines 135 to 143 in f194c9f

func cloneHeader(h http.Header) http.Header {
h2 := make(http.Header, len(h))
for k, vv := range h {
vv2 := make([]string, len(vv))
copy(vv2, vv)
h2[k] = vv2
}
return h2
}

The new logic calls the new http.Request.Clone method:

go/src/net/http/httputil/reverseproxy.go

Lines 198 to 201 in 0cadf40

outreq := req.Clone(ctx)
if req.ContentLength == 0 {
outreq.Body = nil // Issue 16036: nil Body for http.Transport retries
}

which does not allocate the headers if the source did not have them set:

go/src/net/http/request.go

Lines 378 to 380 in 0cadf40

if r.Header != nil {
r2.Header = r.Header.Clone()
}

The new behavior seems slightly more correct, but the fact that the headers are now nil is an observable effect that breaks users.
@dsnet dsnet added this to the Go1.13 milestone Jul 17, 2019
@dsnet dsnet self-assigned this Jul 17, 2019
@gopherbot
Copy link

Change https://golang.org/cl/186437 mentions this issue: net/http/httputil: fix regression in ReverseProxy.ServeHTTP

@golang golang locked and limited conversation to collaborators Jul 16, 2020
@rsc rsc unassigned dsnet Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
2 participants
@dsnet @gopherbot