This is another attempt at the change attempted in
https://golang.org/cl/27117 and rolled back in https://golang.org/cl/34134
The difference between this and the previous attempt is that this version only
retries if the new field GetBody is set on the Request.
Additionally, this allows retries of requests with idempotent methods even if
they have bodies, as long as GetBody is defined.
This also fixes an existing bug where readLoop could make a redundant call to
setReqCanceler for DELETE/POST/PUT/etc requests with no body with zero bytes
written.
This clarifies the existing TestRetryIdempotentRequestsOnError test (and changes
it into a test with 4 subtests). When that test was written, it was in fact
testing "retry idempotent requests" logic, but the logic had changed since then,
and it was actually testing "retry requests with no body when no bytes have been
written". (You can confirm this by changing the existing test from a GET to a
DELETE; it passes without the changes in this CL.) We now test for the no-Body
and GetBody cases for both idempotent and nothing-written-non-idempotent
requests.
Fixes#18241Fixes#17844
Change-Id: I69a48691796f6dc08c31f7aa7887b7dfd67e278a
Reviewed-on: https://go-review.googlesource.com/42142
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Despite the previously known behavior of Request.WithContext
shallow copying a request, usage of the request inside server.ServeHTTP
mutates the request's URL. This CL implements deep copying of the URL.
Fixes#20068
Change-Id: I86857d7259e23ac624d196401bf12dde401c42af
Reviewed-on: https://go-review.googlesource.com/41308
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Custom logic from request.go has been removed.
Created by running: “go run gen.go -core” from x/text
at fc7fa097411d30e6708badff276c4c164425590c.
Fixesgolang/go#17268
Change-Id: Ie440d6ae30288352283d303e5126e5837f11bece
Reviewed-on: https://go-review.googlesource.com/37111
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If I put a 10 millisecond sleep at testHookWaitResLoop, before the big
select in (*persistConn).roundTrip, two flakes immediately started
happening, TestTransportBodyReadError (#19231) and
TestTransportPersistConnReadLoopEOF.
The problem was that there are many ways for a RoundTrip call to fail
(errors reading from Request.Body while writing the response, errors
writing the response, errors reading the response due to server
closes, errors due to servers sending malformed responses,
cancelations, timeouts, etc.), and many of those failures then tear
down the TCP connection, causing more failures, since there are always
at least three goroutines involved (reading, writing, RoundTripping).
Because the errors were communicated over buffered channels to a giant
select, the error returned to the caller was a function of which
random select case was called, which was why a 10ms delay before the
select brought out so many bugs. (several fixed in my previous CLs the past
few days).
Instead, track the error explicitly in the transportRequest, guarded
by a mutex.
In addition, this CL now:
* differentiates between the two ways writing a request can fail: the
io.Copy reading from the Request.Body or the io.Copy writing to the
network. A new io.Reader type notes read errors from the
Request.Body. The read-from-body vs write-to-network errors are now
prioritized differently.
* unifies the two mapRoundTripErrorFromXXX methods into one
mapRoundTripError method since their logic is now the same.
* adds a (*Request).WithT(*testing.T) method in export_test.go, usable
by tests, to call t.Logf at points during RoundTrip. This is disabled
behind a constant except when debugging.
* documents and deflakes TestClientRedirectContext
I've tested this CL with high -count values, with/without -race,
with/without delays before the select, etc. So far it seems robust.
Fixes#19231 (TestTransportBodyReadError flake)
Updates #14203 (source of errors unclear; they're now tracked more)
Updates #15935 (document Transport errors more; at least understood more now)
Change-Id: I3cccc3607f369724b5344763e35ad2b7ea415738
Reviewed-on: https://go-review.googlesource.com/37495
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
In Go 1.8, we'd removed the Transport's Request.Body
one-byte-Read-sniffing to disambiguate between non-nil Request.Body
with a ContentLength of 0 or -1. Previously, we tried to see whether a
ContentLength of 0 meant actually zero, or just an unset by reading a
single byte of the Request.Body and then stitching any read byte back
together with the original Request.Body.
That historically has caused many problems due to either data races,
blocking forever (#17480), or losing bytes (#17071). Thus, we removed
it in both HTTP/1 and HTTP/2 in Go 1.8. Unfortunately, during the Go
1.8 beta, we've found that a few people have gotten bitten by the
behavior change on requests with methods typically not containing
request bodies (e.g. GET, HEAD, DELETE). The most popular example is
the aws-go SDK, which always set http.Request.Body to a non-nil value,
even on such request methods. That was causing Go 1.8 to send such
requests with Transfer-Encoding chunked bodies, with zero bytes,
confusing popular servers (including but limited to AWS).
This CL partially reverts the no-byte-sniffing behavior and restores
it only for GET/HEAD/DELETE/etc requests, and only when there's no
Transfer-Encoding set, and the Content-Length is 0 or -1.
Updates #18257 (aws-go) bug
And also private bug reports about non-AWS issues.
Updates #18407 also, but haven't yet audited things enough to declare
it fixed.
Change-Id: Ie5284d3e067c181839b31faf637eee56e5738a6a
Reviewed-on: https://go-review.googlesource.com/34668
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The introduction of NoBody and related body-peeking bug fixes also
added a "cleanup" of sorts to make NewRequest set the returned
Requests's ContentLength to -1 when it didn't know it.
Using -1 to mean unknown is what the documentation says, but then
people apparently(?) depended on it being zero so they could do this:
req, _ := http.NewRequest("POST", url, someNonNilReaderWithUnkownSize)
req.Body = nil
res, err := http.DefaultClient.Do(req)
... and expect it to work.
After https://golang.org/cl/31445 the contrived(?) code above stopped
working, since Body was nil and ContentLength was -1, which has been
disallowed since Go 1.0.
So this restores the old behavior of NewRequest, not setting it to -1.
That part of the fix isn't required as of https://golang.org/cl/31726
(which added NoBody)
I still don't know whether this bug is hypothetical or actually
affected people in practice.
Let's assume it's real for now.
Fixes#18117
Change-Id: I42400856ee92a1a4999b5b4668bef97d885fbb53
Reviewed-on: https://go-review.googlesource.com/33801
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
NoBody is new in Go 1.8.
Found while investigating #18117
Change-Id: I6bda030f358e2270f090d108cb3a89c8a2665fcb
Reviewed-on: https://go-review.googlesource.com/33714
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reverts https://golang.org/cl/23672 and tweaks the text to clarify
HTTP/2 request cancelations also cancel the context (not just closing
the TCP conn).
Fixes#18143
Change-Id: I9f838e09b906d455c98f676e5bc5559f8f7ecb17
Reviewed-on: https://go-review.googlesource.com/33769
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Clean up & document the ProtocolError gunk.
Fixes#17558
Change-Id: I5e54c25257907c9cac7433f7a5bdfb176e8c3eee
Reviewed-on: https://go-review.googlesource.com/33096
Reviewed-by: Ian Lance Taylor <iant@golang.org>
- Removes a subject-verb disagreement.
- Documents that PATCH requests also populate PostForm.
- Explains that r.PostForm is always set (but blank for GET etc.).
Fixes#16609
Change-Id: I6b4693f8eb6db7c66fd9b9cd1df8927f50d46d50
Reviewed-on: https://go-review.googlesource.com/32091
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is an alternate solution to https://golang.org/cl/31445
Instead of making NewRequest return a request with Request.Body == nil
to signal a zero byte body, add a well-known variable that means
explicitly zero.
Too many tests inside Google (and presumably the outside world)
broke.
Change-Id: I78f6ecca8e8aa1e12179c234ccfb6bcf0ee29ba8
Reviewed-on: https://go-review.googlesource.com/31726
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
This CL makes NewRequest set Body nil for known-zero bodies, and makes
the http1 Transport not peek-Read a byte to determine whether there's
a body.
Background:
Many fields of the Request struct have different meanings for whether
they're outgoing (via the Transport) or incoming (via the Server).
For outgoing requests, ContentLength and Body are documented as:
// Body is the request's body.
//
// For client requests a nil body means the request has no
// body, such as a GET request. The HTTP Client's Transport
// is responsible for calling the Close method.
Body io.ReadCloser
// ContentLength records the length of the associated content.
// The value -1 indicates that the length is unknown.
// Values >= 0 indicate that the given number of bytes may
// be read from Body.
// For client requests, a value of 0 with a non-nil Body is
// also treated as unknown.
ContentLength int64
Because of the ambiguity of what ContentLength==0 means, the http1 and
http2 Transports previously Read the first byte of a non-nil Body when
the ContentLength was 0 to determine whether there was an actual body
(with a non-zero length) and ContentLength just wasn't populated, or
it was actually empty.
That byte-sniff has been problematic and gross (see #17480, #17071)
and was removed for http2 in a previous commit.
That means, however, that users doing:
req, _ := http.NewRequest("POST", url, strings.NewReader(""))
... would not send a Content-Length header in their http2 request,
because the size of the reader (even though it was known, being one of
the three common recognized types from NewRequest) was zero, and so
the HTTP Transport thought it was simply unset.
To signal explicitly-zero vs unset-zero, this CL changes NewRequest to
signal explicitly-zero by setting the Body to nil, instead of the
strings.NewReader("") or other zero-byte reader.
This CL also removes the byte sniff from the http1 Transport, like
https://golang.org/cl/31326 did for http2.
Updates #17480
Updates #17071
Change-Id: I329f02f124659bf7d8bc01e2c9951ebdd236b52a
Reviewed-on: https://go-review.googlesource.com/31445
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit 59320c396e6448132a52cb5a5d96491eee1e0ad8.
Reasons:
This CL was causing failures on a large regression test that we run
within Google. The issues arises from two bugs in the CL:
* The CL dropped support for ';' as a delimiter (see https://golang.org/issue/2210)
* The handling of an empty string caused an empty record to be added when
no record was added (see https://golang.org/cl/30454 for my attempted fix)
The logic being added is essentially a variation of url.ParseQuery,
but altered to accept an io.Reader instead of a string.
Since it is duplicated (but modified) logic, there needs to be good
tests to ensure that it's implementation doesn't drift in functionality
from url.ParseQuery. Fixing the above issues and adding the associated
regression tests leads to >100 lines of codes.
For a 4% reduction in CPU time, I think this complexity and duplicated
logic is not worth the effort.
As such, I am abandoning my efforts to fix the existing issues and
believe that reverting CL/20301 is the better course of action.
Updates #14655
Change-Id: Ibb5be0a5b48a16c46337e213b79467fcafee69df
Reviewed-on: https://go-review.googlesource.com/30470
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Remove the use of io.ReadAll in http.parsePostForm to avoid converting
the whole input from []byte to string and not performing well
space-allocated-wise.
Instead a new function called parsePostFormURLEncoded is used and is
fed directly an io.Reader that is parsed using a bufio.Reader.
Benchmark:
name old time/op new time/op delta
PostQuery-4 2.90µs ± 6% 2.82µs ± 4% ~ (p=0.094 n=9+9)
name old alloc/op new alloc/op delta
PostQuery-4 1.05kB ± 0% 0.90kB ± 0% -14.49% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
PostQuery-4 6.00 ± 0% 7.00 ± 0% +16.67% (p=0.000 n=10+10)
Fixes#14655
Change-Id: I112c263d4221d959ed6153cfe88bc57a2aa8ea73
Reviewed-on: https://go-review.googlesource.com/20301
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, update bundled http2 to x/net git rev 0d8126f to include
https://golang.org/cl/30150, the HTTP/2 version of this fix.
Fixes#16002
Change-Id: I8da1ca98250357aec012e3e85c8b13acfa2f3fec
Reviewed-on: https://go-review.googlesource.com/30151
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Code movement only, to look more like the equivalent http2 code, and
to make an upcoming fix look more obvious.
Updates #16002 (to be fixed once this code is in)
Change-Id: Iaa4f965be14e98f9996e7c4624afe6e19bed1a80
Reviewed-on: https://go-review.googlesource.com/30087
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
RFC 6265, section 4.2.2 says:
<<<
Although cookies are serialized linearly in the Cookie header,
servers SHOULD NOT rely upon the serialization order. In particular,
if the Cookie header contains two cookies with the same name (e.g.,
that were set with different Path or Domain attributes), servers
SHOULD NOT rely upon the order in which these cookies appear in the
header.
>>>
This statement seems to indicate that cookies should conceptually
be thought of as a map of keys to sets of values (map[key][]value).
However, in practice, everyone pretty much treats cookies as a
map[key]value and the API for Request.Cookie seems to indicate that.
We should update the documentation for Request.Cookie to warn the
user what happens when there is are multiple cookies with the same
key. I deliberately did not want to say *which* cookie is returned.
Change-Id: Id3e0e24b2b14ca2d9ea8b13f82ba739edaa71cf0
Reviewed-on: https://go-review.googlesource.com/29364
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
While you could argue the previous wording technically said that -1 is
an acceptable way to indicate "unknown" on the client, it could be read
as ambiguous. Now it's clear that both 0 and -1 mean unknown.
Change-Id: I3bc5a3fd5afd1999e487296ec121eb548415e6b0
Reviewed-on: https://go-review.googlesource.com/29130
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL makes code like this work:
res, err := http.Get("https://фу.бар/баз")
So far, IDNA support is limited to the http1 and http2 Transports.
The http package is currently responsible for converting domain names
into Punycode before calling the net layer. The http package also has
to Punycode-ify the hostname for the Host & :authority headers for
HTTP/1 and HTTP/2, respectively.
No automatic translation from Punycode back to Unicode is performed,
per Go's historical behavior. Docs are updated where relevant. No
changes needed to the Server package. Things are already in ASCII
at that point.
No changes to the net package, at least yet.
Updates x/net/http2 to git rev 57c7820 for https://golang.org/cl/29071
Updates #13835
Change-Id: I1e9a74c60d00a197ea951a9505da5c3c3187099b
Reviewed-on: https://go-review.googlesource.com/29072
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The comment on http.Request.Context says that the context
is canceled when the client's connection closes even though
this has not been implemented. See #15927
Change-Id: I50b68638303dafd70f77f8f778e6caff102d3350
Reviewed-on: https://go-review.googlesource.com/23672
Reviewed-by: Andrew Gerrand <adg@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>
Standardize on space between "RFC" and number. Additionally change
the couple "a RFC" instances to "an RFC."
Fixes#15258
Change-Id: I2b17ecd06be07dfbb4207c690f52a59ea9b04808
Reviewed-on: https://go-review.googlesource.com/21902
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently only used by the client. The server is not yet wired up. A
TODO remains to document how it works server-side, once implemented.
Updates #14660
Change-Id: I27c2e74198872b2720995fa8271d91de200e23d5
Reviewed-on: https://go-review.googlesource.com/21496
Reviewed-by: Andrew Gerrand <adg@golang.org>
The http2 spec defines a magic string which initates an http2 session:
"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
It was intentionally chosen to kinda look like an HTTP request, but
just different enough to break things not ready for it. This change
makes Go ready for it.
Notably: Go now accepts the request header (the prefix "PRI *
HTTP/2.0\r\n\r\n") as a valid request, even though it doesn't have a
Host header. But we now mark it as "Connection: close" and teach the
Server to never read a second request from the connection once that's
seen. If the http.Handler wants to deal with the upgrade, it has to
hijack the request, read out the "body", compare it against
"SM\r\n\r\n", and then speak http2. One of the new tests demonstrates
that hijacking.
Fixes#14451
Updates #14141 (h2c)
Change-Id: Ib46142f31c55be7d00c56fa2624ec8a232e00c43
Reviewed-on: https://go-review.googlesource.com/21327
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes sure the net/http package never attempts to transmit a
bogus header field key or value and instead fails fast with an error
to the user, rather than relying on the server to maybe return an
error.
It's still possible to use x/net/http2.Transport directly to send
bogus stuff. This change only stops h1 & h2 usage via the net/http
package. A future change will update x/net/http2.
This change also moves some code from request.go to lex.go, which in a
separate future change should be moved so it can be shared with http2
to reduce code bloat.
Updates #14048
Change-Id: I0a44ae1ab357fbfcbe037aa4b5d50669a87f2856
Reviewed-on: https://go-review.googlesource.com/21326
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I was wondering why cmd/go includes the HTTP server implementations.
Dumping the linker's deadcode dependency graph into a file and doing
some graph analysis, I found that the only reason cmd/go included an
HTTP server was because the maxBytesReader type (used by both the HTTP
transport & HTTP server) did a static type assertion to an HTTP server
type.
Changing it to a interface type assertion reduces the size of cmd/go
by 533KB (5.2%)
On linux/amd64, cmd/go goes from 10549200 to 10002624 bytes.
Add a test too so this doesn't regress. The test uses cmd/go as the
binary to test (a binary which needs the HTTP client but not the HTTP
server), but this change and test are equally applicable to any such
program.
Change-Id: I93865f43ec03b06d09241fbd9ea381817c2909c5
Reviewed-on: https://go-review.googlesource.com/20763
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ensures that after request.ParseMultipartForm has been invoked,
Request.PostForm and Request.Form are both populated with the
same formValues read in, instead of only populating Request.Form.
Fixes#9305
Change-Id: I3d4a11b006fc7dffaa35360014fe15b8c74d00a3
Reviewed-on: https://go-review.googlesource.com/19986
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@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>
Named returned values should only be used on public funcs and methods
when it contributes to the documentation.
Named return values should not be used if they're only saving the
programmer a few lines of code inside the body of the function,
especially if that means there's stutter in the documentation or it
was only there so the programmer could use a naked return
statement. (Naked returns should not be used except in very small
functions)
This change is a manual audit & cleanup of public func signatures.
Signatures were not changed if:
* the func was private (wouldn't be in public godoc)
* the documentation referenced it
* the named return value was an interesting name. (i.e. it wasn't
simply stutter, repeating the name of the type)
There should be no changes in behavior. (At least: none intended)
Change-Id: I3472ef49619678fe786e5e0994bdf2d9de76d109
Reviewed-on: https://go-review.googlesource.com/20024
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
No need to say "by default" because there is no alternative and no way
to override. Always HTTP/2.0 is officially spelled HTTP/2 these days.
Fixes#13985 harder
Change-Id: Ib1ec03cec171ca865342b8e7452cd4c707d7b770
Reviewed-on: https://go-review.googlesource.com/18720
Reviewed-by: Rob Pike <r@golang.org>
Third time's a charm.
Thanks to Ralph Corderoy for noticing the DEL omission.
Update #11207
Change-Id: I174fd01eaecceae1eb220f2c9136e12d40fbe943
Reviewed-on: https://go-review.googlesource.com/18375
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As Andy Balholm noted in #11207:
"RFC2616 §4.2 says that a header's field-content can consist of *TEXT,
and RFC2616 §2.2 says that TEXT is <any OCTET except CTLs, but
including LWS>, so that would mean that bytes greater than 128 are
allowed."
This is a partial rollback of the strictness from
https://golang.org/cl/11207 (added in the Go 1.6 dev cycle, only
released in Go 1.6beta1)
Fixes#11207
Change-Id: I3a752a7941de100e4803ff16a5d626d5cfec4f03
Reviewed-on: https://go-review.googlesource.com/18374
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Adds a test that both http1 and http2's Transport send a default
User-Agent, with the same behavior.
Updates bundled http2 to golang.org/x/net git rev 1ade16a545 (for
https://go-review.googlesource.com/18285)
The http1 behavior changes slightly: if req.Header["User-Agent"] is
defined at all, even if it's nil or a zero-length slice, then the
User-Agent header is omitted. This is a slight behavior change for
http1, but is consistent with how http1 & http2 do optional headers
elsewhere (such as "Date", "Content-Type"). The old behavior (set it
explicitly to "", aka []string{""}) still works as before. And now
there are even tests.
Fixes#13685
Change-Id: I5786a6913b560de4a5f1f90e595fe320ff567adf
Reviewed-on: https://go-review.googlesource.com/18284
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
