Using DumpResponse can Lead to Unclosed Response Bodies

On the surface httputil.DumpResponse(resp *http.Response, body bool) ([]byte, error) seems like a very simple function that serializes a http.Response into a []byte. However, using this function correctly is harder than it seems. In fact, even the standard library’s example has a subtle bug that stems from using httputil.DumpResponse().

Let’s take a look at the example in question:

func main() {
	const body = "Go is a general-purpose language designed with systems programming in mind."
	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Date", "Wed, 19 Jul 1972 19:00:00 GMT")
		fmt.Fprintln(w, body)
	}))
	defer ts.Close()

	resp, err := http.Get(ts.URL)
	if err != nil {
		log.Fatal(err)
	}
	defer resp.Body.Close()

	dump, err := httputil.DumpResponse(resp, true)
	if err != nil {
		log.Fatal(err)
	}

	fmt.Printf("%q", dump)

}

Running that program will produce the following output:

> "HTTP/1.1 200 OK\r\nContent-Length: 76\r\nContent-Type: text/plain; charset=utf-8\r\nDate: Wed, 19 Jul 1972 19:00:00 GMT\r\n\r\nGo is a general-purpose language designed with systems programming in mind.\n"

That seems to be correct, so what’s the problem you ask ?

DumpResponse Mutates the Response Body

In order to dump the response body, we first need to read all of it. This poses a problem as now the client can’t read the response body it requested. Go solves this by reading the original response body, closing it, and mutating the response body with a new (unread and unclosed) response body that has the same content as the original. Let’s see what it looks like in code:

// drainBody reads all of b to memory and then returns two equivalent
// ReadClosers yielding the same bytes.
//
// 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.
func drainBody(b io.ReadCloser) (r1, r2 io.ReadCloser, err error) {
	if b == nil || b == http.NoBody {
		// No copying needed. Preserve the magic sentinel meaning of NoBody.
		return http.NoBody, http.NoBody, nil
	}
	var buf bytes.Buffer
	if _, err = buf.ReadFrom(b); err != nil {
		return nil, b, err
	}
	if err = b.Close(); err != nil {
		return nil, b, err
	}
	return ioutil.NopCloser(&buf), ioutil.NopCloser(bytes.NewReader(buf.Bytes())), nil
}

// DumpResponse is like DumpRequest but dumps a response.
func DumpResponse(resp *http.Response, body bool) ([]byte, error) {
  	var b bytes.Buffer
  /* omit code for brevity */
  save, resp.Body, err = drainBody(resp.Body)
  /* omit code for brevity */
 	err = resp.Write(&b)
	resp.Body = save
	return b.Bytes(), nil
}

drainBody needs to return two io.ReadClosers so that the second return value r2 can be used to read the and dump the response body to a byte buffer.The next step is to set the response.Body to r1 which allows a client to read the response body even after it has been dumped. Thus, the response body after a call to DumpResonse() is completely different to what it was before. Let’s revist the example from the standard library but with some changes to prove our hypothesis:

type WrappedBody struct {
	closeCount int
	name       string
	io.ReadCloser
}

func (wb *WrappedBody) Read(p []byte) (n int, err error) {
	return wb.ReadCloser.Read(p)
}

func (wb *WrappedBody) Close() error {
	wb.closeCount += 1
	fmt.Printf("\nresponse.Body named: %s close count: %d\n", wb.name, wb.closeCount)
	return wb.ReadCloser.Close()
}

func main() {
	const body = "Go is a general-purpose language designed with systems programming in mind."
	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Date", "Wed, 19 Jul 1972 19:00:00 GMT")
		fmt.Fprintln(w, body)
	}))
	defer ts.Close()

	resp, err := http.Get(ts.URL)
	if err != nil {
		log.Fatal(err)
	}
	resp.Body = &WrappedBody{
		name:       "original",
		ReadCloser: resp.Body,
	}
	defer resp.Body.Close()

	dump, err := httputil.DumpResponse(resp, true)
	if err != nil {
		log.Fatal(err)
	}

	fmt.Printf("%q", dump)
}
response.Body named: original close count: 1
"HTTP/1.1 200 OK\r\nContent-Length: 76\r\nContent-Type: text/plain; charset=utf-8\r\nDate: Wed, 19 Jul 1972 19:00:00 GMT\r\n\r\nGo is a general-purpose language designed with systems programming in mind.\n"
response.Body named: original close count: 2

Wrapping the response body to intercept the Close() call shows that the response body is being closed twice. The first time is from within drainBody() and the second is when our program returns from main and runs defer resp.Body.Close().

The astute reader may have realized that if we are closing the original response body twice, it means we are not closing the new response body returned by DumpResponse.

Detour into Defer

It is worth pointing out that even though the call to close the response body is deferred in our example, it doesn’t call Close() on the new mutated reponse body but on the original one.

This is a consequence of defer evaluating the arguments and method receivers when the defer statement is executed, even though the Close() function is run after main() returns.

Let’s Fix the Example

Putting everything we’ve learned so far together, we can fix the example in two ways, both of which are shown below:

func main() {
	const body = "Go is a general-purpose language designed with systems programming in mind."
	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Date", "Wed, 19 Jul 1972 19:00:00 GMT")
		fmt.Fprintln(w, body)
	}))
	defer ts.Close()

	resp, err := http.Get(ts.URL)
	if err != nil {
		log.Fatal(err)
	}
	resp.Body = &WrappedBody{
		name:       "original",
		ReadCloser: resp.Body,
	}

	// Alternative Fix:
	// defer func() { resp.Body.Close() }()
	dump, err := httputil.DumpResponse(resp, true)
	if err != nil {
		log.Fatal(err)
	}

	resp.Body = &WrappedBody{
		name:       "postDumpResponse",
		ReadCloser: resp.Body,
	}

	defer resp.Body.Close()

	fmt.Printf("%q", dump)
}
response.Body named: original close count: 1
"HTTP/1.1 200 OK\r\nContent-Length: 76\r\nContent-Type: text/plain; charset=utf-8\r\nDate: Wed, 19 Jul 1972 19:00:00 GMT\r\n\r\nGo is a general-purpose language designed with systems programming in mind.\n"
response.Body named: postDumpResponse close count: 1

DumpResponse() closes the original response body and our defer correctly closes the mutated response body.

Good News

Usually, when we forget to close a response body, we have a memory/go-routine leak. However, in this case the io.ReadCloser returned by drainBody is a NopCloser which means we won’t leak any memory or connections even if we fail to close the response body.