Revert "net/http: do not discard body content when closing it within request handlers"

This reverts commit cb0d9980f5721715ebb73dd2e580eaa11c2ddee2.

Reason for revert: the old behavior seems to be relied on by current
users, e.g.
https://github.com/connectrpc/connect-go/blob/cb2e11fb88c9a61804043355a619c12d4a30a1a5/protocol_connect.go#L837.

For #75933

Change-Id: I996280238e5c70a8d760a0b31e3a13c6a44b8616
Reviewed-on: https://go-review.googlesource.com/c/go/+/721761
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>

1 files changed, 10 insertions, 21 deletions

diff --git a/src/net/http/server.go b/src/net/http/server.go
index 5d8e576f71..02554d1a20 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -1077,6 +1077,9 @@ func (c *conn) readRequest(ctx context.Context) (w *response, err error) {
 	req.ctx = ctx
 	req.RemoteAddr = c.remoteAddr
 	req.TLS = c.tlsState
+	if body, ok := req.Body.(*body); ok {
+		body.doEarlyClose = true
+	}
 
 	// Adjust the read deadline if necessary.
 	if !hdrDeadline.Equal(wholeReqDeadline) {
@@ -1708,11 +1711,9 @@ func (w *response) finishRequest() {
 
 	w.conn.r.abortPendingRead()
 
-	// Try to discard the body (regardless of w.closeAfterReply), so we can
-	// potentially reuse it in the same connection.
-	if b, ok := w.reqBody.(*body); ok {
-		b.tryDiscardBody()
-	}
+	// Close the body (regardless of w.closeAfterReply) so we can
+	// re-use its bufio.Reader later safely.
+	w.reqBody.Close()
 
 	if w.req.MultipartForm != nil {
 		w.req.MultipartForm.RemoveAll()
@@ -1740,16 +1741,16 @@ func (w *response) shouldReuseConnection() bool {
 		return false
 	}
 
-	if w.didIncompleteDiscard() {
+	if w.closedRequestBodyEarly() {
 		return false
 	}
 
 	return true
 }
 
-func (w *response) didIncompleteDiscard() bool {
+func (w *response) closedRequestBodyEarly() bool {
 	body, ok := w.req.Body.(*body)
-	return ok && body.didIncompleteDiscard()
+	return ok && body.didEarlyClose()
 }
 
 func (w *response) Flush() {
@@ -2105,18 +2106,6 @@ func (c *conn) serve(ctx context.Context) {
 		// But we're not going to implement HTTP pipelining because it
 		// was never deployed in the wild and the answer is HTTP/2.
 		inFlightResponse = w
-		// Ensure that Close() invocations within request handlers do not
-		// discard the body.
-		if b, ok := w.reqBody.(*body); ok {
-			b.mu.Lock()
-			b.inRequestHandler = true
-			b.mu.Unlock()
-			defer func() {
-				b.mu.Lock()
-				b.inRequestHandler = false
-				b.mu.Unlock()
-			}()
-		}
 		serverHandler{c.server}.ServeHTTP(w, w.req)
 		inFlightResponse = nil
 		w.cancelCtx()
@@ -2127,7 +2116,7 @@ func (c *conn) serve(ctx context.Context) {
 		w.finishRequest()
 		c.rwc.SetWriteDeadline(time.Time{})
 		if !w.shouldReuseConnection() {
-			if w.requestBodyLimitHit || w.didIncompleteDiscard() {
+			if w.requestBodyLimitHit || w.closedRequestBodyEarly() {
 				c.closeWriteAndWait()
 			}
 			return