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.

Sign up for GitHub

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: ReverseProxy doesn't handle nil request body... #12344

Closed
samprakos opened this issue Aug 26, 2015 · 5 comments
Closed

net/http: ReverseProxy doesn't handle nil request body... #12344

samprakos opened this issue Aug 26, 2015 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.5.1

Comments

@samprakos
Copy link

My code is paraphrased here (where rp is an httputil.NewSingleHostReverseProxy):

req, _ := http.NewRequest("GET", myURL, nil)
rp.ServeHTTP(w, req)

This worked in 1.4.2 but panics in 1.5 with:

goroutine 84 [running]:
net/http/httputil.(_runOnFirstRead).Read(0xc820458d40, 0xc82044bb90, 0x1, 0x1, 0x0, 0x0, 0x0)
/usr/local/Cellar/go/1.5/libexec/src/net/http/httputil/reverseproxy.go:118 +0x83
go.(_struct { io.Reader; io.Closer }).Read(0xc820458d60, 0xc82044bb90, 0x1, 0x1, 0x0, 0x0, 0x0)
:43 +0x82
io.ReadAtLeast(0x28047d0, 0xc820458d60, 0xc82044bb90, 0x1, 0x1, 0x1, 0x0, 0x0, 0x0)
/usr/local/Cellar/go/1.5/libexec/src/io/io.go:298 +0xe6
io.ReadFull(0x28047d0, 0xc820458d60, 0xc82044bb90, 0x1, 0x1, 0xc71788, 0x0, 0x0)
/usr/local/Cellar/go/1.5/libexec/src/io/io.go:316 +0x62
net/http.newTransferWriter(0x5d80a0, 0xc8202e7960, 0xc820445490, 0x0, 0x0)
/usr/local/Cellar/go/1.5/libexec/src/net/http/transfer.go:71 +0xa99
net/http.(_Request).write(0xc8202e7960, 0xc716e8, 0xc8203adfc0, 0x0, 0xc82045cb10, 0x0, 0x0)
/usr/local/Cellar/go/1.5/libexec/src/net/http/request.go:435 +0x9ac
net/http.(_persistConn).writeLoop(0xc8203262c0)
/usr/local/Cellar/go/1.5/libexec/src/net/http/transport.go:1015 +0x27c
created by net/http.(*Transport).dialConn
/usr/local/Cellar/go/1.5/libexec/src/net/http/transport.go:686 +0xc9d

I changed my NewRequest code to this: http.NewRequest("GET", meUrl, strings.NewReader("")) and it works fine.
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Aug 26, 2015
@ianlancetaylor ianlancetaylor changed the title ReverseProxy doesn't handle nil request body... net/http: ReverseProxy doesn't handle nil request body... Aug 26, 2015
@bradfitz
Copy link
Contributor

ReverseProxy is generally used to turn incoming server requests into outgoing client requests to the proxy backend.

Server requests are guaranteed to have a non-nil body. The docs say:

        // For server requests the Request Body is always non-nil
        // but will return EOF immediately when no body is present.
        // The Server will close the request body. The ServeHTTP
        // Handler does not need to.
        Body io.ReadCloser

Still, this is easy enough to fix.

@bradfitz
Copy link
Contributor

I'm tagging this as Go 1.5.1 so somebody else can decide whether to take this simple fix. Of course, there's a workaround: fork the httputil package and fix ReverseProxy in the fork.

Fix is: https://go-review.googlesource.com/13935

@bradfitz bradfitz modified the milestones: Go1.5.1, Go1.6 Aug 26, 2015
@gopherbot
Copy link

CL https://golang.org/cl/13935 mentions this issue.

@samprakos
Copy link
Author

Thanks!! The documentation for http.NewRequest specifies that body is optional and has nil checking there.

@gopherbot
Copy link

CL https://golang.org/cl/14245 mentions this issue.

bradfitz added a commit that referenced this issue Sep 8, 2015
@bradfitz @broady
[release-branch.go1.5] net/http/httputil: permit nil request body in …
c0dd201 
…ReverseProxy

Accepting a request with a nil body was never explicitly supported but
happened to work in the past.

This doesn't happen in most cases because usually people pass
a Server's incoming Request to the ReverseProxy's ServeHTTP method,
and incoming server requests are guaranteed to have non-nil bodies.

Still, it's a regression, so fix.

Fixes #12344

Change-Id: Id9a5a47aea3f2875d195b66c9a5f8581c4ca2aed
Reviewed-on: https://go-review.googlesource.com/13935
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/14245
@golang golang locked and limited conversation to collaborators Sep 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5.1
Development

No branches or pull requests
4 participants
@bradfitz @ianlancetaylor @gopherbot @samprakos