net/http/httputil: clarify docs on the Dump functions

Also don't nil out the Request or Response Body on error. Just leave
it in its previous broken state. The docs now say it's undefined, but
it always was.

Fixes #14036

Change-Id: I7fe175a36cbc01b4158f4dffacd8733b2ffa9999
Reviewed-on: https://go-review.googlesource.com/18726
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/net/http/httputil/dump.go b/src/net/http/httputil/dump.go
index 6fe8fea110b7cafadf4b92776ee7e5f7d59609ea..e22cc66dbfccfee8e211832226e2e009e995eb79 100644 (file)
--- a/src/net/http/httputil/dump.go
+++ b/src/net/http/httputil/dump.go
@@ -25,10 +25,10 @@ import (
 func drainBody(b io.ReadCloser) (r1, r2 io.ReadCloser, err error) {
        var buf bytes.Buffer
        if _, err = buf.ReadFrom(b); err != nil {
-               return nil, nil, err
+               return nil, b, err
        }
        if err = b.Close(); err != nil {
-               return nil, nil, err
+               return nil, b, err
        }
        return ioutil.NopCloser(&buf), ioutil.NopCloser(bytes.NewReader(buf.Bytes())), nil
 }
@@ -175,15 +175,22 @@ func dumpAsReceived(req *http.Request, w io.Writer) error {
        return nil
 }
 
-// DumpRequest returns the as-received wire representation of req, optionally
-// including the request body, for debugging. It is for use in servers; use
-// DumpRequestOut for client requests.
+// DumpRequest returns the given request in its HTTP/1.x wire
+// representation. It should only be used by servers to debug client
+// requests. The returned representation is an approximation only;
+// some details of the initial request are lost while parsing it into
+// an http.Request. In particular, the order and case of header field
+// names are lost. The order of values in multi-valued headers is kept
+// intact. HTTP/2 requests are dumped in HTTP/1.x form, not in their
+// original binary representations.
+//
+// If body is true, DumpRequest also returns the body. To do so, it
+// consumes req.Body and then replaces it with a new io.ReadCloser
+// that yields the same bytes. If DumpRequest returns an error,
+// the state of req is undefined.
 //
-// DumpRequest is semantically a no-op, but in order to
-// dump the body, it reads the body data into memory and
-// changes req.Body to refer to the in-memory copy.
 // The documentation for http.Request.Write details which fields
-// of req are used.
+// of req are included in the dump.
 func DumpRequest(req *http.Request, body bool) (dump []byte, err error) {
        save := req.Body
        if !body || req.Body == nil {
@@ -191,7 +198,7 @@ func DumpRequest(req *http.Request, body bool) (dump []byte, err error) {
        } else {
                save, req.Body, err = drainBody(req.Body)
                if err != nil {
-                       return
+                       return nil, err
                }
        }
 
@@ -285,7 +292,7 @@ func DumpResponse(resp *http.Response, body bool) (dump []byte, err error) {
        } else {
                save, resp.Body, err = drainBody(resp.Body)
                if err != nil {
-                       return
+                       return nil, err
                }
        }
        err = resp.Write(&b)