When a Content-Type that triggers content sniffing in old (but still in
significant use) browsers is sent, add the
X-Content-Type-Options: nosniff header, unless explicitly disabled.
Expose httpguts.SniffedContentType for use in the HTTP 1 implementation.
Will be tested by net/http.TestNoSniffHeader_h2.
Updates golang/go#24513
Change-Id: Id1ffea867a496393cb52c5a9f45af97d4b2fcf12
Reviewed-on: https://go-review.googlesource.com/112015
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
httplex was the original package name for shared code between net/http
and x/net/http2, but its name was too specific, and http/httpguts was
added later for other shared code.
We discussed merging httplex into httpguts at the time, but it didn't
happen earlier. This finishes the move.
Updates golang/go#23908
Change-Id: Ic7d6f39e584ca579d34b5ef5ec6a0c002a38a83c
Reviewed-on: https://go-review.googlesource.com/111875
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, we close the connection immediately after sending a GOAWAY
frame if all outstanding responses have been completely sent. However,
the client may have had requests in-flight at that time which have been
queued in the kernel receive buffer. On both Windows and Linux, if the
connection is close()'d when the receive buffer is not empty, the kernel
sends RST. This has the effect of aborting both sides of the connection,
meaning the client may not actually receive all responses that were sent
before the GOAWAY.
Instead, we should delay calling close() until after the receive buffer
has been drained. We don't want to delay indefinitely, which means we
need some kind of timeout. Ideally that timeout should be about 1 RTT +
epsilon, under the assumption that the client will not send any more
frames after receiving the GOAWAY. However, 1 RTT is difficult to
measure. It turns out we were already using a 1 second delay in other
cases, so we reuse that same delay here.
Note that we do not call CloseWrite() to half-close the underlying TLS
connection. This seems unnecessary -- GOAWAY is effectively a half-close
at the HTTP/2 level.
Updates golang/go#18701 (fixes after it's bundled into net/http)
Change-Id: I4d68bada6369ba95e5db02afe6dfad0a393c0334
Reviewed-on: https://go-review.googlesource.com/71372
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
resetStream(st) queues a RST_STREAM frame and then calls closeStream(st).
Unfortunately, closeStream(st) flushes any writes pending on st, which
can drop the RST_STREAM that was just queued. (If we are lucky, the
RST_STREAM will fit in the send buffer and won't be dropped, however,
if we are unlucky the RST_STREAM will be dropped.)
I fixed this bug by removing closeStream(st) from resetStream. Instead,
closeStream happens after the RST_STREAM frame is written. This is a more
direct implementation of the diagram in RFC 7540 Section 5.1, which says
that a stream does not transition to "closed" until after the RST_STREAM
has been sent.
A side-effect is that the stream may stay open for longer than it did
previously (since it won't close until *after* the RST_STREAM frame is
actually written). Situations like the following are a problem:
- Client sends a DATA frame that exceeds its flow-control window
- Server returns streamError(ErrCodeFlowControl) from processData
- RST_STREAM is queued behind other frames
- Server process the request body and releases flow-control
- Client sends another DATA frame, this one fits in the flow-control
window. Server should NOT process this frame.
To avoid the above problem, we set a bool st.resetQueued=true when
RST_STREAM is queued, then ignore all future incoming HEADERS and DATA
frames on that stream.
I also removed st.sentReset and st.gotReset, which were used to ignore
frames after RST_STREAM is sent. Now we just check if the stream is closed.
Fixesgolang/go#18111
Change-Id: Ieb7c848989431add5b7d95f40d6d91db7edc4980
Reviewed-on: https://go-review.googlesource.com/34238
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes x/net/http2's ResponseWriter implement the new interface,
http.Pusher. This new interface requires Go 1.8. When compiled against
older versions of Go, the ResponseWriter does not have a Push method.
Fixesgolang/go#13443
Change-Id: I8486ffe4bb5562a94270ace21e90e8c9a4653da0
Reviewed-on: https://go-review.googlesource.com/29439
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Don't do async frame writes when the write is known to be small enough
to definitely fit in the write buffer and not cause a flush (which
might block). This avoids starting a new goroutine and doing a channel
send operation for many frame writes.
name old time/op new time/op delta
ServerGets-4 146µs ± 2% 144µs ± 1% -1.46% (p=0.000 n=14+14)
ServerPosts-4 162µs ± 1% 160µs ± 1% -1.15% (p=0.000 n=15+15)
Server_GetRequest-4 145µs ± 1% 143µs ± 0% -1.26% (p=0.000 n=15+15)
Server_PostRequest-4 160µs ± 1% 158µs ± 1% -1.32% (p=0.000 n=15+15)
The headers frame is the main last one which might show some benefit
if there's a cheap enough way to conservatively calculate its size.
Change-Id: I9be2ddbf04689340819d8701ea671fff378d9e79
Reviewed-on: https://go-review.googlesource.com/31495
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The http2.Transport was able to send bogus header keys & values.
This changes rejects them earlier, before they hit the wire.
In the process, mirror the lexical rules from the http package to x/net.
Maintaining two copies has gotten increasingly annoying.
Updates golang/go#14048
Change-Id: I20abcdeea92e7dc8706a1bbd60688ee8843a2b12
Reviewed-on: https://go-review.googlesource.com/23229
Reviewed-by: Andrew Gerrand <adg@golang.org>
This adds a way for http.Handlers to set trailers after the header has
already been flushed. This comment from the code explains:
// promoteUndeclaredTrailers permits http.Handlers to set trailers
// after the header has already been flushed. Because the Go
// ResponseWriter interface has no way to set Trailers (only the
// Header), and because we didn't want to expand the ResponseWriter
// interface, and because nobody used trailers, and because RFC 2616
// says you SHOULD (but not must) predeclare any trailers in the
// header, the official ResponseWriter rules said trailers in Go must
// be predeclared, and then we reuse the same ResponseWriter.Header()
// map to mean both Headers and Trailers. When it's time to write the
// Trailers, we pick out the fields of Headers that were declared as
// trailers. That worked for a while, until we found the first major
// user of Trailers in the wild: gRPC (using them only over http2),
// and gRPC libraries permit setting trailers mid-stream without
// predeclarnig them. So: change of plans. We still permit the old
// way, but we also permit this hack: if a Header() key begins with
// "Trailer:", the suffix of that key is a Trailer. Because ':' is an
// invalid token byte anyway, there is no ambiguity. (And it's already
// filtered out) It's mildly hacky, but not terrible.
The official pre-declaring way still works. Example from net/http docs:
https://golang.org/pkg/net/http/#example_ResponseWriter_trailers
And ResponseWriter docs explaining it:
https://golang.org/pkg/net/http/#ResponseWriter
I don't want to add a new interface-assertable upgrade type (like
Hijacker or Flusher) for this because it's hurts composability and
makes everybody in the ecocsystem implement those, and two optional
interfaces is already bad enough. This is a weird enough feature
(Trailers by itself is weird enough), that I don't feel like a third
optional interface is worth it.
This code also filters invalid header fields (updates golang/go#14048),
since I had to update that code as part of this. But I want to later
return an error back to the user if possible. Or log something.
With this CL, all the grpc-go end2end tests pass with a new http2-based
Server implementation:
a06f8f0593
Change-Id: I80f863d05a1810bd6f302f34932ad9df0a6646a6
Reviewed-on: https://go-review.googlesource.com/19131
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
GODEBUG=h2debug=1 is now GODEBUG=http2debug=1, per golang/go#13611
(comment https://github.com/golang/go/issues/13611#issuecomment-169534496)
and there is a new debugging level h2debug=2 for logging all written frames
as well, which was code I originally wrote for debugging the lego/ACME/Akamai
issues in golang/go#13637 and letsencrypt/boulder#1279.
This also moves the common vlogf calls inside if VerboseLogs blocks,
to avoid allocations. I didn't do the rare ones.
Example client output, fetching https://ip.appspot.com/:
2016/01/07 23:24:52 http2: Transport failed to get client conn for ip.appspot.com:443: http2: no cached connection was available
2016/01/07 23:24:52 http2: Transport creating client conn to 64.233.183.141:443
2016/01/07 23:24:52 http2: Framer 0xc82028c420: wrote SETTINGS len=12, settings: ENABLE_PUSH=0, INITIAL_WINDOW_SIZE=4194304
2016/01/07 23:24:52 http2: Framer 0xc82028c420: wrote WINDOW_UPDATE len=4 (conn) incr=1073741824
2016/01/07 23:24:52 http2: Framer 0xc82028c420: wrote SETTINGS flags=ACK len=0
2016/01/07 23:24:52 http2: Transport encoding header ":authority" = "ip.appspot.com"
2016/01/07 23:24:52 http2: Transport encoding header ":method" = "GET"
2016/01/07 23:24:52 http2: Transport encoding header ":path" = "/"
2016/01/07 23:24:52 http2: Transport encoding header ":scheme" = "https"
2016/01/07 23:24:52 http2: Transport encoding header "accept-encoding" = "gzip"
2016/01/07 23:24:52 http2: Transport encoding header "user-agent" = "Go-http-client/2.0"
2016/01/07 23:24:52 http2: Transport received WINDOW_UPDATE len=4 (conn) incr=983041
2016/01/07 23:24:52 http2: Framer 0xc82028c420: wrote HEADERS flags=END_STREAM|END_HEADERS stream=1 len=35
2016/01/07 23:24:52 http2: Transport received SETTINGS flags=ACK len=0
2016/01/07 23:24:52 http2: Transport received HEADERS flags=END_HEADERS stream=1 len=123
2016/01/07 23:24:52 http2: Transport decoded header field ":status" = "200"
2016/01/07 23:24:52 http2: Transport decoded header field "content-type" = "text/plain;"
2016/01/07 23:24:52 http2: Transport decoded header field "date" = "Thu, 07 Jan 2016 23:24:52 GMT"
2016/01/07 23:24:52 http2: Transport decoded header field "server" = "Google Frontend"
2016/01/07 23:24:52 http2: Transport decoded header field "content-length" = "14"
2016/01/07 23:24:52 http2: Transport decoded header field "alternate-protocol" = "443:quic,p=1"
2016/01/07 23:24:52 http2: Transport decoded header field "alt-svc" = "quic=\":443\"; ma=604800; v=\"30,29,28,27,26,25\""
2016/01/07 23:24:52 http2: Transport received DATA flags=END_STREAM stream=1 len=14 data="146.148.92.232"
2016/01/07 23:24:52 http2: Transport received PING len=8 ping="\x00\x00\x00\x00\x00\x00\x00\x00"
2016/01/07 23:24:52 http2: Framer 0xc82028c420: wrote PING flags=ACK len=8 ping="\x00\x00\x00\x00\x00\x00\x00\x00"
Example server output, with a client fetching / and getting a 404:
2016/01/07 23:25:45 http2: server connection from 72.14.229.81:58273 on 0xc820066100
2016/01/07 23:25:45 http2: server: error reading preface from client 72.14.229.81:58273: EOF
2016/01/07 23:25:45 http2: Framer 0xc820228210: wrote SETTINGS len=18, settings: MAX_FRAME_SIZE=1048576, MAX_CONCURRENT_STREAMS=250, MAX_HEADER_LIST_SIZE=1048896
2016/01/07 23:25:45 http2: server connection from 72.14.229.81:6801 on 0xc820066100
2016/01/07 23:25:45 http2: Framer 0xc8202e8370: wrote SETTINGS len=18, settings: MAX_FRAME_SIZE=1048576, MAX_CONCURRENT_STREAMS=250, MAX_HEADER_LIST_SIZE=1048896
2016/01/07 23:25:45 http2: server: client 72.14.229.81:6801 said hello
2016/01/07 23:25:45 http2: server read frame SETTINGS len=12, settings: MAX_CONCURRENT_STREAMS=1000, INITIAL_WINDOW_SIZE=6291456
2016/01/07 23:25:45 http2: server processing setting [MAX_CONCURRENT_STREAMS = 1000]
2016/01/07 23:25:45 http2: server processing setting [INITIAL_WINDOW_SIZE = 6291456]
2016/01/07 23:25:45 http2: Framer 0xc8202e8370: wrote SETTINGS flags=ACK len=0
2016/01/07 23:25:45 http2: server read frame WINDOW_UPDATE len=4 (conn) incr=15663105
2016/01/07 23:25:45 http2: server read frame HEADERS flags=END_STREAM|END_HEADERS|PRIORITY stream=1 len=238
2016/01/07 23:25:45 http2: server decoded header field ":method" = "GET"
2016/01/07 23:25:45 http2: server decoded header field ":authority" = "(redacted):4430"
2016/01/07 23:25:45 http2: server decoded header field ":scheme" = "https"
2016/01/07 23:25:45 http2: server decoded header field ":path" = "/"
2016/01/07 23:25:45 http2: server decoded header field "cache-control" = "max-age=0"
2016/01/07 23:25:45 http2: server decoded header field "accept" = "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"
2016/01/07 23:25:45 http2: server decoded header field "upgrade-insecure-requests" = "1"
2016/01/07 23:25:45 http2: server decoded header field "user-agent" = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537."
2016/01/07 23:25:45 http2: server decoded header field "accept-encoding" = "gzip, deflate, sdch"
2016/01/07 23:25:45 http2: server decoded header field "accept-language" = "en-US,en;q=0.8"
2016/01/07 23:25:45 http2: server encoding header ":status" = "404"
2016/01/07 23:25:45 http2: server encoding header "content-type" = "text/plain; charset=utf-8"
2016/01/07 23:25:45 http2: server encoding header "x-content-type-options" = "nosniff"
2016/01/07 23:25:45 http2: server encoding header "content-length" = "19"
2016/01/07 23:25:45 http2: server encoding header "date" = "Thu, 07 Jan 2016 23:25:45 GMT"
2016/01/07 23:25:45 http2: Framer 0xc8202e8370: wrote HEADERS flags=END_HEADERS stream=1 len=73
2016/01/07 23:25:45 http2: Framer 0xc8202e8370: wrote DATA flags=END_STREAM stream=1 len=19 data="404 page not found\n"
2016/01/07 23:25:45 http2: server read frame SETTINGS flags=ACK len=0
Change-Id: Ifb3fe4e588ff54abd8bc3facbb419c3c3809bcba
Reviewed-on: https://go-review.googlesource.com/18372
Reviewed-by: Blake Mizerany <blake.mizerany@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Fixes found in the process of adding more A/B tests to net/http,
comparing HTTP/1 and HTTP/2 behaviors.
Most of the new tests are in Gerrit change Id9c45fad44cdf70ac9
in the "go" repo.
Fixesgolang/go#13315Fixesgolang/go#13316Fixesgolang/go#13317
Fixes other stuff found in the process too
Updates golang/go#6891 (http2 support in general)
Change-Id: I83b5bfb471047312c0dcb0a0b21d709008f34136
Reviewed-on: https://go-review.googlesource.com/17204
Reviewed-by: Andrew Gerrand <adg@golang.org>
This changes makes sure we never write to *writeData in the ServeHTTP
goroutine until the serve goroutine is done with it.
Also, it makes sure we don't transition the stream to the closed state
on the final DATA frame concurrently with the write.
To fix both, the writeFrameAsync goroutine no longer replies directly back
to the ServeHTTP goroutine with the write result. It's now passed to
the serve goroutine instead, which looks at the frameWriteMsg to
decide how to advance the state machine, then signals the ServeHTTP
goroutine with the result, and then advances the state machine.
Because advancing the state machine could transition it to closed,
which the ServeHTTP goroutine might also be selecting on, make the
ServeHTTP goroutine prefer its frameWriteMsg response channel for errors
over the stream closure in its select.
Various code simplifications and robustness in the process.
Tests now pass reliably even with high -count values, -race on/off,
etc. I've been unable to make h2load be unhappy now either.
Thanks to Tatsuhiro Tsujikawa (Github user @tatsuhiro-t) for the bug
report and debugging clues.
Fixesgolang/go#12998
Change-Id: I441c4c9ca928eaba89fd4728d213019606edd899
Reviewed-on: https://go-review.googlesource.com/16063
Reviewed-by: Andrew Gerrand <adg@golang.org>
