Add an explicit way for Handlers to abort their response to the client
and also not spam their error log with stack traces.
panic(nil) also worked in the past (for http1 at least), so continue
to make that work (and test it). But ErrAbortHandler is more explicit.
Updates #17790 (needs http2 updates also)
Change-Id: Ib1456905b27e2ae8cf04c0983dc73e314a4a751e
Reviewed-on: https://go-review.googlesource.com/33099
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@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>
Server.TLSNextProto being nil is necessary but not sufficient but
http2 being automatically enabled.
Fixes#16588
Change-Id: I5b18690582f9b12ef05b58235e1eaa52483be285
Reviewed-on: https://go-review.googlesource.com/33090
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fix another case of a parallel test relying on a global variable
(DefaultTransport) implicitly.
Use the private Transport already in scope instead. It's closed at the
end, instead of randomly via another test.
Change-Id: I95e51926177ad19a766cabbb306782ded1bbb59b
Reviewed-on: https://go-review.googlesource.com/32913
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A few tests were using the global DefaultTransport implicitly.
Be explicit instead. And then make some parallel while I'm there.
Change-Id: I3c617e75429ecc8f6d23567d1470f5e5d0cb7cfd
Reviewed-on: https://go-review.googlesource.com/32758
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Before: 8.9 seconds for go test -short
After: 2.8 seconds
There are still 250 tests without t.Parallel, but I got the important
onces using a script:
$ go test -short -v 2>&1 | go run ~/slowtests.go
Where slowtests.go is https://play.golang.org/p/9mh5Wg1nLN
The remaining 250 (the output lines from slowtests.go) all have a
reported duration of 0ms, except one 50ms test which has to be serial.
Where tests can't be parallel, I left a comment at the top saying why,
so people don't add t.Parallel later and get surprised at failures.
Updates #17751
Change-Id: Icbe32cbe2b996e23c89f1af6339287fa22af5115
Reviewed-on: https://go-review.googlesource.com/32684
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
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>
- Added support for If-Match and If-Unmodified-Since.
- Precondition checks now more strictly follow RFC 7232 section 6, which
affects precedence when multiple condition headers are present.
- When serving a 304, Last-Modified header is now removed when no ETag is
present (as suggested by RFC 7232 section 4.1).
- If-None-Match supports multiple ETags.
- ETag comparison now correctly handles weak ETags.
Fixes#17572
Change-Id: I35039dea6811480ccf2889f8ed9c6a39ce34bfff
Reviewed-on: https://go-review.googlesource.com/32014
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Since Dial(":80") was implemented on Plan 9 (CL 32593),
TestProtocolDialError is failing because dialing a port
superior to 65535 is supported on Plan 9.
This change disallows dialing and listening on ports
superior to 65535.
Fixes#17761.
Change-Id: I95e8a163eeacf1ccd8ece7b650f16a0531c59709
Reviewed-on: https://go-review.googlesource.com/32594
Reviewed-by: Russ Cox <rsc@golang.org>
CL 32101 fixed Dial(":80") on Windows and added TestDialLocal,
which was failing on Plan 9, because it wasn't implemented
on Plan 9.
This change implements Dial(":80") by connecting to 127.0.0.1
or ::1 (depending on network), so it works as documented.
Fixes#17760.
Change-Id: If0ff769299e09bebce11fc3708639c1d8c96c280
Reviewed-on: https://go-review.googlesource.com/32593
Reviewed-by: Russ Cox <rsc@golang.org>
Windows sockets allow bind to 0.0.0.0:80 but not connect to it.
To make Listen(":80") / Dial(":80") work as documented on Windows,
connect to 127.0.0.1 or ::1 (depending on network) in place of 0.0.0.0.
Fixes#6290.
Change-Id: Ia27537067276871648546678fbe0f1b8478329fe
Reviewed-on: https://go-review.googlesource.com/32101
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Fixes#13290
Change-Id: I0f7e7683d86db501cbedb6a0b7349ceb0769701c
Reviewed-on: https://go-review.googlesource.com/32481
Reviewed-by: Martin Möhrmann <martisch@uos.de>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Go's http1 implementation originally had a mechanism to send HTTP
trailers based on pre-declaring the trailer keys whose values you'd
later let after the header was written.
http2 copied the same mechanism, but it was found to be unsufficient
for gRPC's wire protocol. A second trailer mechanism was added later
(but only to http2) for handlers that want to send a trailer without
knowing in advance they'd need to.
Copy the same mechanism back to http1 and document it.
Fixes#15754
Change-Id: I8c40d55e28b0e5b7087d3d1a904a392c56ee1f9b
Reviewed-on: https://go-review.googlesource.com/32479
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Adds ModifyResponse, an optional func to ReverseProxy
that modifies a response in the backend, right before
the headers of the response are written to the internal
response writer.
If ModifyResponse returns an error, the proxy returns
a StatusBadGateway error.
Fixes#14237.
Change-Id: I8e03139e34dea0084512ccbd8cc49e941bf9fb5d
Reviewed-on: https://go-review.googlesource.com/32356
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also updates x/net/http2 to git rev 541150 for:
http2: add support for graceful shutdown of Server
https://golang.org/cl/32412
http2: make http2.Server access http1's Server via an interface check
https://golang.org/cl/32417Fixes#4674Fixes#9478
Change-Id: I8021a18dee0ef2fe3946ac1776d2b10d3d429052
Reviewed-on: https://go-review.googlesource.com/32329
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
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>
Following RFC 6265 Section 5.1.1.5, ensure that the minimum
year for which an Expires value is valid and can be included in
the cookie's string, is 1601 instead of the Epoch year 1970.
A detailed specification for parsing the Expiry field is at:
https://tools.ietf.org/html/rfc6265#section-5.2.1
I stumbled across this bug due to this StackOverflow answer
that recommends setting the Expiry to the Epoch:
http://stackoverflow.com/a/5285982Fixes#17632
Change-Id: I3c1bdf821d369320334a5dc1e4bf22783cbfe9fc
Reviewed-on: https://go-review.googlesource.com/32142
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 12905 disallowed "Bob" <""@example.com> but inadvertently
also disallowed "" <bob@example.com>. Move the empty string
check to apply only in the addr-spec.
Fixes#14866.
Change-Id: Ia0b7a1a32810aa78157ae77bd0130b78154c460d
Reviewed-on: https://go-review.googlesource.com/32176
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The receiver itself is not transmitted and does not need to be
marshalable by encoding/gob.
Fixes#16803.
Change-Id: I42a3603fb7d3b36c97dcc2e51a398cd65ec3227d
Reviewed-on: https://go-review.googlesource.com/32094
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@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 issue has been fixed in CL 31271.
Fixes#8908.
Change-Id: I8015490e2d992e09c664560e42188315e0e0669e
Reviewed-on: https://go-review.googlesource.com/32150
Run-TryBot: David du Colombier <0intro@gmail.com>
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>
In general, these functions cannot behave correctly when given a
hostname, because a hostname may represent multiple IP addresses, and
first(isIPv4) chooses at most one.
Updates #9334
Change-Id: Icfb629f84af4d976476385a3071270253c0000b1
Reviewed-on: https://go-review.googlesource.com/31931
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 31468 added TestLookupNonLDH, which was failing on Plan 9,
because LookupHost was expecting to return errNoSuchHost
on DNS resolution failure, while Plan 9 returned the
"dns failure" string.
In the Plan 9 implementation of lookupHost, we now return
errNoSuchHost instead of the "dns failure" string, so
the behavior is more consistant with other operating systems.
Fixes#17568.
Change-Id: If64f580dc0626a4a4f19e5511ba2ca5daff5f789
Reviewed-on: https://go-review.googlesource.com/31873
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Since CL 30614, TestCloseError is failing on Plan 9,
because File.Write now checks f.fd == badFd before
calling syscall.Write.
The f.fd == badFd check returns os.ErrClosed, while
syscall.Write returned a syscall.ErrorString error.
TestCloseError was failing because it expected a
syscall.ErrorString error.
We add a case in parseCloseError to handle the
os.ErrClosed case.
Fixes#17569.
Change-Id: I6b4d956d18ed6d3c2ac5211ffd50a4888f7521e1
Reviewed-on: https://go-review.googlesource.com/31872
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes#16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
Reviewed-on: https://go-review.googlesource.com/31582
Reviewed-by: Quentin Smith <quentin@golang.org>
The Go resolver reports invalid domain name for '!!!.local',
but that is allowed by multicast DNS. In general we can't predict
what future relaxations might come along, and libc resolvers
do not distinguish 'no such host' from 'invalid name', so stop
making that distinction here too. Always use 'no such host'.
Fixes#12421.
Change-Id: I8f22604767ec9e270434e483da52b337833bad71
Reviewed-on: https://go-review.googlesource.com/31468
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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 makes it possible to use URLs with gob.
Ideally we'd also implement TextMarshaler and TextUnmarshaler,
but that would change the JSON encoding of a URL from something like:
{"Scheme":"https","Opaque":"","User":null,"Host":"www.google.com","Path":"/x","RawPath":"","ForceQuery":false,"RawQuery":"y=z","Fragment":""}
to something like:
"https://www.google.com/x?y=z"
That'd be nice, but it would break code expecting the old form.
Fixes#10964.
Change-Id: I83f06bc2bedd2ba8a5d8eef03ea0056d045c258f
Reviewed-on: https://go-review.googlesource.com/31467
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that we have the Clone method on tls.Config, net/http doesn't need
any custom functions to do that any more.
Change-Id: Ib60707d37f1a7f9a7d7723045f83e59eceffd026
Reviewed-on: https://go-review.googlesource.com/31595
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates http2 to x/net/http2 git rev 40a0a18 for:
http2: fix Server race with concurrent Read/Close
http2: make Server reuse 64k request body buffer between requests
http2: never Read from Request.Body in Transport to determine ContentLength
Fixes#17480
Updates #17071
Change-Id: If142925764a2e148f95957f559637cfc1785ad21
Reviewed-on: https://go-review.googlesource.com/31737
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The old wording over-promised.
Fixes#16957
Change-Id: Iaac04de0d24eb17a0db66beeeab9de70d0f6d391
Reviewed-on: https://go-review.googlesource.com/31735
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
This issue has been fixed in CL 31390.
Fixes#9554.
Change-Id: Ib8ff4cb1ffcb7cdbf117510b98b4a7e13e4efd2b
Reviewed-on: https://go-review.googlesource.com/31520
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
The way to send an explicitly-zero Content-Length is to set a nil Body.
Fix this test to do that, rather than relying on type sniffing.
Updates #17480
Updates #17071
Change-Id: I6a38e20f17013c88ec4ea69d73c507e4ed886947
Reviewed-on: https://go-review.googlesource.com/31434
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
This issue has been fixed in CL 31390.
Fixes#11476.
Change-Id: I6658bda2e494d3239d62c49d0bd5d34a36b744d0
Reviewed-on: https://go-review.googlesource.com/31394
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously, in acceptPlan9 we set netFD.ctl to the listener's
/net/tcp/*/listen file instead of the accepted connection's
/net/tcp/*/ctl file.
In netFD.Read, we write "close" to netFD.ctl to close the
connection and wake up the readers. However, in the
case of an accepted connection, we got the error
"write /net/tcp/*/listen: inappropriate use of fd"
because the /net/tcp/*/listen doesn't handle the "close" message.
In this case, the connection wasn't closed and the readers
weren't awake.
We modified the netFD structure so that netFD.ctl represents
the accepted connection and netFD.listen represents the
listener.
Change-Id: Ie38c7dbaeaf77fe9ff7da293f09e86d1a01b3e1e
Reviewed-on: https://go-review.googlesource.com/31390
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously, we used to write the "hangup" message to
the TCP connection control file to be able to close
a connection, while waking up the readers.
The "hangup" message closes the TCP connection with a
RST message. This is a problem when closing a connection
consecutively to a write, because the reader may not have
time to acknowledge the message before the connection is
closed, resulting in loss of data.
We use a "close" message, newly implemented in the Plan 9
kernel to be able to close a TCP connection gracefully with a FIN.
Updates #15464.
Change-Id: I2050cc72fdf7a350bc6c9128bae7d14af11e599c
Reviewed-on: https://go-review.googlesource.com/31271
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change documents that the InterfaceAddrs function is less usable on
multi-homed IP nodes because of the lack of network interface
identification information.
Also updates documentation on exposed network interface API.
Fixes#14518.
Change-Id: I5e86606f8019ab475eb5d385bd797b052cba395d
Reviewed-on: https://go-review.googlesource.com/31371
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL changes how the http1 Server reads from the client.
The goal of this change is to make the Request.Context given to Server
Handlers become done when the TCP connection dies (has seen any read
or write error). I didn't finish that for Go 1.7 when Context was
added to http package.
We can't notice the peer disconnect unless we're blocked in a Read
call, though, and previously we were only doing read calls as needed,
when reading the body or the next request. One exception to that was
the old pre-context CloseNotifier mechanism.
The implementation of CloseNotifier has always been tricky. The past
few releases have contained the complexity and moved the
reading-from-TCP-conn logic into the "connReader" type. This CL
extends connReader to make sure that it's always blocked in a Read
call, at least once the request body has been fully consumed.
In the process, this deletes all the old CloseNotify code and unifies
it with the context cancelation code. The two notification mechanisms
are nearly identical, except the CloseNotify path always notifies on
the arrival of pipelined HTTP/1 requests. We might want to change that
in a subsequent commit. I left a TODO for that. For now there's no
change in behavior except that the context now cancels as it was
supposed to.
As a bonus that fell out for free, a Handler can now use CloseNotifier
and Hijack together in the same request now.
Fixes#15224 (make http1 Server always in a Read, like http2)
Fixes#15927 (cancel context when underlying connection closes)
Updates #9763 (CloseNotifier + Hijack)
Change-Id: I972cf6ecbab7f1230efe8cc971e89f8e6e56196b
Reviewed-on: https://go-review.googlesource.com/31173
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Flesh out nacl's fake network system to match how all the other
platforms work: all other systems' SetReadDeadline and
SetWriteDeadline affect currently-blocked read & write calls.
This was documented in golang.org/cl/30164 because it was the status
quo and existing packages relied on it. (notably the net/http package)
And add a test.
Change-Id: I074a1054dcabcedc97b173dad5e827f8babf7cfc
Reviewed-on: https://go-review.googlesource.com/31178
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
No need to skip it. It passes.
Maybe it was fixed at some point.
Change-Id: I9848924aefda44f9b3a574a8705fa549d657f28d
Reviewed-on: https://go-review.googlesource.com/31177
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Make a zero-byte write to a hijacked connection not log anything, so handlers
can test whether a connection is hacked by doing a Write(nil).
Fixes#16456
Change-Id: Id56caf822c8592067bd8422672f0c1aec89e866c
Reviewed-on: https://go-review.googlesource.com/30812
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
The new implementation parses the plan9 interface files
and builds Interface representations for the net package.
Updates #17218
Change-Id: I3199d18d9e96a17e922186c3abff1d7cd9cbec2e
Reviewed-on: https://go-review.googlesource.com/29963
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Renamed TestPOSTQuery to TestParseFormQuery and added testing
for the ";" delimiter, an empty key, an empty value and an
empty key + value.
Also added TestParseFormQueryMethods to make sure forms sent in
PATCH and PUT (and no others) request are parsed correctly in
ParseForm.
Fixes#17368
Change-Id: I445aad324ffc7b38d179ea41953bffbac0cddffe
Reviewed-on: https://go-review.googlesource.com/30555
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit 59320c396e.
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>
I avoided anywhere in the compiler or things which might be used by
the compiler in the future, since they need to build with Go 1.4.
I also avoided anywhere where there was no benefit to changing it.
I probably missed some.
Updates #16721
Change-Id: Ib3c895ff475c6dec2d4322393faaf8cb6a6d4956
Reviewed-on: https://go-review.googlesource.com/30250
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
All implementations have always implemented this behavior, it's
tested, and it's depended on by other packages. (notably, by net/http)
The one exception is Plan 9 which doesn't support I/O deadlines at all
(tracked in #11932). As a result, a bunch of tests fail on plan9
(#7237). But once Plan 9 adds I/O deadline support, it'll also need
this behavior.
Change-Id: Idb71767f0c99279c66dce29f7bdc78ef467e47aa
Reviewed-on: https://go-review.googlesource.com/30164
Reviewed-by: Sam Whited <sam@samwhited.com>
Reviewed-by: Ian Lance Taylor <iant@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>
Updates bundled http2 to x/net git rev a333c53 for:
http2: add Transport support for IdleConnTimeout
https://golang.org/cl/30075
And add tests.
The bundled http2 also includes a change adding a Ping method to
http2.ClientConn, but that type isn't exposed in the standard
library. Nevertheless, the code gets compiled and adds a dependency on
"crypto/rand", requiring an update to go/build's dependency
test. Because net/http already depends on crypto/tls, which uses
crypto/rand, it's not really a new dependency.
Fixes#16808
Change-Id: I1ec8666ea74762f27c70a6f30a366a6647f923f7
Reviewed-on: https://go-review.googlesource.com/30078
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I decided not to expand the API for this per discusion on #16220.
Fixes#16220
Change-Id: I65cb2eacd4ec28c79438a8f7c30024524a484ce6
Reviewed-on: https://go-review.googlesource.com/30082
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fix an old TODO and use atomic.Value for holding the Transport's
alternate protocol map. It is very frequently accessed and almost
never set or updated.
Change-Id: Ic5a71c504bdac76678114c6390d1fc0673e07aa9
Reviewed-on: https://go-review.googlesource.com/29967
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
No fast path currently for solaris, windows, nacl, plan9.
Fixes#13451
Change-Id: I24b3233a2e3a57fc6445e276a5c0d7b097884007
Reviewed-on: https://go-review.googlesource.com/29951
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
In https://golang.org/cl/28951 I cleaned up the lookupProtocol and
lookupPort paths to be consistently case-insensitive across operating
systems and to share the same baked-in maps of port & proto values
that can be relied on to exist on any platform.
I missed the fallback to the baked-in maps on Windows, though, which
broke Windows XP. This should fix it.
Fixes#17175
Change-Id: Iecd434fb684304137ee27f5521cfaa8c351a1bde
Reviewed-on: https://go-review.googlesource.com/29968
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This sentence is partially guilty why httptrace is considered as an
http.Client tracing package. Removing the mention.
Updates #17152.
Change-Id: I69f78a6e10817db933f44e464a949ae896e44ec6
Reviewed-on: https://go-review.googlesource.com/29755
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change modifies URL.String to prepend "./" to a relative URL which
contains a colon in the first path segment.
Per RFC 3986 §4.2:
> A path segment that contains a colon character (e.g., "this:that")
> cannot be used as the first segment of a relative-path reference, as
> it would be mistaken for a scheme name. Such a segment must be
> preceded by a dot-segment (e.g., "./this:that") to make a relative-
> path reference.
https://go-review.googlesource.com/27440 corrects the behavior for http.FileServer,
but URL.String will still return an invalid URL. This CL reverts the changes to
http.FileServer as they are unnecessary with this fix.
Fixes#17184
Change-Id: I9211ae20f82c91b785d1b079b2cd766487d94225
Reviewed-on: https://go-review.googlesource.com/29610
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new Resolver type (a struct) has 9 Lookup methods, all taking a
context.Context.
There's now a new DefaultResolver global, like http's
DefaultTransport and DefaultClient.
net.Dialer now has an optional Resolver field to set the Resolver.
This also does finishes some resolver cleanup internally, deleting
lookupIPMerge and renaming lookupIPContext into Resolver.LookupIPAddr.
The Resolver currently doesn't let you tweak much, but it's a struct
specifically so we can add knobs in the future. Currently I just added
a bool to force the pure Go resolver. In the future we could let
people provide an interface to implement the methods, or add a Timeout
time.Duration, which would wrap all provided contexts in a
context.WithTimeout.
Fixes#16672
Change-Id: I7ba1f886704f06def7b6b5c4da9809db51bc1495
Reviewed-on: https://go-review.googlesource.com/29440
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change fills the ContentLength field in the http.Response returned by
ResponseRecorder.Result.
Fixes#16952.
Change-Id: I9c49b1bf83e3719b5275b03a43aff5033156637d
Reviewed-on: https://go-review.googlesource.com/28302
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Tracing happens at the http.Trace level. Fix the example to demostrate
tracing in the lifecycle of a RoundTrip.
Updates #17152.
Change-Id: Ic7d7bcc550176189206185482e8962dbf1504ff1
Reviewed-on: https://go-review.googlesource.com/29431
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of ranging over servers in the config, grab an offset
from the config that is used to determine indices.
When the rotate option is enabled, the offset increases which
rotates queries through servers. Otherwise, it is always 0
which uses servers in config order.
Fixes#17126
Change-Id: If233f6de7bfa42f88570055b9ab631be08a76b3e
Reviewed-on: https://go-review.googlesource.com/29233
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
ReadMsgUnix and WriteMsgUnix both will read/write 1 byte from/to the
socket if they were given no buffer to read/write, to avoid a common
pitfall in out of band operations (they will usually block
indefinitely if there's no actual data to read).
This patch adds a note about this behaviour in their documentation, so
users can be aware of it.
Change-Id: I751f0e12bb4d80311e94ea8de023595c5d40ec3e
Reviewed-on: https://go-review.googlesource.com/29180
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@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 CL 28951 added TestLookupPort_Minimal, which was failing
on Plan 9, because lookupPort was not case-insensitive.
Change-Id: Ic80dd29dad4ffd1c84c2590e3d5d0e588ab2e6c2
Reviewed-on: https://go-review.googlesource.com/29051
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Remove the naked returns and goto statements from parse.
Make tests more consistent in the got/want ordering, and clean up some
unnecessary helper functions.
Change-Id: Iaa244cb8c00dd6b42836d95448bf02caa72bfabd
Reviewed-on: https://go-review.googlesource.com/28890
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also, flesh out the baked-in /etc/services table for LookupPort a bit.
This services map moves from a unix-specific file to a portable file
where nacl can use it.
Also, remove the duplicated entries in the protocol map in different
cases, and just canonicalize the input before looking in the map. Now
it handles any case, including MiXeD cAse.
In the process, add a test that service names for LookupPort are case
insensitive. They were on Windows, but not cgo. Now there's a test and
they're case insensitive in all 3+ paths. Maybe it breaks plan9. We'll
see.
Fixes#17045
Change-Id: Idce7d68703f371727c7505cda03a32bd842298cd
Reviewed-on: https://go-review.googlesource.com/28951
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
CL 28484 mistakenly assumed that WSARecv returns WSAEINPROGRESS
when there is nothing to read. But the error is ERROR_IO_PENDING.
Fix that mistake.
I was about to write a test for it. But I have found
TestTCPReadWriteAllocs in net package that does nearly what I need,
but was conveniently disabled. So enable and extend the test.
Fixes#16988
Change-Id: I55e5cf8998a9cf29e92b398d702280bdf7d6fc85
Reviewed-on: https://go-review.googlesource.com/28990
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
Currently, path resolution is done using the un-escaped version of
paths. This means that path elements like one%2ftwo%2fthree are
handled incorrectly, and optional encodings (%2d vs. -) are dropped.
This function makes escaped handling consistent with Parse: provided
escapings are honoured, and RawPath is only set if necessary.
A helper method setPath is introduced to handle the correct setting of
Path and RawPath given the encoded path.
Fixes#16947
Change-Id: I40b1215e9066e88ec868b41635066eee220fde37
Reviewed-on: https://go-review.googlesource.com/28343
Run-TryBot: Dave Day <djd@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Hop-by-hop headers (explicitly mentioned in RFC 2616) were already
removed from the response. This removes the custom hop-by-hop
headers listed in the "Connection" header of the response.
Updates #16875
Change-Id: I6b8f261d38b8d72040722f3ded29755ef0303427
Reviewed-on: https://go-review.googlesource.com/28810
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were already making a copy of the map before removing
hop-by-hop headers. This commit does the same for proxied
headers mentioned in the "Connection" header.
A test is added to ensure request headers are not modified.
Updates #16875
Change-Id: I85329d212787958d5ad818915eb0538580a4653a
Reviewed-on: https://go-review.googlesource.com/28493
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
https://golang.org/cl/27206 fixed the dtoi function such that
it now properly parses negative number. Ironically, this causes
several other functions that depended on dtoi to now (incorrectly)
parse negative numbers.
For example, ParseCIDR("-1.0.0.0/32") used to be rejected prior to the
above CL, but is now accepted even though it is an invalid CIDR notation.
This CL fixes that regression.
We fix this by removing the signed parsing logic entirely from dtoi.
It was introduced relatively recently in https://golang.org/cl/12447
to fix a bug where an invalid port was improperly being parsed as OK.
It seems to me that the fix in that CL to the port handling logic was
sufficient such that a change to dtoi was unnecessary.
Updates #16350
Change-Id: I414bb1aa27d0a226ebd4b05a09cb40d784691b43
Reviewed-on: https://go-review.googlesource.com/28414
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
The http.Transport's retry can't retry requests with non-nil
bodies. When cloning an incoming server request into an outgoing
client request, nil out the Body field if the ContentLength is 0. (For
server requests, Body is always non-nil, even for GET, HEAD, etc.)
Also, don't use the deprecated CancelRequest and use Context instead.
And don't set Proto, ProtoMajor, ProtoMinor. Those are ignored in
client requests, which was probably a later documentation
clarification.
Fixes#16036
Updates #16696 (remove useless Proto lines)
Change-Id: I70a869e9bd4bf240c5838e82fb5aa695a539b343
Reviewed-on: https://go-review.googlesource.com/28412
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
RFC 2616, section 14.10 says:
>>>
HTTP/1.1 proxies MUST parse the Connection header field before a message
is forwarded and, for each connection-token in this field, remove any
header field(s) from the message with the same name as the
connection-token. Connection options are signaled by the presence of a
connection-token in the Connection header field, not by any
corresponding additional header field(s), since the additional header
field may not be sent if there are no parameters associated with that
connection option.
<<<
The same requirement was included in RFC 7230, section 6.1.
Fixes#16875
Change-Id: I57ad4a4a17775537c8810d0edd7de1604317b5fa
Reviewed-on: https://go-review.googlesource.com/27970
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Per RFC 7230, Section 3.3.2: "A server MUST NOT send a Content-Length
header field in any response with a status code of 1xx (Informational)
or 204 (No Content).".
Fixes#16942
Change-Id: I8006c76c126304e13618966e6eafb08a3885d3cd
Reviewed-on: https://go-review.googlesource.com/28351
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In Go 1.0, the Config struct consisted only of exported fields.
In Go 1.1, it started to grow private, uncopyable fields (sync.Once,
sync.Mutex, etc).
Ever since, people have been writing their own private Config.Clone
methods, or risking it and doing a language-level shallow copy and
copying the unexported sync variables.
Clean this up and export the Config.clone method as Config.Clone.
This matches the convention of Template.Clone from text/template and
html/template at least.
Fixes#15771
Updates #16228 (needs update in x/net/http2 before fixed)
Updates #16492 (not sure whether @agl wants to do more)
Change-Id: I48c2825d4fef55a75d2f99640a7079c56fce39ca
Reviewed-on: https://go-review.googlesource.com/28075
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
As @pmarks-net said in the bug, this is something of a prisoner's
dilemma, but it does help people who occasionally report problems.
This is temporary. IPv6 is happening regardless of our decision here,
so we'll do this for now.
Fixes#15324
Change-Id: I8cc29c6efa56222970996c71182fc9ee89d78539
Reviewed-on: https://go-review.googlesource.com/28077
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
This change makes sure that tests are run with the correct
version of the go tool. The correct version is the one that
we invoked with "go test", not the one that is first in our path.
Fixes#16577
Change-Id: If22c8f8c3ec9e7c35d094362873819f2fbb8559b
Reviewed-on: https://go-review.googlesource.com/28089
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ordering fix: this CL swaps the order of the log write and the channel close
in WroteRequest. I could reproduce the bug by putting a sleep between the two
when the channel close was first. It needs to happen after the log.
Data race: use the log buffer's mutex when reading too. Not really
important once the ordering fix above is fixed (since nobody is
concurrently writing anymore), but for consistency.
Fixes#16414
Change-Id: If6657884e67be90b4455c8f5a6f7bc6981999ee4
Reviewed-on: https://go-review.googlesource.com/28078
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The handling of "options timeout:n" is supposed to be per individual
DNS server exchange, not per Lookup call.
Fixes#16865.
Change-Id: I2304579b9169c1515292f142cb372af9d37ff7c1
Reviewed-on: https://go-review.googlesource.com/28057
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add support for writing TLS client random and master secret
in NSS key log format.
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format
Normally this is enabled by a developer debugging TLS based
applications, especially HTTP/2, by setting the KeyLogWriter
to an open file. The keys negotiated in handshake are then
logged and can be used to decrypt TLS sessions e.g. in Wireshark.
Applications may choose to add support similar to NSS where this
is enabled by environment variable, but no such mechanism is
built in to Go. Instead each application must explicitly enable.
Fixes#13057.
Change-Id: If6edd2d58999903e8390b1674ba4257ecc747ae1
Reviewed-on: https://go-review.googlesource.com/27434
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
RFC 7233, section 4.4 says:
>>>
For byte ranges, failing to overlap the current extent means that the
first-byte-pos of all of the byte-range-spec values were greater than the
current length of the selected representation. When this status code is
generated in response to a byte-range request, the sender SHOULD generate a
Content-Range header field specifying the current length of the selected
representation
<<<
Thus, we should send the Content-Range only if none of the ranges
overlap.
Fixes#15798.
Change-Id: Ic9a3e1b3a8730398b4bdff877a8f2fd2e30149e3
Reviewed-on: https://go-review.googlesource.com/24212
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The net package sets a finalizer on *netFD. I looked through all the
uses of *netFD in the package, looking for each case where a *netFD
was passed as an argument and the final reference to the argument was
not a function or method call. I added a call to runtime.KeepAlive after
each such final reference (there were only three).
The code is safe today without the KeepAlive calls because the compiler
keeps arguments alive for the duration of the function. However, that is
not a language requirement, so adding the KeepAlive calls ensures that
this code remains safe even if the compiler changes in the future.
Change-Id: I4e2bd7c5a946035dc509ccefb4828f72335a9ee3
Reviewed-on: https://go-review.googlesource.com/27650
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
On some error when using io.Copy with TCPConn, it displays an error
correlation like the following:
read tcp 192.0.2.1:1111->192.0.2.2:2222: read tcp [2001:db8::2]:2222->[2001:db8::3]:3333 read: connection reset by peer
the correlation "some error on reading after reading operation" looks a
bit confusing because the operation on the ReadFrom method of TCPConn is
actually "writing after reading." To clarify and avoid confusion, this
change sets "readfrom" to the Op field of outer-most OpError instead of
"read."
Change-Id: I6bf4e2e7247143fa54bbcf9cef7a8ae1ede1b35c
Reviewed-on: https://go-review.googlesource.com/25220
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This permits the error message to distinguish between a context that was
canceled and a context that timed out.
Updates #16381.
Change-Id: I3994b98e32952abcd7ddb5fee08fa1535999be6d
Reviewed-on: https://go-review.googlesource.com/24978
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
example for httptest.Recorder was inspecting Recoder directly.
Using Result() to convert Recorder into a http.Response yields a much
better user experience.
Closes#16837
Change-Id: Id0e636c12cd6adb1ba11f89953ff2b0f43758cf3
Reviewed-on: https://go-review.googlesource.com/27495
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The old deprecation docs were referencing another deprecated field.
Fixes#16752
Change-Id: I44a690048e00ddc790a80214ecb7f5bb0a5b7b34
Reviewed-on: https://go-review.googlesource.com/27510
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Go 1.7 crashed after Transport.IdleConnTimeout if an HTTP/2 connection
was established but but its caller no longer wanted it. (Assuming the
connection cache was enabled, which it is by default)
Fixes#16208
Change-Id: I9628757f7669e344f416927c77f00ed3864839e3
Reviewed-on: https://go-review.googlesource.com/27450
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Certain browsers (Chrome 53, Safari 9.1.2, Firefox 46) won't correctly
follow a directory listing's links if the file name begins with a run
of characters then a colon, e.g. "foo:bar". Probably mistaking it for
a URI. However, they are happy to follow "./foo:bar", so this change
prepends "./" to all link hrefs in the directory listing of
FileServer.
Change-Id: I60ee8e1ebac73cbd3a3ac0f23e80fdf52e3dc352
Reviewed-on: https://go-review.googlesource.com/27440
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The RFC is clear that the Parameters in an AlgorithmIdentifer for an RSA
public key must be NULL. BoringSSL enforces this so we have strong
evidence that this is a widely compatible change.
Embarrassingly enough, the major source of violations of this is us. Go
used to get this correct in only one of two places. This was only fixed
in 2013 (with 4874bc9b). That's why lots of test certificates are
updated in this change.
Fixes#16166.
Change-Id: Ib9a4551349354c66e730d44eb8cee4ec402ea8ab
Reviewed-on: https://go-review.googlesource.com/27312
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
.local addresses are used by things like Kubernetes and Weave DNS; Go
should not avoid resolving them.
This is a partial revert of https://golang.org/cl/21328 which was too
strict of an interpretation of RFC 6762.
Fixes#16739
Change-Id: I349415b4eab5d61240dd18217bd95dc7d2105cd5
Reviewed-on: https://go-review.googlesource.com/27250
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BIND libresolv allows values from 0 to 15.
For invalid values and negative numbers, 0 is used.
For numbers greater than 15, 15 is used.
Fixes#15419
Change-Id: I1009bc119c3e87919bcb55a80a35532e9fc3ba52
Reviewed-on: https://go-review.googlesource.com/24901
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If the server failed on us before we even tried to write any bytes,
it's safe to retry the request on a new connection, regardless of the
HTTP method/idempotence.
Fixes#15723
Change-Id: I25360f82aac530d12d2b3eef02c43ced86e62906
Reviewed-on: https://go-review.googlesource.com/27117
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Let the kernel pick a port for testing, and have the server in the
child process tell the parent (benchmarking) process the port that
was selected.
Fixes flakes like seen in https://golang.org/cl/27050 (and previously)
Change-Id: Ia2b705dc4152f70e0a5725015bdae09984d09d53
Reviewed-on: https://go-review.googlesource.com/27051
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The Go HTTP/1 client will make as many new TCP connections as the user requests.
The HTTP/2 client tried to have that behavior, but the policy of
whether a connection is re-usable didn't take into account the extra 1
stream counting against SETTINGS_MAX_CONCURRENT_STREAMS so in practice
users were getting errors.
For example, if the server's advertised max concurrent streams is 100
and 200 concurrrent Go HTTP requests ask for a connection at once, all
200 will think they can reuse that TCP connection, but then 100 will
fail later when the number of concurrent streams exceeds 100.
Instead, recognize the "no cached connections" error value in the
shouldRetryRequest method, so those 100 will retry a new connection.
This is the conservative fix for Go 1.7 so users don't get errors, and
to match the HTTP/1 behavior. Issues #13957 and #13774 are the more
involved bugs for Go 1.8.
Updates #16582
Updates #13957
Change-Id: I1f15a7ce60c07a4baebca87675836d6fe03993e8
Reviewed-on: https://go-review.googlesource.com/25580
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Updates bundled http2 to x/net/http2 git rev 075e191 for:
http2: adjust flow control on open streams when processing SETTINGS
https://golang.org/cl/25508Fixes#16612
Change-Id: Ib0513201bff44ab747a574ae6894479325c105d2
Reviewed-on: https://go-review.googlesource.com/25543
Run-TryBot: Chris Broadfoot <cbro@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This was previously fixed in https://golang.org/cl/21497 but not enough.
Fixes#16523
Change-Id: I678543a656304c82d654e25e12fb094cd6cc87e8
Reviewed-on: https://go-review.googlesource.com/25330
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates x/net/http2 to git rev 6a513af for:
http2: return flow control for closed streams
https://golang.org/cl/25231
http2: make Transport prefer HTTP response header recv before body write error
https://golang.org/cl/24984
http2: make Transport treat "Connection: close" the same as Request.Close
https://golang.org/cl/24982Fixesgolang/go#16481
Change-Id: Iaddb166387ca2df1cfbbf09a166f8605578bec49
Reviewed-on: https://go-review.googlesource.com/25282
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
From at least Go 1.4 to Go 1.6, Transport.RoundTrip would return the
error value from net.Conn.Read directly when the initial Read (1 byte
Peek) failed while reading the HTTP response, if a request was
outstanding. While never a documented or tested promise, Go 1.7 changed the
behavior (starting at https://golang.org/cl/23160).
This restores the old behavior and adds a test (but no documentation
promises yet) while keeping the fix for spammy logging reported in #15446.
This looks larger than it is: it just changes errServerClosedConn from
a variable to a type, where the type preserves the underlying
net.Conn.Read error, for unwrapping later in Transport.RoundTrip.
Fixes#16465
Change-Id: I6fa018991221e93c0cfe3e4129cb168fbd98bd27
Reviewed-on: https://go-review.googlesource.com/25153
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Noticed when investigating a separate issue.
No external bug report or repro yet.
Change-Id: I8a1641a43163f22b09accd3beb25dd9e2a68a238
Reviewed-on: https://go-review.googlesource.com/25152
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
This copies the frozen wording from the log/syslog package.
Fixes#16436
Change-Id: If5d478023328925299399f228d8aaf7fb117c1b4
Reviewed-on: https://go-review.googlesource.com/25080
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Because,
* The CGI spec defines that incoming request header "Foo: Bar" maps to
environment variable HTTP_FOO == "Bar". (see RFC 3875 4.1.18)
* The HTTP_PROXY environment variable is conventionally used to configure
the HTTP proxy for HTTP clients (and is respected by default for
Go's net/http.Client and Transport)
That means Go programs running in a CGI environment (as a child
process under a CGI host) are vulnerable to an incoming request
containing "Proxy: attacker.com:1234", setting HTTP_PROXY, and
changing where Go by default proxies all outbound HTTP requests.
This is CVE-2016-5386, aka https://httpoxy.org/Fixes#16405
Change-Id: I6f68ade85421b4807785799f6d98a8b077e871f0
Reviewed-on: https://go-review.googlesource.com/25010
Run-TryBot: Chris Broadfoot <cbro@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Only run TestDialerDualStack on the builders, as to not annoy or
otherwise distract users when it's not their fault.
Even though the intention is to only run this on the builders, very
few of the builders have IPv6 support. Oh well. We'll get some
coverage.
Updates #13324
Change-Id: I13e7e3bca77ac990d290cabec88984cc3d24fb67
Reviewed-on: https://go-review.googlesource.com/24985
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Regression from Go 1.6 to Go 1.7rc1: we had broken the ability for
users to vendor "golang.org/x/net/http2" or "golang.org/x/net/route"
because we were vendoring them ourselves and cmd/go and cmd/compile do
not understand multiple vendor directories across multiple GOPATH
workspaces (e.g. user's $GOPATH and default $GOROOT).
As a short-term fix, since fixing cmd/go and cmd/compile is too
invasive at this point in the cycle, just rename "golang.org" to
"golang_org" for the standard library's vendored copy.
Fixes#16333
Change-Id: I9bfaed91e9f7d4ca6bab07befe80d71d437a21af
Reviewed-on: https://go-review.googlesource.com/24902
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The test was checking for 1 of 2 possible error values. But based on
goroutine scheduling and the randomness of select statement receive
cases, it was possible for a 3rd type of error to be returned.
This modifies the code (not the test) to make that third type of error
actually the second type of error, which is a nicer error message.
The test is no longer flaky. The flake was very reproducible with a
5ms sleep before the select at the end of Transport.getConn.
Thanks to Github user @jaredborner for debugging.
Fixes#16049
Change-Id: I0d2a036c9555a8d2618b07bab01f28558d2b0b2c
Reviewed-on: https://go-review.googlesource.com/24748
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates x/net/http2 to git rev b400c2e for https://golang.org/cl/24214,
"http2: add additional blacklisted ciphersuites"
Both TLS_RSA_WITH_AES_128_GCM_SHA256 & TLS_RSA_WITH_AES_256_GCM_SHA384
are now blacklisted, per http://httpwg.org/specs/rfc7540.html#BadCipherSuites
Change-Id: I8b9a7f4dc3c152d0675e196523ddd36111744984
Reviewed-on: https://go-review.googlesource.com/24684
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There was only one use of "HTTP/1.n" compared to "HTTP/1.x":
h2_bundle.go:// "Just as in HTTP/1.x, header field names are strings of ASCII
httputil/dump.go:// DumpRequest returns the given request in its HTTP/1.x wire
httputil/dump.go:// intact. HTTP/2 requests are dumped in HTTP/1.x form, not in their
response.go:// Write writes r to w in the HTTP/1.x server response format,
server.go: // Request.Body. For HTTP/1.x requests, handlers should read any
server.go:// The default HTTP/1.x and HTTP/2 ResponseWriter implementations
server.go:// The default ResponseWriter for HTTP/1.x connections supports
server.go:// http1ServerSupportsRequest reports whether Go's HTTP/1.x server
server.go: // about HTTP/1.x Handlers concurrently reading and writing, like
server.go: // HTTP/1.x from here on.
transport.go: return fmt.Errorf("net/http: HTTP/1.x transport connection broken: %v", err)
Be consistent.
Change-Id: I93c4c873e500f51af2b4762055e22f5487a625ac
Reviewed-on: https://go-review.googlesource.com/24610
Reviewed-by: Andrew Gerrand <adg@golang.org>
Updates x/net/http2 to git rev ef2e00e88 for https://golang.org/cl/24560,
"http2: make Transport return server's GOAWAY error back to the user"
Fixes#14627
Change-Id: I2bb123a3041e168db7c9446beef4ee47638f17ee
Reviewed-on: https://go-review.googlesource.com/24561
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Don't configure HTTP/2 in http.Server.Serve(net.Listener) if the
Server's TLSConfig is set and doesn't include the "h2" NextProto
value. This avoids mutating a *tls.Config already in use if
previously passed to tls.NewListener.
Also document this. (it's come up a few times now)
Fixes#15908
Change-Id: I283eed82fdb29a791f80d801aadd9f75db244de0
Reviewed-on: https://go-review.googlesource.com/24508
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The version of pprof in gperftools has been deprecated.
No need to have a pointer to that version since go tool pprof
is included with the Go distro.
Change-Id: I6d769a68f64280f5db89ff6fbc67bfea9c8f1526
Reviewed-on: https://go-review.googlesource.com/24509
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Change ExampleResponseRecorder to use httptest.NewRequest instead of
http.NewRequest. This makes the example shorter and shows how to use
one more function from the httptest package.
Change-Id: I3d35869bd0a4daf1c7551b649428bb2f2a45eba2
Reviewed-on: https://go-review.googlesource.com/24480
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#16063
Change-Id: I2e8695beb657b0aef067e83f086828d8857787ed
Reviewed-on: https://go-review.googlesource.com/24130
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Do run it on the builders, though.
Fixes#15881.
Change-Id: Ie42204d553cb18547ffd6441afc261717bbd9205
Reviewed-on: https://go-review.googlesource.com/24111
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates x/net/http2 to git rev 313cf39 for CLs 23812 and 23880:
http2: GotFirstResponseByte hook should only fire once
http2: fix data race on pipe
Fixes#16000
Change-Id: I9c3f1b2528bbd99968aa5a0529ae9c5295979d1d
Reviewed-on: https://go-review.googlesource.com/23881
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Fixes#15948
Change-Id: Idd79859b3e98d61cd4e3ef9caa5d3b2524fd026a
Reviewed-on: https://go-review.googlesource.com/23810
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
