Skip to content

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

New issue
New issue
Closed
Closed
net/http/httputil: ReverseProxy.ServeHTTP regression#33142
Labels
FrozenDueToAgerelease-blocker
Milestone
Go1.13
@dsnet

Description

@dsnet
dsnet
opened on 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgerelease-blocker

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions