Add a new inflow type for tracking inbound flow control.
An inflow tracks both the window sent to the peer, and the
window we are willing to send. Updates are accumulated and
sent in a batch when the unsent window update is large
enough.
This change makes both the client and server use the same
algorithm to decide when to send window updates. This should
slightly reduce the rate of updates sent by the client, and
significantly reduce the rate sent by the server.
Fix a client flow control tracking bug: When processing data
for a canceled stream, the record of flow control consumed
by the peer was not updated to account for the discard
stream.
Fixesgolang/go#28732Fixesgolang/go#56558
Change-Id: Id119d17b84b46f3dc2719f28a86758d9a10085d9
Reviewed-on: https://go-review.googlesource.com/c/net/+/448155
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
The canonical header cache is a per-connection cache mapping header
keys to their canonicalized form. (For example, "foo-bar" => "Foo-Bar").
We limit the number of entries in the cache to prevent an attacker
from consuming unbounded amounts of memory by sending many unique
keys, but a small number of very large keys can still consume an
unreasonable amount of memory.
Track the amount of memory consumed by the cache and limit it based
on memory rather than number of entries.
Thanks to Josselin Costanzi for reporting this issue.
For golang/go#56350
Change-Id: I41db4c9823ed5bf371a9881accddff1268489b16
Reviewed-on: https://go-review.googlesource.com/c/net/+/455635
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add support for handling of SETTINGS_HEADER_TABLESIZE in SETTINGS
frames.
Add http2.Transport.MaxDecoderHeaderTableSize to set the advertised
table size for new client connections. Add
http2.Transport.MaxEncoderHeaderTableSize to cap the accepted size for
new client connections.
Add http2.Server.MaxDecoderHeaderTableSize and MaxEncoderHeaderTableSize
to do the same on the server.
Fixesgolang/go#29356Fixesgolang/go#56054
Change-Id: I16ae0f84b8527dc1e09dfce081e9f408fd514513
Reviewed-on: https://go-review.googlesource.com/c/net/+/435899
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
When processing an HTTP/1 Upgrade: h2c request, detect errors reading
the request body and fail the request rather than passing off the
partially-read request to the HTTP/2 server.
Correctly handles the case where a MaxBytesHandler has limited the
size of the initial HTTP/1 request body.
Fixesgolang/go#56352
Change-Id: I08d60953cea26961cffbab3094cc1b44236f4e37
Reviewed-on: https://go-review.googlesource.com/c/net/+/447396
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: John Howard <howardjohn@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
The existing implementation does not reset gz.zr. After Close,
gzipReader closes underlying response body but buffered data can still
be read.
gzipReader on Close sets the gz.zerr to fs.ErrClosed so next Read after
Close will return it immediately.
Fixesgolang/go#56020
Change-Id: I8a31e4c65656b9abc3023855b8e04342e1e77cbb
Reviewed-on: https://go-review.googlesource.com/c/net/+/440555
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Currently Server builds two process-global maps to cut down allocations
due to lower-casing and canonicalization of common headers.
Lower-casing/canonicalization has also been a significant source of
garbage in Transport - this change extends use of the same
process-global maps to the client.
Change-Id: I2324c9567a61f28d4dd633a2c0618f08ddbf457c
Reviewed-on: https://go-review.googlesource.com/c/net/+/442175
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
This change adds additional common CORS headers and two de-facto
standard and common X- headers to the shared headermap cache to cut down
on allocations from lower-casing/canonicalization.
Change-Id: I61121925b0b28414ed6ce07190155662b0444f93
Reviewed-on: https://go-review.googlesource.com/c/net/+/442176
Auto-Submit: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
After sending a GOAWAY with NO_ERROR, we should discard all frames
for streams with larger identifiers than the last stream identifier
in the GOAWAY frame. We weren't discarding RST_STREAM frames, which
could cause us to incorrectly detect a protocol error when handling
a RST_STREAM for a discarded stream.
Hoist post-GOAWAY frame discarding higher in the loop rather than
handling it on a per-frame-type basis.
We are also supposed to count discarded DATA frames against
connection-level flow control, possibly sending WINDOW_UPDATE
messages to return the flow control. We weren't doing this;
this is now fixed.
Fixesgolang/go#55846
Change-Id: I7603a529c00b8637e648eee9cc4608fb5fa5199b
Reviewed-on: https://go-review.googlesource.com/c/net/+/434909
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: LI ZHEN <mr.imuz@gmail.com>
Reviewed-by: Antonio Ojea <antonio.ojea.garcia@gmail.com>
CL 432038 reduces sending WindowUpdates by introducing a threshold. Once
the remaining bytes are below the threshold, a single WindowUpdate is
sent to reset the amount back to the maximum amount configured.
The window increment size for a stream is calculated from:
sc.srv.initialStreamRecvWindowSize() - st.inflow.available()
Where (*flow).available is defined as:
func (f *flow) available() int32 {
n := f.n
if f.conn != nil && f.conn.n < n {
n = f.conn.n
}
return n
}
When f.conn.c < f.n, it gets a bigger increment size. It should be
calculated from:
sc.srv.initialStreamRecvWindowSize() - st.inflow.n
While we're here, remove an unnecessary type conversion too.
Updates golang/go#56315.
Change-Id: I4b26b27e4c5c5cd66e6a32b152d68f304adc65d8
GitHub-Last-Rev: 02fc09c1e7
GitHub-Pull-Request: golang/net#155
Reviewed-on: https://go-review.googlesource.com/c/net/+/444816
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
As the static version of headerFieldTable in hpack is generated in runtime,
we may use the go:generate to prepare the struct before the initialization phase.
This is supposed to save init time and allocations for many binaries,
as net/http imports hpack.
Before:
init golang.org/x/net/http2/hpack @1.1 ms, 0.097 ms clock, 21240 bytes, 29 allocs
After:
init golang.org/x/net/http2/hpack @0.67 ms, 0.015 ms clock, 8120 bytes, 9 allocs
Fixesgolang/go#55881
Change-Id: Ia6575f67ffcba7cc4d75899b24a9c56deb58ccac
Reviewed-on: https://go-review.googlesource.com/c/net/+/434875
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Moving the Request.Body.Close call out from the ClientConn mutex
results in some cases where RoundTrip returns while the Close is
still in progress. This should be legal (RoundTrip explicitly allows
for this), but net/http relies on Close never being called after
RoundTrip returns.
Add additional synchronization to ensure Close calls complete
before RoundTrip returns.
Fixesgolang/go#55896
Change-Id: Ie3d4773966745e83987d219927929cb56ec1a7ad
Reviewed-on: https://go-review.googlesource.com/c/net/+/435535
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The existing implementation holds a lock on a connection which causes
issues on a slow Request.Body.Close call.
Unlock before Request.Body.Close call. The abortStream closes the request
body after unlocking a mutex. The abortStreamLocked returns reqBody as
io.ReadCloser if the caller needs to close the body after unlocking the
mutex.
Fixesgolang/go#52853
Change-Id: I0b74ba5263f65393c0e69e1c645d10c4db048903
Reviewed-on: https://go-review.googlesource.com/c/net/+/424755
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Once a connection has received a GOAWAY from the server,
close it after the last outstanding request on the connection
completes.
We're lax about when we call ClientConn.closeConn, frequently
closing the underlying net.Conn multiple times. Stop propagating
errors on closing the net.Conn up through ClientConn.Close and
ClientConn.Shutdown, since these errors are likely to be caused
by double-closing the connection rather than a real fault.
Fixesgolang/go#39752.
Change-Id: I06d59e6daa6331c3091e1d49cdbeac313f17e6bd
Reviewed-on: https://go-review.googlesource.com/c/net/+/429060
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
This rolls-forward CL 150197 with an added fix for
TestProtocolErrorAfterGoAway.
Rather than send a WindowUpdate on every chunk of bytes read, allow them
to collect until we go past half the configured window size. Once the
threshold is reached, send a single WindowUpdate to reset the amount
back to the maximum amount configured.
Fixesgolang/go#28732
Change-Id: Icee93dedf68d6166aa6fe0c3845d717e66586e73
Reviewed-on: https://go-review.googlesource.com/c/net/+/432038
Run-TryBot: Damien Neil <dneil@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tatiana Bradley <tatiana@golang.org>
Fix a bug wherein undeclared trailers (set with the "Trailer:" prefix)
were not sent when the response body is neither written nor flushed.
We were testing responseWriterState.hasTrailers before promoting
undeclared trailers into rws.trailers, resulting in the response headers
being sent with an END_STREAM flag. Promote undeclared headers earlier
so that we leave the stream open for them to be sent.
For golang/go#54723
Change-Id: Ic036925f4a7ec775282b6e474aa72249d6418b23
Reviewed-on: https://go-review.googlesource.com/c/net/+/426874
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Rather than send a WindowUpdate on every chunk of bytes read, allow them
to collect until we go past half the configured window size. Once the
threshold is reached, send a single WindowUpdate to reset the amount
back to the maximum amount configured.
Fixesgolang/go#28732
Change-Id: I177f962ee0a9b8daa1c4817e0ab7698e828bad96
Reviewed-on: https://go-review.googlesource.com/c/net/+/150197
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
RFC 7231 permits HEAD requests to contain a body, although it does
state there are no defined semantics for payloads of HEAD requests
and that some servers may reject HEAD requests with a payload.
Accept HEAD requests with a body.
Test is in net/http CL 418614.
For golang/go#53960.
Change-Id: I946d3ec796054c3908beb8a69cc78aa51c04c972
Reviewed-on: https://go-review.googlesource.com/c/net/+/418634
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
HTTP2 server does not send WINDOW_UPDATE when the client
sends more data than declared in the content-length header.
Client flow control can eventually run out of available bytes which
hangs the client connection as it cannot write DATA frames for any
stream any longer.
Fixes: golang/go#54185
Change-Id: I48ae3212fb31ce302715abe129adf5c9625faf12
GitHub-Last-Rev: 1351d3b416
GitHub-Pull-Request: golang/net#143
Reviewed-on: https://go-review.googlesource.com/c/net/+/421974
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In dialCall.dial, close the done channel to mark dial completion after
adding the new connection to the clientConnPool.
Fixes a race condition in TestTransportBodyReadError, where the client
side of the test could observe the clientConnPool as unexpectedly
containing no conns, because the new conn had not yet been added to
the pool.
Fixesgolang/go#44304.
Change-Id: I121200f9aa664fae29d0532e7fa2da47de3fe6a8
Reviewed-on: https://go-review.googlesource.com/c/net/+/410934
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
If the test gets completely stuck at these points, we will want a
goroutine dump in order to debug the hang, which log.Fatalf will
not produce (but a test timeout will).
If the test does not get completely stuck (as on a slow or overloaded
builder), then we should let it continue to run until the overall test
timeout, which (unlike hard-coded constants) should already take the
speed of the builder into account.
As a side-effect, this also moves some t.Fatalf calls out of
background goroutines and into the main test-function goroutines where
they belong (see golang/go#24678).
Fixesgolang/go#52051.
Change-Id: I37504081e6fdf0b4c244305fc83c575e30b7b453
Reviewed-on: https://go-review.googlesource.com/c/net/+/410096
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
The http2 priority write scheduler should not queue RST_STREAM
frames with the DATA frames, and instead treat them as control frames.
There can be deadlock situations if data frames block the queue,
because if the sender wants to close the stream it sends an RST frame,
but if the client is not draining the queue, the RST frame is stuck
and the sender is not able to finish.
Fixesgolang/go#49741
Signed-off-by: Antonio Ojea <antonio.ojea.garcia@gmail.com>
Change-Id: Ie4462603380039f7aba8f701fe810d1d84376efa
Reviewed-on: https://go-review.googlesource.com/c/net/+/382014
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
The initial request on an h2c-upgraded connection is written as an
HTTP/1 request, with the response sent as an HTTP/2 stream.
The h2c package handled this request by constructing a sequence of
bytes representing an HTTP/2 stream containing the initial request,
prepending those bytes to the remainder of the connection, and
presenting that to the HTTP/2 server as if no upgrade had happened.
This translation did not handle request bodies. Handling request
bodies under this model would be difficult, since it would require
also translating the HTTP/2 flow control.
Rewrite the h2c upgrade to explicitly hand off the request to the
HTTP/2 server instead.
Fixesgolang/go#52882.
Change-Id: I26e0f12e2b1c8b48fd36ba47baea076424983553
Reviewed-on: https://go-review.googlesource.com/c/net/+/407454
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
This uses a uint64 bit buffer so can append 4 bytes at a time, instead
of using append every byte.
GOAMD64=v1
name old time/op new time/op delta
AppendHuffmanString-4 316ns ± 3% 169ns ± 1% -46.60% (p=0.000 n=10+8)
GOAMD64=v3
name old time/op new time/op delta
AppendHuffmanString-4 263ns ± 4% 114ns ± 1% -56.68% (p=0.000 n=10+8)
Change-Id: Ib48a0e0711589dd1b311c377f61e30d7c18c2953
GitHub-Last-Rev: 0a3eb27fee
GitHub-Pull-Request: golang/net#131
Reviewed-on: https://go-review.googlesource.com/c/net/+/403434
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently, it's not possible to send informational responses such as
103 Early Hints or 102 Processing.
This patch allows calling WriteHeader() multiple times in order
to send informational responses before the final one.
If the status code is in the 1xx range, the current content of the header map
is also sent. Its content is not removed after the call to WriteHeader()
because the headers must also be included in the final response.
The Chrome and Fastly teams are starting a large-scale experiment to measure
the real-life impact of the 103 status code.
Using Early Hints is proposed as a (partial) alternative to Server Push,
which are going to be removed from Chrome:
https://groups.google.com/a/chromium.org/g/blink-dev/c/K3rYLvmQUBY/m/21anpFhxAQAJ
Being able to send this status code from servers implemented using Go would
help to see if implementing it in browsers is worth it.
Fixesgolang/go#26089.
Fixesgolang/go#36734.
Updates golang/go#26088.
Change-Id: Iadb7be92844a3588ef4ce044f887ef5c1792f431
GitHub-Last-Rev: eee95b58b3
GitHub-Pull-Request: golang/net#96
Reviewed-on: https://go-review.googlesource.com/c/net/+/291029
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
