The godoc for RoundTrip already specifies when it's ok to reuse a
request that contains a body: the caller must wait until RoundTrip
calls Close on Request.Body.
This CL adds a small clarification: If the request does not have a
body, it can be reused as long as the caller does not mutate the
Request until RoundTrip fails or the Response.Body is closed.
Fixes#19653
Change-Id: I56652a9369978d11650e2e6314104831c2ce5e78
Reviewed-on: https://go-review.googlesource.com/75671
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If the client sends a request with a custom Host header and receives
a relative redirect in response, the second request should use the
same Host header as the first request. However, if the response is
an abolute redirect, the Host header should not be preserved. See
further discussion on the issue tracker.
Fixes#22233
Change-Id: I8796e2fbc1c89b3445e651f739d5d0c82e727c14
Reviewed-on: https://go-review.googlesource.com/70792
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: Ic3f7f575d3640706adb7d64545ed8027add6c58f
Reviewed-on: https://go-review.googlesource.com/61350
Run-TryBot: Tom Bergan <tombergan@google.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
Add a test and fix for the request body being closed twice.
Fixes#19186
Change-Id: I1e35ad4aebfef68e6099c1dba7986883afdef4d7
Reviewed-on: https://go-review.googlesource.com/37298
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Brad noticed a bullet list was rendered as preformatted text because of
the indentation. One can use a unicode bullet as an ersatz for bullet
lists.
Fixes#20043
Change-Id: Iaed3582d14bd05920455669039a900d7155960d9
Reviewed-on: https://go-review.googlesource.com/41212
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The presence of Request.GetBody being set on a request was causing all
redirected requests to have a body, even if the redirect status didn't
warrant one.
This bug came from 307/308 support (https://golang.org/cl/29852) which
removed the line that set req.Body to nil after POST/PUT redirects.
Change-Id: I2a4dd5320f810ae25cfd8ea8ca7c9700e5dbd369
Reviewed-on: https://go-review.googlesource.com/35633
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
In Go1.7, a 301, 302, or 303 redirect on a HEAD method, would still
cause the following redirects to still use a HEAD.
In CL/29852 this behavior was changed such that those codes always
caused a redirect with the GET method. Fix this such that both
GET and HEAD will preserve the method.
Fixes#18570
Change-Id: I4bfe69872a30799419e3fad9178f907fe439b449
Reviewed-on: https://go-review.googlesource.com/34981
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The client code was using time.Now() (wall time) to determine whether
the cause of a non-nil error meant that a timeout had occured. But on
Windows, the clock used for timers (time.After, time.Sleep, etc) is
much more accurate than the time.Now clock, which doesn't update
often.
But it turns out that as of the recent https://golang.org/cl/32478 we
already have the answer available easily. It just wasn't in scope.
Instead of passing this information along by decorating the errors
(risky this late in Go 1.8, especially with #15935 unresolved), just
passing along the "didTimeout" func internally for now. We can remove
that later in Go 1.9 if we overhaul Transport errors.
Fixes#18287 (I hope)
Change-Id: Icbbfceaf02de6c7ed04fe37afa4ca16374b58f3c
Reviewed-on: https://go-review.googlesource.com/34381
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
And move some code to make control flow more obvious.
No functional change.
Change-Id: Iefaa96f664070ab2accade1857e1946e56df6902
Reviewed-on: https://go-review.googlesource.com/34285
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL tweaks the new (unreleased) 307/308 support added in
https://golang.org/cl/29852 for #10767.
Change 1: if a 307/308 response doesn't have a Location header in its
response (as observed in the wild in #17773), just do what we used to
do in Go 1.7 and earlier, and don't try to follow that redirect.
Change 2: don't follow a 307/308 if we sent a body on the first
request and the caller's Request.GetBody func is nil so we can't
"rewind" the body to send it again.
Updates #17773 (will be fixed more elsewhere)
Change-Id: I183570f7346917828a4b6f7f1773094122a30406
Reviewed-on: https://go-review.googlesource.com/32595
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Provides redirection support for 307, 308 server statuses.
Provides redirection support for DELETE method.
Updates old tests that assumed all redirects were treated
the way 301, 302 and 303 are processed.
Fixes#9348Fixes#10767Fixes#13994
Change-Id: Iffa8dbe0ff28a1afa8da59869290ec805b1dd2c4
Reviewed-on: https://go-review.googlesource.com/29852
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In the situation where the Client.Jar is set and the Request.Header
has cookies manually inserted, the redirect logic needs to be
able to apply changes to cookies from "Set-Cookie" headers to both
the Jar and the manually inserted Header cookies.
Since Header cookies lack information about the original domain
and path, the logic in this CL simply removes cookies from the
initial Header if any subsequent "Set-Cookie" matches. Thus,
in the event of cookie conflicts, the logic preserves the behavior
prior to change made in golang.org/cl/28930.
Fixes#17494
Updates #4800
Change-Id: I645194d9f97ff4d95bd07ca36de1d6cdf2f32429
Reviewed-on: https://go-review.googlesource.com/31435
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Copy all of the original request's headers on redirect, unless they're
sensitive. Only send sensitive ones to the same origin, or subdomains
thereof.
Fixes#4800
Change-Id: Ie9fa75265c9d5e4c1012c028d31fd1fd74465712
Reviewed-on: https://go-review.googlesource.com/28930
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Francesc Campoy Flores <campoy@golang.org>
Reviewed-by: Ross Light <light@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
- Ensures that the empty port and preceeding ":"
in a URL.Host are stripped.
Normalize the empty port in a URL.Host's ":port" as
mandated by RFC 3986 Section 6.2.3 which states that:
`Likewise an explicit ":port", for which the port is empty or
the default for the scheme, is equivalent to one where the port
and its ":" delimiter are elided and thus should be
removed by scheme-based normalization.`
- Moves function `hasPort` from client.go (where it was defined but
not used directly), to http.go the common area.
Fixes#14836
Change-Id: I2067410377be9c71106b1717abddc2f8b1da1c03
Reviewed-on: https://go-review.googlesource.com/22140
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Issue #8633 (and #9134) noted that we didn't document the rules about
closing the Response.Body when Client.Do returned both a non-nil
*Response and a non-nil error (which can only happen when the user's
CheckRedirect returns an error).
In the process of investigating, I cleaned this code up a bunch, but
no user-visible behavior should have changed, except perhaps some
better error messages in some cases.
It turns out it's always been the case that when a CheckRedirect error
occurs, the Response.Body is already closed. Document that.
And the new code makes that more obvious too.
Fixes#8633
Change-Id: Ibc40cc786ad7fc4e0cf470d66bb559c3b931684d
Reviewed-on: https://go-review.googlesource.com/21364
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
On HTTP redirect, the HTTP client creates a new request and don't copy
over the Cancel channel. This prevents any redirected request from being
cancelled.
Fixes#14053
Change-Id: I467cdd4aadcae8351b6e9733fc582b7985b8b9d3
Reviewed-on: https://go-review.googlesource.com/18810
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
In the beginning, there was no way to cancel an HTTP request.
We later added Transport.CancelRequest to cancel an in-flight HTTP
request by breaking its underlying TCP connection, but it was hard to
use correctly and didn't work in all cases. And its error messages
were terrible. Some of those issues were fixed over time, but the most
unfixable problem was that it didn't compose well. All RoundTripper
implementations had to choose to whether to implement CancelRequest
and both decisions had negative consequences.
In Go 1.5 we added Request.Cancel, which composed well, worked in all
phases, had nice error messages, etc. But we forgot to use it in the
implementation of Client.Timeout (a timeout which spans multiple
requests and reading request bodies).
In Go 1.6 (upcoming), we added HTTP/2 support, but now Client.Timeout
didn't work because the http2.Transport didn't have a CancelRequest
method.
Rather than add a CancelRequest method to http2, officially deprecate
it and update the only caller (Client, for Client.Cancel) to use
Request.Cancel instead.
The http2 Client timeout tests are enabled now.
For compatibility, we still use CancelRequest in Client if we don't
recognize the RoundTripper type. But documentation has been updated to
tell people that CancelRequest is deprecated.
Fixes#13540
Change-Id: I15546b90825bb8b54905e17563eca55ea2642075
Reviewed-on: https://go-review.googlesource.com/18260
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Patch from Russ.
No bug identified, but I didn't search exhaustively. The new code is
easier to read.
Fixes#13621
Change-Id: Ifda936e4101116fa254ead950b5fe06adb14e977
Reviewed-on: https://go-review.googlesource.com/17981
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Inspect the crypto/tls error to recognize this case and give a more
helpful error.
Fixes#11111.
Change-Id: I63f6af8c375aa892326ccccbd29655d54d68df0b
Reviewed-on: https://go-review.googlesource.com/16079
Run-TryBot: Caleb Spare <cespare@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The one in misc/makerelease/makerelease.go is particularly bad and
probably warrants rotating our keys.
I didn't update old weekly notes, and reverted some changes involving
test code for now, since we're late in the Go 1.5 freeze. Otherwise,
the rest are all auto-generated changes, and all manually reviewed.
Change-Id: Ia2753576ab5d64826a167d259f48a2f50508792d
Reviewed-on: https://go-review.googlesource.com/12048
Reviewed-by: Rob Pike <r@golang.org>
Instead of ambiguously referring to "the Client's CheckRedirect
function" in Head, describe the default behavior like for Get as users
aren't expected to change DefaultClient.CheckRedirect.
While here, use consistent punctuation for the Get and Head Client
method documentation.
Change-Id: I9e7046c73b0d0bc4de002234924d9e7c59aceb41
Reviewed-on: https://go-review.googlesource.com/11362
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes#10366 (how to set custom headers)
Fixes#9836 (PATCH in PostForm)
Fixes#9276 (generating a server-side Request for testing)
Update #8991 (clarify Response.Write for now; export ReverseProxy's copy later?)
Change-Id: I95a11bf3bb3eeeeb72775b6ebfbc761641addc35
Reviewed-on: https://go-review.googlesource.com/9410
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Co-hacking with Dave Cheney.
Fixes#9405
Change-Id: I14fc3b6a47dcdb5e514e93d062b804bb24e89f47
Reviewed-on: https://go-review.googlesource.com/1875
Reviewed-by: Dave Cheney <dave@cheney.net>
http.Client calls URL.String() to fill in the Referer header, which may
contain authentication info. This patch removes authentication info from
the Referer header without introducing any API changes.
A new test for net/http is also provided.
This is the polished version of Alberto García Hierro's
https://golang.org/cl/9766046/
It should handle https Referer right.
Fixes#8417
LGTM=bradfitz
R=golang-codereviews, gobot, bradfitz, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/151430043
