net/http/httputil: report ErrBodyReadAfterClose on dumped bodies

src/net/http/httputil/dump.go

@@ -14,11 +14,62 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+	"sync/atomic"
 	"time"
 )
 
+// dumpBody is an io.ReadCloser that reports [http.ErrBodyReadAfterClose]
+// once it has been closed, as a real request or response body does.
+//
+// Close may be called concurrently with Read, as [http.Request.Body]
+// requires. The readers dumpBody wraps are all in memory, so Read never
+// blocks and there is nothing for Close to interrupt.
+type dumpBody struct {
+	r      io.Reader
+	closed atomic.Bool
+}
+
+func (b *dumpBody) Read(p []byte) (int, error) {
+	if b.closed.Load() {
+		return 0, http.ErrBodyReadAfterClose
+	}
+	return b.r.Read(p)
+}
+
+func (b *dumpBody) Close() error {
+	b.closed.Store(true)
+	return nil
+}
+
+// dumpBodyWriterTo is a dumpBody whose underlying reader implements
+// [io.WriterTo]. It forwards WriteTo so that copying from the body avoids
+// a buffer copy, just as [io.NopCloser] does for such readers.
+type dumpBodyWriterTo struct{ *dumpBody }
+
+func (b dumpBodyWriterTo) WriteTo(w io.Writer) (int64, error) {
+	if b.closed.Load() {
+		return 0, http.ErrBodyReadAfterClose
+	}
+	return b.r.(io.WriterTo).WriteTo(w)
+}
+
+// newDumpBody returns an [io.ReadCloser] that yields the bytes of r and
+// reports [http.ErrBodyReadAfterClose] once it has been closed.
+func newDumpBody(r io.Reader) io.ReadCloser {
+	b := &dumpBody{r: r}
+	if _, ok := r.(io.WriterTo); ok {
+		return dumpBodyWriterTo{b}
+	}
+	return b
+}
+
 // drainBody reads all of b to memory and then returns two equivalent
-// ReadClosers yielding the same bytes.
+// ReadClosers yielding the same bytes. Reading from r1 after it has been
+// closed reports [http.ErrBodyReadAfterClose], as reading from a real
+// request or response body does; r1 is the one the Dump functions hand
+// back to their caller. r2 is only used to produce the dump, so it stays
+// an [io.NopCloser], which lets net/http recognize it as an in-memory
+// reader and copy from it without an intermediate buffer.
 //
 // It returns an error if the initial slurp of all bytes fails. It does not attempt
 // to make the returned ReadClosers have identical error-matching behavior.
@@ -34,7 +85,7 @@ func drainBody(b io.ReadCloser) (r1, r2 io.ReadCloser, err error) {
 	if err = b.Close(); err != nil {
 		return nil, b, err
 	}
-	return io.NopCloser(&buf), io.NopCloser(bytes.NewReader(buf.Bytes())), nil
+	return newDumpBody(&buf), io.NopCloser(bytes.NewReader(buf.Bytes())), nil
 }
 
 // dumpConn is a net.Conn which writes to Writer and reads from Reader

src/net/http/httputil/dump_test.go

@@ -16,6 +16,7 @@ import (
 	"runtime"
 	"runtime/pprof"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 )
@@ -491,6 +492,136 @@ func TestDumpResponse(t *testing.T) {
 	}
 }
 
+// Issue 77463: the body the Dump functions leave behind should report
+// http.ErrBodyReadAfterClose once closed, as a real body does.
+func TestDumpBodyReadAfterClose(t *testing.T) {
+	const body = "hello body"
+
+	newReq := func() *http.Request {
+		req, err := http.NewRequest("POST", "http://example.com/", strings.NewReader(body))
+		if err != nil {
+			t.Fatal(err)
+		}
+		return req
+	}
+
+	tests := []struct {
+		name string
+		dump func() (io.ReadCloser, error)
+	}{
+		{"DumpRequest", func() (io.ReadCloser, error) {
+			req := newReq()
+			_, err := DumpRequest(req, true)
+			return req.Body, err
+		}},
+		{"DumpRequestOut", func() (io.ReadCloser, error) {
+			req := newReq()
+			_, err := DumpRequestOut(req, true)
+			return req.Body, err
+		}},
+		{"DumpResponse", func() (io.ReadCloser, error) {
+			res := &http.Response{
+				StatusCode:    200,
+				ProtoMajor:    1,
+				ProtoMinor:    1,
+				ContentLength: int64(len(body)),
+				Body:          io.NopCloser(strings.NewReader(body)),
+			}
+			_, err := DumpResponse(res, true)
+			return res.Body, err
+		}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b, err := tt.dump()
+			if err != nil {
+				t.Fatalf("%s: %v", tt.name, err)
+			}
+			// The body is still readable until it is closed.
+			got, err := io.ReadAll(b)
+			if err != nil {
+				t.Fatalf("ReadAll before Close: %v", err)
+			}
+			if string(got) != body {
+				t.Errorf("body before Close = %q, want %q", got, body)
+			}
+			if err := b.Close(); err != nil {
+				t.Fatalf("Close: %v", err)
+			}
+			n, err := b.Read(make([]byte, 1))
+			if n != 0 || err != http.ErrBodyReadAfterClose {
+				t.Errorf("Read after Close = (%v, %v), want (0, %v)", n, err, http.ErrBodyReadAfterClose)
+			}
+		})
+	}
+}
+
+// http.Request.Body requires that Close may be called concurrently with
+// Read, so the body left behind by the Dump functions must allow it too.
+// Run under -race to be meaningful.
+func TestDumpBodyConcurrentReadClose(t *testing.T) {
+	req, err := http.NewRequest("POST", "http://example.com/", strings.NewReader(strings.Repeat("x", 4096)))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := DumpRequest(req, true); err != nil {
+		t.Fatal(err)
+	}
+	body := req.Body
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+	go func() {
+		defer wg.Done()
+		buf := make([]byte, 16)
+		for {
+			if _, err := body.Read(buf); err != nil {
+				return
+			}
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		body.Close()
+	}()
+	wg.Wait()
+}
+
+// The body left behind by the Dump functions should still implement
+// io.WriterTo, as the io.NopCloser it replaced did, so that copying from
+// it does not need an intermediate buffer.
+func TestDumpBodyWriterTo(t *testing.T) {
+	const body = "hello body"
+	req, err := http.NewRequest("POST", "http://example.com/", strings.NewReader(body))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := DumpRequest(req, true); err != nil {
+		t.Fatal(err)
+	}
+
+	wt, ok := req.Body.(io.WriterTo)
+	if !ok {
+		t.Fatalf("dumped body %T does not implement io.WriterTo", req.Body)
+	}
+	var buf bytes.Buffer
+	if _, err := wt.WriteTo(&buf); err != nil {
+		t.Fatalf("WriteTo: %v", err)
+	}
+	if buf.String() != body {
+		t.Errorf("WriteTo wrote %q, want %q", buf.String(), body)
+	}
+
+	// Once closed, WriteTo reports the same error as Read.
+	if err := req.Body.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+	if _, err := wt.WriteTo(io.Discard); err != http.ErrBodyReadAfterClose {
+		t.Errorf("WriteTo after Close = %v, want %v", err, http.ErrBodyReadAfterClose)
+	}
+}
+
 // Issue 38352: Check for deadlock on canceled requests.
 func TestDumpRequestOutIssue38352(t *testing.T) {
 	if testing.Short() {