Skip to content

N
net
Switch branch/tag
  1. 07 Dec, 2017 2 commits
  3. 29 Nov, 2017 1 commit
  5. 28 Nov, 2017 5 commits
    • Tom Bergan's avatar
      http2: fix flake in net/http's TestCloseIdleConnections_h2 · 894f8ed5
      Tom Bergan authored 
      That test makes a request with no body and receives a response with no
body. The client will receive a HEADERS frame with END_STREAM. The test
assumes that the stream is closed immediately on receipt of that HEADERS
frame, i.e., before RoundTrip returns.

This assumption was broken by https://golang.org/cl/70510, which made
stream closure asynchronous w.r.t. RoundTrip.

To fix TestCloseIdleConnections_h2 while preserving the intent of CL
70510, we break processHeaders into two cases:

1. The request has a body. In this case, END_STREAM puts the stream in a
   half-closed-remote state, which means the connection is not
   necessarily idle when RoundTrip returns (since the request body is
   still being uploaded). In this case, we preserve the behavior from CL
   70510.

2. The request does not have a body. In this case, END_STREAM puts the
   stream in a closed state and we must close the stream before
   returning from RoundTrip.

The following command passes when this CL is merged into net/http:
go test -count=100000 -run=TestCloseIdleConnections_h2 net/http

Updates golang/go#22413

Change-Id: Iff2a0685a636ad51bff380e86a42b0d0eea984e5
Reviewed-on: https://go-review.googlesource.com/80139
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      894f8ed5
    • Gregory Man's avatar
      http2: discard logger output in invalid response tests · 6921abc3
      Gregory Man authored 
      In invalid response tests logger write error messages to stderr and spam
test output.

Since we know response are invalid in these tests we can safely discard
logger output.

Fixes golang/go#22850

Change-Id: Id8c97be910f0cf7dbe2380ba632960364bc8478b
Reviewed-on: https://go-review.googlesource.com/80235Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
      6921abc3
    • Brad Fitzpatrick's avatar
      http/httpproxy: fix build on Go 1.8 · fc492d2e
      Brad Fitzpatrick authored 
      Go 1.8 doesn't have t.Helper.

Fix the row of red on the dashboard.

Change-Id: I85d4bb9fe38e989dc3b6a4e99705599745b83cef
Reviewed-on: https://go-review.googlesource.com/80140
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 's avatarTom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
      fc492d2e
    • Tom Bergan's avatar
      http2: fix leak in activeRes by removing activeRes · e6a4aa30
      Tom Bergan authored 
      AFAICT, activeRes serves no real purpose. It is used in just two ways:

- To reduce the number of calls to closeIfIdle, which reduces the number
  of acquires of cc.mu when there are many concurrent streams. I dug
  through the CL history and could not find any benchmarks showing that
  this is necessary.

- To avoid redundant calls to cs.bufPipe.CloseWithError(err) when a read
  loop is shutdown. This is unnecessary, since redundant CloseWithError
  calls are ignored.

Since there isn't a good reason to have activeRes, the simplest way to
fix the leak is to remove activeRes entirely.

Updates golang/go#21543

Change-Id: I1d1d2dc6c946425a2772c8bf71436707021ac269
Reviewed-on: https://go-review.googlesource.com/80137Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
      e6a4aa30
    • Tom Bergan's avatar
      http2: don't autodetect Content-Type when the response has an empty body · db473f6b
      Tom Bergan authored 
      This change was originally made in https://golang.org/cl/46631, however,
that change was applited to net/http/h2_bundle.go instead of x/net/http2.

Updates golang/go#20784

Change-Id: I947fa4c19f3efc400856573768140bece28276a2
Reviewed-on: https://go-review.googlesource.com/80135
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      db473f6b
  7. 27 Nov, 2017 3 commits
  9. 23 Nov, 2017 1 commit
  11. 22 Nov, 2017 1 commit
  13. 15 Nov, 2017 1 commit
  15. 07 Nov, 2017 1 commit
    • Anmol Sethi's avatar
      http2: require either ECDSA or RSA ciphersuite · a337091b
      Anmol Sethi authored 
      The HTTP/2 RFC does indeed mandate TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
