[go] net/http: allow Client.CheckRedirect to use most recent response

[Brad Fitzpatrick (Gerrit)]

Reviewers: Andrew Gerrand

Brad Fitzpatrick uploaded a change:
https://go-review.googlesource.com/23207

net/http: allow Client.CheckRedirect to use most recent response

Fixes #10069

Change-Id: I3819ff597d5a0c8e785403bf9d65a054f50655a6
---
M src/net/http/client.go
M src/net/http/request.go
M src/net/http/response.go
3 files changed, 42 insertions(+), 15 deletions(-)



diff --git a/src/net/http/client.go b/src/net/http/client.go
index 1127634..dd9e57f 100644
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -47,6 +47,9 @@
// method returns both the previous Response (with its Body
// closed) and CheckRedirect's error (wrapped in a url.Error)
// instead of issuing the Request req.
+ // As a special case, if CheckRedirect returns ErrUseLastResponse,
+ // the the most recent response is returned with its body
+ // unclosed, along with a nil error.
//
// If CheckRedirect is nil, the Client uses its default policy,
// which is to stop after 10 consecutive requests.
@@ -417,6 +420,12 @@

func alwaysFalse() bool { return false }

+// ErrUseLastResponse can be returned by Client.CheckRedirect hooks to
+// control how redirects are processed. If returned, the next request
+// is not sent and the most recent response is returned with its body
+// unclosed.
+var ErrUseLastResponse = errors.New("net/http: use last response")
+
// checkRedirect calls either the user's configured CheckRedirect
// function, or the default.
func (c *Client) checkRedirect(req *Request, via []*Request) error {
@@ -467,11 +476,12 @@
}
ireq := reqs[0]
req = &Request{
- Method: ireq.Method,
- URL: u,
- Header: make(Header),
- Cancel: ireq.Cancel,
- ctx: ireq.ctx,
+ Method: ireq.Method,
+ Response: resp,
+ URL: u,
+ Header: make(Header),
+ Cancel: ireq.Cancel,
+ ctx: ireq.ctx,
}
if ireq.Method == "POST" || ireq.Method == "PUT" {
req.Method = "GET"
@@ -481,7 +491,27 @@
if ref := refererForURL(reqs[len(reqs)-1].URL, req.URL); ref != "" {
req.Header.Set("Referer", ref)
}
- if err := c.checkRedirect(req, reqs); err != nil {
+ err = c.checkRedirect(req, reqs)
+
+ // Sentinel error to let users select the
+ // previous response, without closing its
+ // body. See Issue 10069.
+ if err == ErrUseLastResponse {
+ return resp, nil
+ }
+
+ // Close the previous response's body. But
+ // read at least some of the body so if it's
+ // small the underlying TCP connection will be
+ // re-used. No need to check for errors: if it
+ // fails, the 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 err != nil {
// Special case for Go 1 compatibility: return both the response
// and an error if the CheckRedirect function failed.
// See https://golang.org/issue/3795
@@ -508,14 +538,6 @@
if !shouldRedirect(resp.StatusCode) {
return resp, nil
}
-
- // 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()
}
}

diff --git a/src/net/http/request.go b/src/net/http/request.go
index 45507d2..e8780de 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -255,6 +255,11 @@
// set, it is undefined whether Cancel is respected.
Cancel <-chan struct{}

+ // Response is the redirect response which caused this request
+ // to be created. This field is only populated during client
+ // redirects.
+ Response *Response
+
// ctx is either the client or server context. It should only
// be modified via copying the whole Request using WithContext.
// It is unexported to prevent people from using Context wrong
diff --git a/src/net/http/response.go b/src/net/http/response.go
index 0164a09..979651c 100644
--- a/src/net/http/response.go
+++ b/src/net/http/response.go
@@ -96,7 +96,7 @@
// any trailer values sent by the server.
Trailer Header

- // The Request that was sent to obtain this Response.
+ // Request is the request that was sent to obtain this Response.
// Request's Body is nil (having already been consumed).
// This is only populated for Client requests.
Request *Request

--
https://go-review.googlesource.com/23207
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick uploaded a new patch set:

https://go-review.googlesource.com/23207

net/http: allow Client.CheckRedirect to use most recent response

Fixes #10069

Change-Id: I3819ff597d5a0c8e785403bf9d65a054f50655a6
---
M src/net/http/client.go

M src/net/http/client_test.go
M src/net/http/request.go
M src/net/http/response.go
4 files changed, 80 insertions(+), 15 deletions(-)

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has posted comments on this change.

net/http: allow Client.CheckRedirect to use most recent response

Patch Set 2: Run-TryBot+1

--
https://go-review.googlesource.com/23207
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>

Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

net/http: allow Client.CheckRedirect to use most recent response

Patch Set 2:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=923d6455

--
https://go-review.googlesource.com/23207
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>

Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

net/http: allow Client.CheckRedirect to use most recent response

Patch Set 2: TryBot-Result+1

TryBots are happy.

--
https://go-review.googlesource.com/23207
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

net/http: allow Client.CheckRedirect to use most recent response

Patch Set 2: Code-Review+2

(1 comment)

https://go-review.googlesource.com/#/c/23207/2/src/net/http/client.go
File src/net/http/client.go:

Line 51: // the the most recent response is returned with its body
then the

--
https://go-review.googlesource.com/23207
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>

Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Russ Cox <r...@golang.org>
Gerrit-HasComments: Yes

[Brad Fitzpatrick (Gerrit)]

Reviewers: Russ Cox, Gobot Gobot

Brad Fitzpatrick uploaded a new patch set:

https://go-review.googlesource.com/23207

net/http: allow Client.CheckRedirect to use most recent response

Fixes #10069

Change-Id: I3819ff597d5a0c8e785403bf9d65a054f50655a6
---
M src/net/http/client.go

M src/net/http/client_test.go
M src/net/http/request.go
M src/net/http/response.go
4 files changed, 80 insertions(+), 15 deletions(-)

--
https://go-review.googlesource.com/23207
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has submitted this change and it was merged.

net/http: allow Client.CheckRedirect to use most recent response

Fixes #10069

Change-Id: I3819ff597d5a0c8e785403bf9d65a054f50655a6

Reviewed-on: https://go-review.googlesource.com/23207
Reviewed-by: Russ Cox <r...@golang.org>

---
M src/net/http/client.go
M src/net/http/client_test.go
M src/net/http/request.go
M src/net/http/response.go
4 files changed, 80 insertions(+), 15 deletions(-)

Approvals:
Russ Cox: Looks good to me, approved

--
https://go-review.googlesource.com/23207
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>