[release-branch.go1.7] http2: never Read from Request.Body in Transport to determine ContentLength
Updates golang/go#17480 (fixes after vendor to std)
Updates golang/go#17071 (a better fix than the original one)
Change-Id: Ibb49d2cb825e28518e688b5c3e0952956a305050
Reviewed-on: https://go-review.googlesource.com/31326
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Reviewed-on: https://go-review.googlesource.com/31361
Run-TryBot: Chris Broadfoot <cbro@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/http2/transport.go b/http2/transport.go
index 5762039..7ea513e 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -610,34 +610,17 @@
return nil
}
-func bodyAndLength(req *http.Request) (body io.Reader, contentLen int64) {
- body = req.Body
- if body == nil {
- return nil, 0
+// actualContentLength returns a sanitized version of
+// req.ContentLength, where 0 actually means zero (not unknown) and -1
+// means unknown.
+func actualContentLength(req *http.Request) int64 {
+ if req.Body == nil {
+ return 0
}
if req.ContentLength != 0 {
- return req.Body, req.ContentLength
+ return req.ContentLength
}
-
- // We have a body but a zero content length. Test to see if
- // it's actually zero or just unset.
- var buf [1]byte
- n, rerr := body.Read(buf[:])
- if rerr != nil && rerr != io.EOF {
- return errorReader{rerr}, -1
- }
- if n == 1 {
- // Oh, guess there is data in this Body Reader after all.
- // The ContentLength field just wasn't set.
- // Stitch the Body back together again, re-attaching our
- // consumed byte.
- if rerr == io.EOF {
- return bytes.NewReader(buf[:]), 1
- }
- return io.MultiReader(bytes.NewReader(buf[:]), body), -1
- }
- // Body is actually zero bytes.
- return nil, 0
+ return -1
}
func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
@@ -658,8 +641,9 @@
return nil, errClientConnUnusable
}
- body, contentLen := bodyAndLength(req)
+ body := req.Body
hasBody := body != nil
+ contentLen := actualContentLength(req)
// TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere?
var requestedGzip bool
diff --git a/http2/transport_test.go b/http2/transport_test.go
index bfc5400..dcf7e59 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -374,6 +374,40 @@
return string(b)
}
+type panicReader struct{}
+
+func (panicReader) Read([]byte) (int, error) { panic("unexpected Read") }
+func (panicReader) Close() error { panic("unexpected Close") }
+
+func TestActualContentLength(t *testing.T) {
+ tests := []struct {
+ req *http.Request
+ want int64
+ }{
+ // Verify we don't read from Body:
+ 0: {
+ req: &http.Request{Body: panicReader{}},
+ want: -1,
+ },
+ // nil Body means 0, regardless of ContentLength:
+ 1: {
+ req: &http.Request{Body: nil, ContentLength: 5},
+ want: 0,
+ },
+ // ContentLength is used if set.
+ 2: {
+ req: &http.Request{Body: panicReader{}, ContentLength: 5},
+ want: 5,
+ },
+ }
+ for i, tt := range tests {
+ got := actualContentLength(tt.req)
+ if got != tt.want {
+ t.Errorf("test[%d]: got %d; want %d", i, got, tt.want)
+ }
+ }
+}
+
func TestTransportBody(t *testing.T) {
bodyTests := []struct {
body string
@@ -381,8 +415,6 @@
}{
{body: "some message"},
{body: "some message", noContentLen: true},
- {body: ""},
- {body: "", noContentLen: true},
{body: strings.Repeat("a", 1<<20), noContentLen: true},
{body: strings.Repeat("a", 1<<20)},
{body: randString(16<<10 - 1)},
