net/http: in Client, consume small redirect bodies before making next request

In Go 1.2, closing a request body without reading to EOF
causes the underlying TCP connection to not be reused. This
client code following redirects was never updated when that
happened.

This was part of a previous CL but moved to its own CL at
Josh's request.  Now with test too.

LGTM=josharian
R=josharian
CC=golang-codereviews
https://golang.org/cl/70800043

diff --git a/src/pkg/net/http/client.go b/src/pkg/net/http/client.go
index 952799a1be613fc80206d976b594bf963b6b4c37..ee0753d35e7091b03c72bb0a97deae98e2e74cdf 100644 (file)
--- a/src/pkg/net/http/client.go
+++ b/src/pkg/net/http/client.go
@@ -14,6 +14,7 @@ import (
        "errors"
        "fmt"
        "io"
+       "io/ioutil"
        "log"
        "net/url"
        "strings"
@@ -337,6 +338,12 @@ func (c *Client) doFollowingRedirects(ireq *Request, shouldRedirect func(int) bo
                }
 
                if shouldRedirect(resp.StatusCode) {
+                       // Read the body if small so underlying TCP connection will be re-used.
+                       // No need to check for errors: if it fails, Transport won't reuse it anyway.
+                       const maxBodySlurpSize = 2 << 10
+                       if resp.ContentLength == -1 || resp.ContentLength <= maxBodySlurpSize {
+                               io.CopyN(ioutil.Discard, resp.Body, maxBodySlurpSize)
+                       }
                        resp.Body.Close()
                        if urlStr = resp.Header.Get("Location"); urlStr == "" {
                                err = errors.New(fmt.Sprintf("%d response missing Location header", resp.StatusCode))

diff --git a/src/pkg/net/http/client_test.go b/src/pkg/net/http/client_test.go
index f44fb199dceccc4231508042d8fec926c48abccf..db825d21dcd3f0c6b5b31563b243add24fed79f5 100644 (file)
--- a/src/pkg/net/http/client_test.go
+++ b/src/pkg/net/http/client_test.go
@@ -879,3 +879,43 @@ func TestClientTimeout(t *testing.T) {
                t.Errorf("timeout after %v waiting for timeout of %v", failTime, timeout)
        }
 }
+
+func TestClientRedirectEatsBody(t *testing.T) {
+       defer afterTest(t)
+       saw := make(chan string, 2)
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+               saw <- r.RemoteAddr
+               if r.URL.Path == "/" {
+                       Redirect(w, r, "/foo", StatusFound) // which includes a body
+               }
+       }))
+       defer ts.Close()
+
+       res, err := Get(ts.URL)
+       if err != nil {
+               t.Fatal(err)
+       }
+       _, err = ioutil.ReadAll(res.Body)
+       if err != nil {
+               t.Fatal(err)
+       }
+       res.Body.Close()
+
+       var first string
+       select {
+       case first = <-saw:
+       default:
+               t.Fatal("server didn't see a request")
+       }
+
+       var second string
+       select {
+       case second = <-saw:
+       default:
+               t.Fatal("server didn't see a second request")
+       }
+
+       if first != second {
+               t.Fatal("server saw different client ports before & after the redirect")
+       }
+}