http2: end stream eagerly after sending the request body
Check EOF eagerly on the request body when its content-length
is specified and it is expected to end. Thus, the data frame
containing the last chunk of data of the body will be marked with
END_STREAM eagerly.
In case the request body is larger than the specified content-length,
the request will be aborted and returned with an error.
Fixes golang/go#32254
Change-Id: Id24c043c7cc3a41421dfd099a139f1b1e08056b9
Reviewed-on: https://go-review.googlesource.com/c/net/+/181157
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/http2/transport.go b/http2/transport.go
index aeac7d8..c51a73c 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -1216,6 +1216,8 @@
// abort request body write, but send stream reset of cancel.
errStopReqBodyWriteAndCancel = errors.New("http2: canceling request")
+
+ errReqBodyTooLong = errors.New("http2: request body larger than specified content length")
)
func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) (err error) {
@@ -1238,10 +1240,32 @@
req := cs.req
hasTrailers := req.Trailer != nil
+ remainLen := actualContentLength(req)
+ hasContentLen := remainLen != -1
var sawEOF bool
for !sawEOF {
- n, err := body.Read(buf)
+ n, err := body.Read(buf[:len(buf)-1])
+ if hasContentLen {
+ remainLen -= int64(n)
+ if remainLen == 0 && err == nil {
+ // The request body's Content-Length was predeclared and
+ // we just finished reading it all, but the underlying io.Reader
+ // returned the final chunk with a nil error (which is one of
+ // the two valid things a Reader can do at EOF). Because we'd prefer
+ // to send the END_STREAM bit early, double-check that we're actually
+ // at EOF. Subsequent reads should return (0, EOF) at this point.
+ // If either value is different, we return an error in one of two ways below.
+ var n1 int
+ n1, err = body.Read(buf[n:])
+ remainLen -= int64(n1)
+ }
+ if remainLen < 0 {
+ err = errReqBodyTooLong
+ cc.writeStreamReset(cs.ID, ErrCodeCancel, err)
+ return err
+ }
+ }
if err == io.EOF {
sawEOF = true
err = nil
diff --git a/http2/transport_test.go b/http2/transport_test.go
index d90fa12..89ce5e5 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -4319,3 +4319,114 @@
func TestTransportBodyReadError_Immediately(t *testing.T) { testTransportBodyReadError(t, nil) }
func TestTransportBodyReadError_Some(t *testing.T) { testTransportBodyReadError(t, []byte("123")) }
+
+// Issue 32254: verify that the client sends END_STREAM flag eagerly with the last
+// (or in this test-case the only one) request body data frame, and does not send
+// extra zero-len data frames.
+func TestTransportBodyEagerEndStream(t *testing.T) {
+ const reqBody = "some request body"
+ const resBody = "some response body"
+
+ ct := newClientTester(t)
+ ct.client = func() error {
+ defer ct.cc.(*net.TCPConn).CloseWrite()
+ body := strings.NewReader(reqBody)
+ req, err := http.NewRequest("PUT", "https://dummy.tld/", body)
+ if err != nil {
+ return err
+ }
+ _, err = ct.tr.RoundTrip(req)
+ if err != nil {
+ return err
+ }
+ return nil
+ }
+ ct.server = func() error {
+ ct.greet()
+
+ for {
+ f, err := ct.fr.ReadFrame()
+ if err != nil {
+ return err
+ }
+
+ switch f := f.(type) {
+ case *WindowUpdateFrame, *SettingsFrame:
+ case *HeadersFrame:
+ case *DataFrame:
+ if !f.StreamEnded() {
+ ct.fr.WriteRSTStream(f.StreamID, ErrCodeRefusedStream)
+ return fmt.Errorf("data frame without END_STREAM %v", f)
+ }
+ var buf bytes.Buffer
+ enc := hpack.NewEncoder(&buf)
+ enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"})
+ ct.fr.WriteHeaders(HeadersFrameParam{
+ StreamID: f.Header().StreamID,
+ EndHeaders: true,
+ EndStream: false,
+ BlockFragment: buf.Bytes(),
+ })
+ ct.fr.WriteData(f.StreamID, true, []byte(resBody))
+ return nil
+ case *RSTStreamFrame:
+ default:
+ return fmt.Errorf("Unexpected client frame %v", f)
+ }
+ }
+ }
+ ct.run()
+}
+
+type chunkReader struct {
+ chunks [][]byte
+}
+
+func (r *chunkReader) Read(p []byte) (int, error) {
+ if len(r.chunks) > 0 {
+ n := copy(p, r.chunks[0])
+ r.chunks = r.chunks[1:]
+ return n, nil
+ }
+ panic("shouldn't read this many times")
+}
+
+// Issue 32254: if the request body is larger than the specified
+// content length, the client should refuse to send the extra part
+// and abort the stream.
+//
+// In _len3 case, the first Read() matches the expected content length
+// but the second read returns more data.
+//
+// In _len2 case, the first Read() exceeds the expected content length.
+func TestTransportBodyLargerThanSpecifiedContentLength_len3(t *testing.T) {
+ body := &chunkReader{[][]byte{
+ []byte("123"),
+ []byte("456"),
+ }}
+ testTransportBodyLargerThanSpecifiedContentLength(t, body, 3)
+}
+
+func TestTransportBodyLargerThanSpecifiedContentLength_len2(t *testing.T) {
+ body := &chunkReader{[][]byte{
+ []byte("123"),
+ }}
+ testTransportBodyLargerThanSpecifiedContentLength(t, body, 2)
+}
+
+func testTransportBodyLargerThanSpecifiedContentLength(t *testing.T, body *chunkReader, contentLen int64) {
+ st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
+ // Nothing.
+ }, optOnlyServer)
+ defer st.Close()
+
+ tr := &Transport{TLSClientConfig: tlsConfigInsecure}
+ defer tr.CloseIdleConnections()
+
+ req, _ := http.NewRequest("POST", st.ts.URL, body)
+ req.ContentLength = contentLen
+ _, err := tr.RoundTrip(req)
+ if err != errReqBodyTooLong {
+ t.Fatalf("expected %v, got %v", errReqBodyTooLong, err)
+ }
+}
