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>

http2/transport.go

@@ -1216,6 +1216,8 @@ var (
 
 	// 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 @@ func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) (
 
 	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

http2/transport_test.go

@@ -4319,3 +4319,114 @@ func testTransportBodyReadError(t *testing.T, body []byte) {
 
 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)
+	}
+}