but in practice, people are also using TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
becuase they are only using an ECDSA certificate. This is the case in acme/autocert.

It doesn't make sense to enforce only RSA in cipher suites if it will
never be used because they are using a ECDSA certificate.

Change-Id: I86dac192a3eb9b74e4268310a3b550b3bd88a37f
Reviewed-on: https://go-review.googlesource.com/30721Reviewed-by: 's avatarTom Bergan <tombergan@google.com>
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
      a337091b
  17. 02 Nov, 2017 2 commits
  19. 01 Nov, 2017 1 commit
  21. 27 Oct, 2017 1 commit
  23. 26 Oct, 2017 1 commit
  25. 24 Oct, 2017 1 commit
  27. 23 Oct, 2017 2 commits
  29. 20 Oct, 2017 1 commit
    • Tom Bergan's avatar
      http2: always delay closing the connection after sending GOAWAY · cd69bc3f
      Tom Bergan authored 
      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: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 's avatarJoe Tsai <thebrokentoaster@gmail.com>
      cd69bc3f
  31. 19 Oct, 2017 1 commit
  33. 16 Oct, 2017 1 commit
    • Anand K. Mistry's avatar
      net/http2: reset client stream after processing response headers · 1087133b
      Anand K. Mistry authored 
      When a client receives a HEADER frame with a END_STREAM flag,
clientConn.streamByID closes the stream before processing the headers
which may contain a full non-error response. This causes the request's
bodyWriter cancelation to race with the response.

Closing the stream after processing headers allows the response to be
available before the bodyWriter is canceled.

Updates golang/go#20521

Change-Id: I70740e88f75240836e922163a54a6cd89535f7b3
Reviewed-on: https://go-review.googlesource.com/70510
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: 's avatarTom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
      1087133b
  35. 04 Oct, 2017 2 commits
  37. 27 Sep, 2017 1 commit
  39. 26 Sep, 2017 1 commit
  41. 22 Sep, 2017 1 commit
  43. 20 Sep, 2017 3 commits
  45. 15 Sep, 2017 1 commit
  47. 14 Sep, 2017 1 commit
  49. 12 Sep, 2017 1 commit
  51. 28 Aug, 2017 1 commit
    • Mike Appleby's avatar
      http2: Respect peer's SETTINGS_MAX_HEADER_LIST_SIZE in ClientConn · 66aacef3
      Mike Appleby authored 
      Add a new peerMaxHeaderListSize member to ClientConn which records the
SETTINGS_MAX_HEADER_LIST_SIZE requested by the client's peer, and
respect this limit in (*ClientConn) encodeHeaders / encodeTrailers.

Attempting to send more than peerMaxHeaderListSize bytes of headers or
trailers will result in RoundTrip returning errRequestHeaderListSize.

Updates golang/go#13959

Change-Id: Ic707179782acdf8ae543429ea1af7f4f30e67e59
Reviewed-on: https://go-review.googlesource.com/29243
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarTom Bergan <tombergan@google.com>
      66aacef3
  53. 24 Aug, 2017 1 commit
  55. 09 Aug, 2017 1 commit
    • Tom Bergan's avatar
      http2: block RoundTrip when the Transport hits MaxConcurrentStreams · 1c05540f
      Tom Bergan authored 
      Currently if the http2.Transport hits SettingsMaxConcurrentStreams for a
server, it just makes a new TCP connection and creates the stream on the
new connection. This CL updates that behavior to instead block RoundTrip
until a new stream is available.

I also fixed a second bug, which was necessary to make some tests pass:
Previously, a stream was removed from cc.streams only if either (a) we
received END_STREAM from the server, or (b) we received RST_STREAM from
the server. This CL removes a stream from cc.streams if the request was
cancelled (via ctx.Close, req.Cancel, or resp.Body.Close) before
receiving END_STREAM or RST_STREAM from the server.

Updates golang/go#13774
Updates golang/go#20985
Updates golang/go#21229

Change-Id: I660ffd724c4c513e0f1cc587b404bedb8aff80be
Reviewed-on: https://go-review.googlesource.com/53250
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      1c05540f