From 4ffff35abd9562007b0cf4ee8034ded8760e5e2a Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Wed, 18 May 2011 17:17:26 -0700
Subject: [PATCH] http: Client.Do should follow redirects for GET and HEAD

It's documented as such, but it was never wired up
after Transport went in and Head was fixed.

If people don't want redirects, that's what RoundTripper/
Transport are for.  Or a custom redirect policy.

R=golang-dev, kevlar
CC=golang-dev
https://golang.org/cl/4526065
---
 src/pkg/http/client.go      | 58 ++++++++++++++++++++++---------------
 src/pkg/http/client_test.go | 11 +++++--
 2 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/src/pkg/http/client.go b/src/pkg/http/client.go
index 8b526696421..ac7ff185331 100644
--- a/src/pkg/http/client.go
+++ b/src/pkg/http/client.go
@@ -74,6 +74,9 @@ type readClose struct {
 //
 // Generally Get, Post, or PostForm will be used instead of Do.
 func (c *Client) Do(req *Request) (resp *Response, err os.Error) {
+	if req.Method == "GET" || req.Method == "HEAD" {
+		return c.doFollowingRedirects(req)
+	}
 	return send(req, c.Transport)
 }
 
@@ -144,10 +147,14 @@ func Get(url string) (r *Response, err os.Error) {
 //
 // Caller should close r.Body when done reading from it.
 func (c *Client) Get(url string) (r *Response, err os.Error) {
-	return c.sendFollowingRedirects("GET", url)
+	req, err := NewRequest("GET", url, nil)
+	if err != nil {
+		return nil, err
+	}
+	return c.doFollowingRedirects(req)
 }
 
-func (c *Client) sendFollowingRedirects(method, url string) (r *Response, err os.Error) {
+func (c *Client) doFollowingRedirects(ireq *Request) (r *Response, err os.Error) {
 	// TODO: if/when we add cookie support, the redirected request shouldn't
 	// necessarily supply the same cookies as the original.
 	var base *URL
@@ -157,33 +164,33 @@ func (c *Client) sendFollowingRedirects(method, url string) (r *Response, err os
 	}
 	var via []*Request
 
+	req := ireq
+	url := "" // next relative or absolute URL to fetch (after first request)
 	for redirect := 0; ; redirect++ {
-		var req Request
-		req.Method = method
-		req.Header = make(Header)
-		if base == nil {
-			req.URL, err = ParseURL(url)
-		} else {
+		if redirect != 0 {
+			req = new(Request)
+			req.Method = ireq.Method
+			req.Header = make(Header)
 			req.URL, err = base.ParseURL(url)
-		}
-		if err != nil {
-			break
-		}
-		if len(via) > 0 {
-			// Add the Referer header.
-			lastReq := via[len(via)-1]
-			if lastReq.URL.Scheme != "https" {
-				req.Referer = lastReq.URL.String()
-			}
-
-			err = redirectChecker(&req, via)
 			if err != nil {
 				break
 			}
+			if len(via) > 0 {
+				// Add the Referer header.
+				lastReq := via[len(via)-1]
+				if lastReq.URL.Scheme != "https" {
+					req.Referer = lastReq.URL.String()
+				}
+
+				err = redirectChecker(req, via)
+				if err != nil {
+					break
+				}
+			}
 		}
 
 		url = req.URL.String()
-		if r, err = send(&req, c.Transport); err != nil {
+		if r, err = send(req, c.Transport); err != nil {
 			break
 		}
 		if shouldRedirect(r.StatusCode) {
@@ -193,12 +200,13 @@ func (c *Client) sendFollowingRedirects(method, url string) (r *Response, err os
 				break
 			}
 			base = req.URL
-			via = append(via, &req)
+			via = append(via, req)
 			continue
 		}
 		return
 	}
 
+	method := ireq.Method
 	err = &URLError{method[0:1] + strings.ToLower(method[1:]), url, err}
 	return
 }
@@ -310,5 +318,9 @@ func Head(url string) (r *Response, err os.Error) {
 //    303 (See Other)
 //    307 (Temporary Redirect)
 func (c *Client) Head(url string) (r *Response, err os.Error) {
-	return c.sendFollowingRedirects("HEAD", url)
+	req, err := NewRequest("HEAD", url, nil)
+	if err != nil {
+		return nil, err
+	}
+	return c.doFollowingRedirects(req)
 }
diff --git a/src/pkg/http/client_test.go b/src/pkg/http/client_test.go
index 6adc9a88363..0869015b38c 100644
--- a/src/pkg/http/client_test.go
+++ b/src/pkg/http/client_test.go
@@ -98,13 +98,20 @@ func TestRedirects(t *testing.T) {
 	c := &Client{}
 	_, err := c.Get(ts.URL)
 	if e, g := "Get /?n=10: stopped after 10 redirects", fmt.Sprintf("%v", err); e != g {
-		t.Errorf("with default client, expected error %q, got %q", e, g)
+		t.Errorf("with default client Get, expected error %q, got %q", e, g)
 	}
 
 	// HEAD request should also have the ability to follow redirects.
 	_, err = c.Head(ts.URL)
 	if e, g := "Head /?n=10: stopped after 10 redirects", fmt.Sprintf("%v", err); e != g {
-		t.Errorf("with default client, expected error %q, got %q", e, g)
+		t.Errorf("with default client Head, expected error %q, got %q", e, g)
+	}
+
+	// Do should also follow redirects.
+	greq, _ := NewRequest("GET", ts.URL, nil)
+	_, err = c.Do(greq)
+	if e, g := "Get /?n=10: stopped after 10 redirects", fmt.Sprintf("%v", err); e != g {
+		t.Errorf("with default client Do, expected error %q, got %q", e, g)
 	}
 
 	var checkErr os.Error