Adds the (*tls.Conn).HandshakeContext method. This allows
us to pass the context provided down the call stack to
eventually reach the tls.ClientHelloInfo and
tls.CertificateRequestInfo structs.
These contexts are exposed to the user as read-only via Context()
methods.
This allows users of (*tls.Config).GetCertificate and
(*tls.Config).GetClientCertificate to use the context for
request scoped parameters and cancellation.
Replace uses of (*tls.Conn).Handshake with (*tls.Conn).HandshakeContext
where appropriate, to propagate existing contexts.
Fixes#32406
Change-Id: I33c228904fe82dcf57683b63627497d3eb841ff2
Reviewed-on: https://go-review.googlesource.com/c/go/+/246338
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Instead of always polling 500ms, start with an interval of 1ms and
exponentially back off to at most 500ms. 10% jitter is added to each
interval.
This makes Shutdown more responsive when connections and listeners
close quickly.
Also removes the need for the polling interval to be changed in tests
since if tests' connections and listeners close quickly Shutdown will
also return quickly.
Fixes#42156
Change-Id: I5e59844a2980c09adebff57ae8b58817965e6db4
Reviewed-on: https://go-review.googlesource.com/c/go/+/264479
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Bryan C. Mills <bcmills@google.com>
The old ioutil references are still valid, but update our code
to reflect best practices and get used to the new locations.
Code compiled with the bootstrap toolchain
(cmd/asm, cmd/dist, cmd/compile, debug/elf)
must remain Go 1.4-compatible and is excluded.
Also excluded vendored code.
For #41190.
Change-Id: I6d86f2bf7bc37a9d904b6cee3fe0c7af6d94d5b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/263142
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This is primarily aimed at client requests where the user can supply
their own io.ReadCloser, but also clarifies server request behavior.
A server request body can be one of:
- *body
- *http2RequestBody
- *expectContinueReader
- *maxBytesReader
Of those, *expectContinueReader did not meet these expectations, so this
change also removes the data race.
Change-Id: Id4f1ae573d938347b1123a7b612b271aabb045a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/251087
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
When rejecting a request with an unsupported HTTP protocol version,
return a 505 error ("HTTP Version Not Supported") instead of 400.
Fixes#40454.
Change-Id: I0269f0f5755d90d1b772ba0094a6bb24b5eb4701
Reviewed-on: https://go-review.googlesource.com/c/go/+/261977
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Urban Ishimwe <urbainishimwe@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
On Server.Shutdown, all idle connections are closed.
A caveat for new connections is that they are marked idle
after 5 seconds.
Previously new HTTP/2 connections were marked New, and after 5 seconds,
they would then become idle. With this change, we now mark HTTP/2
connections as Active to allow the proper shutdown sequence to occur.
Fixes#36946Fixes#39776
Change-Id: I31efbf64b9a2850ca544da797f86d7e1b3378e8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/240278
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rather than probe and guess if sendfile will work inside ResponseWriter.ReadFrom(src),
this change fixes the underlying issue of starting to respond before src is readable
We'll no longer send a status OK if a header has not yet been written and reading
from src is destined to fail. This small change implicitly takes care of the need for
the server to sniff the response body to determine the Content-Type.
This allows splice to work on Linux when src is a socket or any non-regular file that's spliceable.
The extra read of 512 bytes may raise an objection, and that's fair, but
we're already swapping some syscall prep work for another and a read of
512 probably will not impact the overall performance. For shorter
bodies, there's likely less setup time. A little initial slop is not too
unusual in zero copy network code, and sometimes actually helps.
Fixes#40888
Change-Id: I4a8e2ad0ace1318bae66dae5671d06ea6d4838ed
GitHub-Last-Rev: 097364ea86
GitHub-Pull-Request: golang/go#40903
Reviewed-on: https://go-review.googlesource.com/c/go/+/249238
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
The StripPrefix wrapper strips a prefix string from the request's
URL.Path field, but doesn't touch the RawPath field. This leads to the
confusing situation when StripPrefix handles a request with URL.RawPath
populated (due to some escaped characters in the request path) and the
wrapped request's RawPath contains the prefix but Path does not.
This change modifies StripPrefix to strip the prefix from both Path and
RawPath. If there are escaped characters in the prefix part of the
request URL the stripped handler serves a 404 instead of invoking the
underlying handler with a mismatched Path/RawPath pair.
This is a backward incompatible change for a very small minority of
requests; I would be surprised if anyone is depending on this behavior,
but it is possible. If that's the case, we could make a more
conservative change where the RawPath is trimmed if possible, but when
the prefix contains escaped characters then we don't 404 but rather send
through the invalid Path/RawPath pair as before.
Fixes#24366
Change-Id: I7030b8c183a3dfce307bc0272bba9a18df4cfe08
Reviewed-on: https://go-review.googlesource.com/c/go/+/233637
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The expectContinueReader writes to the connection on the first
Request.Body read. Since a Handler might be doing a read in parallel or
before a write, expectContinueReader needs to synchronize with the
ResponseWriter, and abort if a response already went out.
The tests will land in a separate CL.
Fixes#34902
Fixes CVE-2020-15586
Change-Id: Icdd8dd539f45e8863762bd378194bb4741e875fc
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/793350
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/242598
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 236857 removed all uses of whitelist/blacklist, which is great.
But it substituted awkward phrasing using allowlist/blocklist,
especially as verbs or participles. This CL uses more standard English,
like "allow the function" or "blocked functions" instead of
"allowlist the function" or "blocklisted functions".
Change-Id: I9106a2fdbd62751c4cbda3a77181358a8a6d0f13
Reviewed-on: https://go-review.googlesource.com/c/go/+/236917
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There's been plenty of discussion on the usage of these terms in tech.
I'm not trying to have yet another debate. It's clear that there are
people who are hurt by them and who are made to feel unwelcome by their
use due not to technical reasons but to their historical and social
context. That's simply enough reason to replace them.
Anyway, allowlist and blocklist are more self-explanatory than whitelist
and blacklist, so this change has negative cost.
Didn't change vendored, bundled, and minified files. Nearly all changes
are tests or comments, with a couple renames in cmd/link and cmd/oldlink
which are extremely safe. This should be fine to land during the freeze
without even asking for an exception.
Change-Id: I8fc54a3c8f9cc1973b710bbb9558a9e45810b896
Reviewed-on: https://go-review.googlesource.com/c/go/+/236857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Khosrow Moossavi <khos2ow@gmail.com>
Reviewed-by: Leigh McCulloch <leighmcc@gmail.com>
Reviewed-by: Urban Ishimwe <urbainishimwe@gmail.com>
Reduces binary size by 4K, not counting the http2 changes (in CL
231119) that'll be bundled into this package in the future.
Updates golang/go#38782
Change-Id: Id360348707e076b8310a8f409e412d68dd2394b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/231118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Wait for Listeners to drop to zero too, not just conns.
Fixes#33313
Change-Id: I09350ae38087990d368dcf9302fbde3e95c02fcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/213442
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hasit Bhatt <hasit.p.bhatt@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This removes the TODO leftover by replacing the original int32 for
atomicBool that mimicks atomic operations for boolean.
Change-Id: I1b2cac0c9573c890c7315e9906ce6bfccee3d770
Reviewed-on: https://go-review.googlesource.com/c/go/+/223357
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The parseURL variable was introduced in CL 49930 in order to work
around the fact that the name "url" was shadowed by a parameter of
exported functions, and couldn't be renamed without sacrificing
documentation readability. Documentation readability takes higher
priority than internal implementation details.
Back then, I considered renaming the net/url import but saw that it
would be too disruptive of a change to the large net/http package.
Now I see a better way: it's possible to import net/url both as url
and as urlpkg (the package is still imported just once, but it becomes
available via two names). This way we eliminate the need for wasting
(a little) memory on the parseURL variable, improve code readability
slightly, and delete some lines of code and comments.
Updates #21077
Change-Id: I42cd9833afdcf4a5f5874fb7ee9c8c11eae557dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/202482
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Also added a test to ensure that any interactions
between TimeoutHandler and Flusher result in the
correct status code and body, but also that we don't
get superfluous logs from stray writes as was seen
in the bug report.
Fixes#34439.
Change-Id: I4af62db256742326f9353f98a2fcb5f71d2a5fd9
Reviewed-on: https://go-review.googlesource.com/c/go/+/197659
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The doc comments of both ServerContextKey and LocalAddrContextKey both suggest that context.WithValue can be used to access (get) properties of the server or connection. This PR fixes those comments to refer to Context.Value instead.
Change-Id: I4ed383ef97ba1951f90c555243007469cfc18d4d
GitHub-Last-Rev: 05bc3acf82
GitHub-Pull-Request: golang/go#33833
Reviewed-on: https://go-review.googlesource.com/c/go/+/191838
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
As of Go 1.13rc1, TimeoutHandler supports the Flusher and Pusher interfaces and
this change corrects its documentation to say that.
Fixes#33769
Updates #29193
Change-Id: Ia0523f7f2e3dc1f8f0b68950b85a7bf81c4abe60
GitHub-Last-Rev: 5310d2c960
GitHub-Pull-Request: golang/go#33770
Reviewed-on: https://go-review.googlesource.com/c/go/+/191237
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 46434 changed the doc for Server.IdleTimeout to include
falling back to Server.ReadHeaderTimeout if both
Server.IdleTimeout and Server.ReadTimeout are zero.
However, we explicitly set the ReadDeadlines firstly based
off Server.IdleTimeout or Server.ReadTimeout before attempting
to read the next request, thus the current doc is incorrect.
This CL reverts CL 46434 and also updates the doc for
Server.ReadHeaderTimeout to documenting falling back
to Server.ReadTimeout, if the former is zero, otherwise
there is no timeout.
Fixes#32053
Change-Id: I43dd0252d1bcee6c29a8529abd84c84a49b2fba9
GitHub-Last-Rev: e1cdb59977
GitHub-Pull-Request: golang/go#32164
Reviewed-on: https://go-review.googlesource.com/c/go/+/178337
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This is the net/http half of #32476. This supplies the method needed
by the other half in x/net/http2 in the already-submitted CL 181259,
which this CL also bundles in h2_bundle.go.
Thanks to Tom Thorogood (@tmthrgd) for the bug report and test.
Fixes#32476
Updates #30694
Change-Id: I79d2a280e486fbf75d116f6695fd3abb61278765
Reviewed-on: https://go-review.googlesource.com/c/go/+/181260
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Roll back CL 161738. That fix changed StripPrefix behavior in the
general case, not just in the situation where where stripping the
prefix from path resulted in the empty string, causing issue #31622.
That kind of change to StripPrefix behavior is not backwards compatible,
and there can be a smaller, more targeted fix for the original issue.
Fixes#31622
Updates #30165
Change-Id: Ie2fcfe6787a32e44f71d564d8f9c9d580fc6f704
Reviewed-on: https://go-review.googlesource.com/c/go/+/180498
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Ensures that our HTTP/1.X Server properly responds
with a 501 Unimplemented as mandated by the spec at
RFC 7230 Section 3.3.1, which says:
A server that receives a request message with a
transfer coding it does not understand SHOULD
respond with 501 (Unimplemented).
Fixes#30710
Change-Id: I096904e6df053cd1e4b551774cc27523ff3d09f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/167017
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit adds a KeepAlive field to ListenConfig and uses it
analogously to Dialer.KeepAlive to set TCP KeepAlives per default on
Accept()
Fixes#23378
Change-Id: I57eaf9508c979e7f0e2b8c5dd8e8901f6eb27fd6
GitHub-Last-Rev: e9e035d53e
GitHub-Pull-Request: golang/go#31242
Reviewed-on: https://go-review.googlesource.com/c/go/+/170678
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This also updates the vendored-in versions of several packages: 'go
mod vendor' selects a consistent version of each module, but we had
previously vendored an ad-hoc selection of packages.
Notably, x/crypto/hkdf was previously vendored in at a much newer
commit than the rest of x/crypto. Bringing the rest of x/crypto up to
that commit introduced an import of golang.org/x/sys/cpu, which broke
the js/wasm build, requiring an upgrade of x/sys to pick up CL 165749.
Updates #30228
Updates #30241
Updates #25822
Change-Id: I5b3dbc232b7e6a048a158cbd8d36137af1efb711
Reviewed-on: https://go-review.googlesource.com/c/go/+/164623
Reviewed-by: Filippo Valsorda <filippo@golang.org>
I had been finding these over a year or so, but none were big enough
changes to warrant CLs. They're a handful now, so clean them all up in a
single commit.
The smaller bodies get a bit simpler, but most importantly, the larger
bodies get unindented.
Change-Id: I5707a6fee27d4c9ff9efd3d363af575d7a4bf2aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/165340
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The path of the new stripped URL should also be cleaned. Since an empty path
may cause unexpected errors in some HTTP handlers, e.g. http.ServeFile.
Fixes#30165
Change-Id: Ib44fdce6388b5d62ffbcab5266925ef8f13f26e2
Reviewed-on: https://go-review.googlesource.com/c/161738
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Packages in vendor/ directories have a "vendor/" path prefix in GOPATH
mode, but intentionally do not in module mode. Since the import path
is embedded in the compiled output, changing that path invalidates
cache entries and causes cmd/go to try to rebuild (and reinstall) the
vendored libraries, which will fail if the directory containing those
libraries is read-only.
If I understood correctly, this is the approach Russ suggested as an
alternative to https://golang.org/cl/136138.
Fixes#27285Fixes#26988
Change-Id: I8a2507fa892b84cde0a803aaa79e460723da572b
Reviewed-on: https://go-review.googlesource.com/c/147443
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Scanning through all path patterns is not necessary, since the
paths do not change frequently. Instead, maintain a sorted list
of path prefixes and return the first match.
name old time/op new time/op delta
ServerMatch-12 134ns ± 3% 17ns ± 4% -86.95% (p=0.000 n=19+20)
Change-Id: I15b4483dc30db413321435ee6815fc9bf2bcc546
Reviewed-on: https://go-review.googlesource.com/c/144937
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds a crypto/tls.RecordHeaderError.Conn field containing the TLS
underlying net.Conn for non-TLS handshake errors, and then uses it in
the net/http Server to return plaintext HTTP 400 errors when a client
mistakenly sends a plaintext HTTP request to an HTTPS server. This is the
same behavior as Apache.
Also in crypto/tls: swap two error paths to not use a value before
it's valid, and don't send a alert record when a handshake contains a
bogus TLS record (a TLS record in response won't help a non-TLS
client).
Fixes#23689
Change-Id: Ife774b1e3886beb66f25ae4587c62123ccefe847
Reviewed-on: https://go-review.googlesource.com/c/143177
Reviewed-by: Filippo Valsorda <filippo@golang.org>
If an illegal header write is detected, find the first caller outside of
net/http using runtime.CallersFrames and include the call site in the log
message.
Fixes#18761
Change-Id: I92be00ac206c6ebdd60344ad7bf40a7c4c188547
Reviewed-on: https://go-review.googlesource.com/c/130997
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also updates the bundled http2 to x/net/http2 git rev 49c15d80 for:
http2: revert CL 107295 (don't sniff Content-type in Server when nosniff)
https://golang.org/cl/126895Fixes#24795
Change-Id: I6ae1a21c919947089274e816eb628d20490f83ce
Reviewed-on: https://go-review.googlesource.com/126896
Reviewed-by: Damien Neil <dneil@google.com>
The only inaccurate part was the HTTP/2 caveat in Server.ServeTLS, which
only applies to the plain Serve variant.
The restriction implemented in shouldConfigureHTTP2ForServe is not on
the setupHTTP2_ServeTLS codepath because ServeTLS owns the tls.Listener,
so we fix it for the user instead of disabling HTTP/2.
Fixes#24607
Change-Id: Ie5f207d0201f09db27bf81b75535e5f6fdaf91e2
Reviewed-on: https://go-review.googlesource.com/103815
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Contains portions and modified portions of CL 103815
Fixes#24607
Change-Id: Ic330850a0f098f183315f04ea4780eded46c5b77
Reviewed-on: https://go-review.googlesource.com/125515
Reviewed-by: Ian Lance Taylor <iant@golang.org>
See big comment in code.
Fixes#23921
Change-Id: I2dbd1acc2e9da07a71f9e0640aafe0c59a335627
Reviewed-on: https://go-review.googlesource.com/123875
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The Server distinguishes "new" vs "idle" connections. A TCP connection
from which no bytes have yet been written is "new". A connection that
has previously served a request and is in "keep-alive" state while
waiting for a second or further request is "idle".
The graceful Server.Shutdown historically only shut down "idle"
connections, with the assumption that a "new" connection was about to
read its request and would then shut down on its own afterwards.
But apparently some clients spin up connections and don't end up using
them, so we have something that's "new" to us, but browsers or other
clients are treating as "idle" to them.
This CL tweaks our heuristic to treat a StateNew connection as
StateIdle if it's been stuck in StateNew for over 5 seconds.
Fixes#22682
Change-Id: I01ba59a6ab67755ca5ab567041b1f54aa7b7da6f
Reviewed-on: https://go-review.googlesource.com/121419
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Some headers, which are set or modified by the http library,
are not written to the standard http.Request.Header and are
not included as part of http.Response.Request.Header.
Exposing all headers alleviates this problem.
This is not a complete solution to 19761 since it does not have http/2 support.
Updates #19761
Change-Id: Ie8d4f702f4f671666b120b332378644f094e288b
Reviewed-on: https://go-review.googlesource.com/67430
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Beginning on Go 1.9, ServeMux has been dropping the port number from the Host
header and in the path pattern. This commit explicitly mentions the change in
behavior and adds a simple test case to ensure consistency.
Fixes#23351.
Change-Id: I0270c8bd96cda92c13ac6437cdf66c2807b3042d
Reviewed-on: https://go-review.googlesource.com/120557
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 96575 introduced concurrency protection for
ServeMux.shouldRedirect with a read lock and deferred unlock.
However, the change produced a noticeable regression.
Instead add the suffix "RLocked" to the function name to
declare that we should hold the read lock as a pre-requisite
before calling it, hence avoiding the defer altogether.
Benchmarks:
name old time/op new time/op delta
ServeMux-8 63.3µs ± 0% 54.6µs ± 0% -13.74% (p=0.000 n=9+9)
ServeMux_SkipServe-8 41.4µs ± 2% 32.7µs ± 1% -21.05% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
ServeMux-8 17.3kB ± 0% 17.3kB ± 0% ~ (all equal)
ServeMux_SkipServe-8 0.00B 0.00B ~ (all equal)
name old allocs/op new allocs/op delta
ServeMux-8 360 ± 0% 360 ± 0% ~ (all equal)
ServeMux_SkipServe-8 0.00 0.00 ~ (all equal)
Updates #25383
Updates #25482
Change-Id: I2ffa4eafe165faa961ce23bd29b5653a89facbc2
Reviewed-on: https://go-review.googlesource.com/113996
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Second try. The previous version (CL 115039 in git rev 3988863) wasn't
accurate.
Fixes#22347
Change-Id: I473165f308c730f50b14ba787cb215f7cb9ea364
Reviewed-on: https://go-review.googlesource.com/119235
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Each URL was manually verified to ensure it did not serve up incorrect
content.
Change-Id: I4dc846227af95a73ee9a3074d0c379ff0fa955df
Reviewed-on: https://go-review.googlesource.com/115798
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
This is a followup to CL 110296. That change added a new behavior
to Redirect, where the short HTML body is not written if the
Content-Type header is already set. It was implemented by doing
an early return. That unintentionally prevented the correct status
code from being written, so it would always default to 200.
Existing tests didn't catch this because they don't check status code.
This change fixes that issue by removing the early return and
moving the code to write a short HTML body behind an if statement.
It adds written status code checks to Redirect tests.
It also tries to improve the documentation wording and code style
in TestRedirect_contentTypeAndBody.
Updates #25166.
Change-Id: Idce004baa88e278d098661c03c9523426c5eb898
Reviewed-on: https://go-review.googlesource.com/111517
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use the vendored ValidTrailerHeader function from x/net/http/httpguts to
check Trailer headers according to RFC 7230. The previous implementation
only omitted illegal Trailer headers defined in RFC 2616.
This CL adds x/net/http/httpguts from CL 104042 (git rev a35a21de97)
Fixes#23908
Change-Id: Ib2329a384040494093c18e209db9b62aaf86e921
Reviewed-on: https://go-review.googlesource.com/104075
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The docs for ResponseWriter.Write say
// If the Header
// does not contain a Content-Type line, Write adds a Content-Type set
// to the result of passing the initial 512 bytes of written data to
// DetectContentType.
The header X-Content-Type-Options:nosniff is an explicit directive that
content-type should not be sniffed.
This changes the behavior of Response.WriteHeader so that, when
there is an X-Content-Type-Options:nosniff header, but there is
no Content-type header, the following happens:
1. A Content-type:application/octet-stream is added
2. A warning is logged via the server's logging mechanism.
Previously, a content-type would have been silently added based on
heuristic analysis of the first 512B which might allow a hosted
GIF like http://www.thinkfu.com/blog/gifjavascript-polyglots to be
categorized as JavaScript which might allow a CSP bypass, loading
as a script despite `Content-Security-Policy: script-src 'self' `.
----
https://fetch.spec.whatwg.org/#x-content-type-options-header
defines the X-Content-Type-Options header.
["Polyglots: Crossing Origins by Crossing Formats"](http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.905.2946&rep=rep1&type=pdf)
explains Polyglot attacks in more detail.
Change-Id: I2c8800d2e4b4d10d9e08a0e3e5b20334a75f03c0
Reviewed-on: https://go-review.googlesource.com/89275
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Since that method uses 'mux.m', we need to lock the mutex to avoid data races.
Change-Id: I998448a6e482b5d6a1b24f3354bb824906e23172
GitHub-Last-Rev: 163a7d4942
GitHub-Pull-Request: golang/go#23994
Reviewed-on: https://go-review.googlesource.com/96575
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Replace references to the obsoleted RFC 2616 with references to RFC
7230 through 7235, to avoid unnecessary confusion.
Obvious inconsistencies are marked with todo comments.
Updates #21974
Change-Id: I8fb4fcdd1333fc5193b93a2f09598f18c45e7a00
Reviewed-on: https://go-review.googlesource.com/94095
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Handlers can be registered for specific hosts by specifying the host as
part of the mux pattern. If a trailing slash route is registered for
these host-based patterns, shouldRedirect should indicate that
a redirect is required.
This change modifies shouldRedirect to also take the host of the
request, and now considers host-based patterns while determining if
a request should be redirected.
Fixes#23183
Change-Id: If8753e130d5d877acdc55344833e3b289bbed2b4
Reviewed-on: https://go-review.googlesource.com/84695
Reviewed-by: Kunpei Sakai <namusyaka@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also vendors x/net/http git rev 42fe2e1c for:
http2: don't check WriteHeader status if we've already sent the header
https://golang.org/cl/86255Fixes#23010
Change-Id: I4f3dd63acb52d5a34a0350aaf847a7a376d6968f
Reviewed-on: https://go-review.googlesource.com/86275
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit 08f19bbde1.
Reason for revert:
The changed transformation takes effect on a larger set
of code snippets than expected.
For example, this:
func foo() {
// Comment
bar()
}
becomes:
func foo() {
// Comment
bar()
}
This is an unintended consequence.
Change-Id: Ifca88d6267dab8a8170791f7205124712bf8ace8
Reviewed-on: https://go-review.googlesource.com/81335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Panic if an http Handler does:
rw.WriteHeader(0)
... or other invalid values. (for a forgiving range of valid)
I previously made it kinda work in https://golang.org/cl/19130 but
there's no good way to fake it in HTTP/2, and we want HTTP/1 and
HTTP/2 behavior to be the same, regardless of what programs do.
Currently HTTP/2 omitted the :status header altogether, which was a
protocol violation. In fixing that, I found CL 19130 added a test
about bogus WriteHeader values with the comment:
// This might change at some point, but not yet in Go 1.6.
This now changes. Time to be strict.
Updates golang/go#228800
Change-Id: I20eb6c0e514a31f4bba305ac4c24266f39b95fd5
Reviewed-on: https://go-review.googlesource.com/80077
Reviewed-by: Tom Bergan <tombergan@google.com>
We set Content-Type to "text/plain; charset=utf-8" even with blank body
before. Let's strip this unnecessary header though it's harmless in most
cases.
Fixes#20784
Signed-off-by: Tw <tw19881113@gmail.com>
Change-Id: Ic58a410dcbc89f457c6ddd92961d9cbf545b2f4f
Reviewed-on: https://go-review.googlesource.com/46631
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing implementation names a c net.Conn return which is
never user. Leaving the returns unamed is marginally clearer.
Change-Id: If9a411c9235b78c116a8ffb21fef71f7a4a4ce8f
Reviewed-on: https://go-review.googlesource.com/66890
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
To improve readability when exported fields are removed,
forbid the printer from emitting an empty line before the first comment
in a const, var, or type block.
Also, when printing the "Has filtered or unexported fields." message,
add an empty line before it to separate the message from the struct
or interfact contents.
Before the change:
<<<
type NamedArg struct {
// Name is the name of the parameter placeholder.
//
// If empty, the ordinal position in the argument list will be
// used.
//
// Name must omit any symbol prefix.
Name string
// Value is the value of the parameter.
// It may be assigned the same value types as the query
// arguments.
Value interface{}
// contains filtered or unexported fields
}
>>>
After the change:
<<<
type NamedArg struct {
// Name is the name of the parameter placeholder.
//
// If empty, the ordinal position in the argument list will be
// used.
//
// Name must omit any symbol prefix.
Name string
// Value is the value of the parameter.
// It may be assigned the same value types as the query
// arguments.
Value interface{}
// contains filtered or unexported fields
}
>>>
Fixes#18264
Change-Id: I9fe17ca39cf92fcdfea55064bd2eaa784ce48c88
Reviewed-on: https://go-review.googlesource.com/71990
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
In CL 50510, the Content-Type header started to be set in Redirect when
request method is GET. (Prior to that, it wasn't set at all, which is
what said CL was fixing.) However, according to HTTP specification,
the expected response for a HEAD request is identical to that of a
GET request, but without the response body.
This CL updates the behavior to set the Content-Type header for HEAD
method in addition to GET.
This actually allows a simpler implementation than before. This change
largely reverts CL 50510, and applies the simpler implementation.
Add a test for Content-Type header and body for GET, HEAD requests.
Updates CL 50510.
Change-Id: If33ea3f4bbc5246bb5dc751458004828cfe681b9
Reviewed-on: https://go-review.googlesource.com/65190
Run-TryBot: Dmitri Shuralyov <shurcool@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
Fixes#22084.
Change-Id: If405ffdc57fcf81de3c0e8473c45fc504db735bc
Reviewed-on: https://go-review.googlesource.com/67410
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
strings.IndexByte was introduced in go1.2 and it can be used
effectively wherever the second argument to strings.Index is
exactly one byte long.
This avoids generating unnecessary string symbols and saves
a few calls to strings.Index.
Change-Id: I1ab5edb7c4ee9058084cfa57cbcc267c2597e793
Reviewed-on: https://go-review.googlesource.com/65930
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ensure that the implicitly created redirect
for
"/route"
after
"/route/"
has been registered doesn't lose the query string information.
A previous attempt (https://golang.org/cl/43779) changed http.Redirect, however, that change broke direct calls to http.Redirect.
To avoid that problem, this change touches ServeMux.Handler only.
Fixes#17841
Change-Id: I303c1b1824615304ae68147e254bb41b0ea339be
Reviewed-on: https://go-review.googlesource.com/61210
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Fixes#20933
Change-Id: I827c8d265674a7448c51c1da991e9b3b4501ba11
Reviewed-on: https://go-review.googlesource.com/59850
Run-TryBot: Tom Bergan <tombergan@google.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In TimeoutHandler, use a request whose context has been configured with
the handler's timeout
Fixes#20712
Change-Id: Ie670148f85fdad46841ff29232042309e15665ae
Reviewed-on: https://go-review.googlesource.com/46412
Run-TryBot: Tom Bergan <tombergan@google.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
Went mainly for the ones that make no sense, such as the ones
mid-sentence or after commas.
Change-Id: Ie245d2c19cc7428a06295635cf6a9482ade25ff0
Reviewed-on: https://go-review.googlesource.com/57293
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the existing implementation, if pattern is an empty string,
program calls a panic with the message which is a concatenation of
"http: invalid pattern " and pattern.
In this case, pattern is an empty, so the commit removes
this concatenation and the trailing space.
Fixes: #21102
Change-Id: I49f58b52d835311a6ac642de871eb15646e48a54
Reviewed-on: https://go-review.googlesource.com/50350
Reviewed-by: Dmitri Shuralyov <shurcool@gmail.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that issue #12438 is resolved, this TODO can be completed.
Create a logf helper, which is similar to Server.logf method,
but takes a *Request to infer the *Server and its ErrorLog from.
Update documentation of Server.ErrorLog to mention a new type
of errors that may be logged to it.
Also update a statement in documentation of Server.ErrorLog from:
// If nil, logging goes to os.Stderr via the log package's
// standard logger.
To:
// If nil, logging is done via the log package's standard logger.
The motivation for doing so is to avoid making inaccurate claims.
Logging may not go to os.Stderr if anyone overrides the log package's
default output via https://godoc.org/log#SetOutput. Saying that
the standard logger is used should be sufficient to explain the
behavior, and users can infer that os.Stderr is used by default,
unless it's changed.
Updates #12438.
Change-Id: I3a4b0db51d652fd25fb2065fbc2157a3dec4dd38
Reviewed-on: https://go-review.googlesource.com/53950
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Setting the Content-Type header explicitly allows browsers to know what
the type of the content is. Otherwise, they have to guess the type from
the content itself, which could lead to unpredictable behavior, and
increases CPU usage.
Not setting the Content-Type despite writing a body may also trigger
unwanted warnings in user middleware, and make it more difficult to
resolve valid issues where the user forgets to set Content-Type in
some situations where it should be set.
There is some precedent for doing this in http.FileServer, which
sets "Content-Type" to "text/html; charset=utf-8" before writing
<pre><a href=...></a></pre> HTML.
Change-Id: I24286827bebf4da8adee9238b8c5a94d4069c8db
Reviewed-on: https://go-review.googlesource.com/50510
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We can make it panic with a more explicit and readable error message
during Go 1.10, but document it for now. This has always been the
case; it's not a new rule.
Updates #20933
Change-Id: I53c1fefb47a8f4aae0bb32fa742afa3a2ed20e8a
Reviewed-on: https://go-review.googlesource.com/50634
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
In CL https://golang.org/cl/4893043 (6 years ago), a new package named
"url" was created (it is currently known as "net/url"). During that
change, some identifier name collisions were introduced, and two
parameters in net/http were renamed to "urlStr".
Since that time, Go has continued to put high emphasis on the quality
and readability of the documentation. Sometimes, that means making small
sacrifices in the implementation details of a package to ensure that
the godoc reads better, since that's what the majority of users interact
with. See https://golang.org/s/style#named-result-parameters:
> Clarity of docs is always more important than saving a line or two
> in your function.
I think the "urlStr" parameter name is suboptimal for godoc purposes,
and just "url" would be better.
During the review of https://golang.org/cl/4893043, it was also noted
by @rsc that having to rename parameters named "url" was suboptimal:
> It's unfortunate that naming the package url means
> you can't have a parameter or variable named url.
However, at the time, the name of the url package was still being
decided, and uri was an alternative name under consideration.
The reason urlStr was chosen is because it was a lesser evil
compared to naming the url package uri instead:
> Let's not get hung up on URI vs. URL, but I'd like s/uri/urlStr/ even for just
> that the "i" in "uri" looks very similar to the "l" in "url" in many fonts.
> Please let's go with urlStr instead of uri.
Now that we have the Go 1 compatibility guarantee, the name of the
net/url package is fixed. However, it's possible to improve the
signature of Redirect, NewRequest functions in net/http package
for godoc purposes by creating a package global alias to url.Parse,
and renaming urlStr parameter to url in the exported funcs. This CL
does so.
Updates #21077.
Change-Id: Ibcc10e3825863a663e6ad91b6eb47b1862a299a6
Reviewed-on: https://go-review.googlesource.com/49930
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Updates #18997
Change-Id: Ib1961a4c26b42f99b98b255beb7e2a74b632e0c1
Reviewed-on: https://go-review.googlesource.com/48551
Reviewed-by: Joe Shaw <joe@joeshaw.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
Server.ServeTLS wraps Server.Serve with added TLS support. This is
particularly useful for serving on manually initialized listeners.
Example use-case includes ability to serve with TLS on listener
provided by systemd's socket activation.
A matching test heavily based on TestAutomaticHTTP2_ListenAndServe
is also included.
Original code by Gurpartap Singh as
https://go-review.googlesource.com/c/38114/Fixes#13228
Change-Id: I73bb703f501574a84d261c2d7b9243a89fa52d62
Reviewed-on: https://go-review.googlesource.com/44074
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 43779/commit 6a6c792eef
broke the builds at tip, and that CL doesn't account for
cases where Redirect is directly invoked with a full URL
that itself has a query string.
Updates #17841
Change-Id: Idb0486bae8625e1f9e033ca4cfcd87de95bc835c
Reviewed-on: https://go-review.googlesource.com/44100
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This will be used to allow http2 servers to register a shutdown function
so that net/http.Server.Shutdown will work when the http2 server is
configured via a manual call to http2.ConfigureServer. Currently, Shutdown
only works when the http2 server is configured automatically by the
net/http package.
Updates #20302
Updates #18471
Change-Id: Ifc2b5f3126126a106b49ea4a7e999279852b9cc9
Reviewed-on: https://go-review.googlesource.com/44003
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Ensure that the implicitly created redirect
for
"/route"
after
"/route/"
has been registered doesn't lose the query string information.
Fixes#17841.
Change-Id: Ib7df9242fab8c9368a18fc0da678003d6bec63b8
Reviewed-on: https://go-review.googlesource.com/43779
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It used to be simple, and then it got complicated for speed (to reduce
allocations, mostly), but that involved a mutex and hurt multi-core
performance, contending on the mutex.
A change was sent to try to improve that mutex contention in
https://go-review.googlesource.com/c/42110/2/src/net/http/server.go
but that introduced its own allocations (the string->interface{}
boxing for the sync.Map key), which runs counter to the whole point of
that statusLine function: to remove allocations.
Instead, make the code simple again and not have a mutex. It's a bit
slower for the single-core case, but nobody with a single-user HTTP
server cares about 50 nanoseconds:
name old time/op new time/op delta
ResponseStatusLine 37.5ns ± 2% 87.1ns ± 2% +132.42% (p=0.029 n=4+4)
ResponseStatusLine-2 63.1ns ± 1% 43.1ns ±12% -31.67% (p=0.029 n=4+4)
ResponseStatusLine-4 53.8ns ± 8% 40.2ns ± 2% -25.29% (p=0.029 n=4+4)
name old alloc/op new alloc/op delta
ResponseStatusLine 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
ResponseStatusLine-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
ResponseStatusLine-4 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
name old allocs/op new allocs/op delta
ResponseStatusLine 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
ResponseStatusLine-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
ResponseStatusLine-4 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
(Note the code could be even simpler with fmt.Fprintf, but that is
relatively slow and involves a bunch of allocations getting arguments
into interface{} for the call)
Change-Id: I1fa119132dbbf97a8e7204ce3e0707d433060da2
Reviewed-on: https://go-review.googlesource.com/42133
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
The LocalAddrContext should have the network address of the actual
interface.
Fixes#18686
Change-Id: I9c401eda312f3a0e7e65b013af827aeeef3b4d3d
Reviewed-on: https://go-review.googlesource.com/35490
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change strips the port in mux.Handler before attempting to
match handlers and adds a test for a request with port.
CONNECT requests continue to use the original path and port.
Fixes#10463
Change-Id: Iff3a2ca2b7f1d884eca05a7262ad6b7dffbcc30f
Reviewed-on: https://go-review.googlesource.com/38194
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The DefaultServeMux included in net/http uses a map to store routes,
but iterates all keys for every request to allow longer paths.
This change checks the map for an exact match first.
To check performance was better, BenchmarkServeMux has been added -
this adds >100 routes and checks the matches.
Exact matches are faster and more predictable on this benchmark
and on most existing package benchmarks.
https://perf.golang.org/search?q=upload:20170312.1
ServeMux-4 2.02ms ± 2% 0.04ms ± 2% −98.08% (p=0.004 n=5+6)
https://perf.golang.org/search?q=upload:20170312.2
ReadRequestChrome-4 184MB/s ± 8% 186MB/s ± 1% ~
ReadRequestCurl-4 45.0MB/s ± 1% 46.2MB/s ± 1% +2.71%
Read...Apachebench-4 45.8MB/s ±13% 48.7MB/s ± 1% ~
ReadRequestSiege-4 63.6MB/s ± 5% 69.2MB/s ± 1% +8.75%
ReadRequestWrk-4 30.9MB/s ± 9% 34.4MB/s ± 2% +11.25%
Change-Id: I8afafcb956f07197419d545a9f1c03ecaa307384
Reviewed-on: https://go-review.googlesource.com/38057
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As of https://golang.org/cl/21530, rules are updated to state
that Handlers shouldn't modify the provided Request. This change
updates StripPrefix to follow that rule.
Resolves#18952.
Change-Id: I29bbb580722e871131fa75a97e6e038ec64fdfcd
Reviewed-on: https://go-review.googlesource.com/36483
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, if the Hijack called stopped the background read call
which read a byte, that byte was sitting in memory, buffered, ready to
be Read by Hijack's returned bufio.Reader, but it wasn't yet in the
bufio.Reader's buffer itself, so bufio.Reader.Buffered() reported 1
byte fewer.
This matters for callers who wanted to stitch together any buffered
data (with bufio.Reader.Peek(bufio.Reader.Buffered())) with Hijack's
returned net.Conn. Otherwise there was no way for callers to know a
byte was read.
Change-Id: Id7cb0a0a33fe2f33d79250e13dbaa9c0f7abba13
Reviewed-on: https://go-review.googlesource.com/35232
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Fixes#18447
Change-Id: I5d60c3632a5ce625d3bac9d85533ce689e301707
Reviewed-on: https://go-review.googlesource.com/34813
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As discussed in #18095 the server should not log for bad user input.
Change-Id: I628a796926eff3a971e5b04abec17ea377c3f9b7
Reviewed-on: https://go-review.googlesource.com/33617
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If the Server's Shutdown (or SetKeepAlivesEnabled) method was called
while a connection was in a Handler, but after the headers had been
written, the connection was not later closed.
Fixes#9478
Updates #17754 (reverts that workaround)
Change-Id: I65324ab8217373fbb38e12e2b8bffd0a91806072
Reviewed-on: https://go-review.googlesource.com/33141
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Updates http2 to x/net/http2 git rev 0e2717d for:
http2: conditionally log stacks from panics in Server Handlers like net/http
https://golang.org/cl/33102Fixes#17790
Change-Id: Idd3f0c65540398d41b412a33f1d80de3f7f31409
Reviewed-on: https://go-review.googlesource.com/33103
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
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>
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>
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>
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>
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 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>
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>
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>
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>
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>
Johan Brandhorst
Dan Peterson
Russ Cox
Ross Light
Damien Neil
Michael Fraenkel
Paul Forgey
Andrew Gerrand
Filippo Valsorda
Brad Fitzpatrick
Robert Kuska
Roman Kollár
pokutuna
Dmitri Shuralyov
Emmanuel T Odeke
Davor Kapsa
Anderson Queiroz
Kevin Gillette
Jeff Hodges
Raghavendra Nagaraj
Andrew Bonventre
Leo Antunes
Bryan C. Mills
Daniel Martí
LE Manh Cuong
Ggicci
Tobias Klauser
Carl Mastrangelo
David Url
Tim Cooper
Meir Fischer
Guilherme Goncalves
Dmitri Shuralyov
Sam Whited
Mike Samuel
Ryan Slade
Kunpei Sakai
Damien Mathieu
Terin Stock
Artyom Pervukhin
Tim Heckman
Joe Tsai
Tw
Ian Kent
mattyw
Joe Tsai
Dmitri Shuralyov
Marvin Stenger
Naoki Kanatani
Ian Lance Taylor
Qiuxuan Zhu
Niklas Schnelle
Emmanuel Odeke
Tom Bergan
Chris Manghane
Filippo Valsorda
Kenny Grant
Koki Ide
Kale Blankenship