github/golang.net
2
0
Fork 0
mirror of https://github.com/golang/net.git synced 2026-03-31 18:37:08 +09:00
Code Issues Packages Projects Releases Wiki Activity

http2: avoid racy access to clientStream.requestedGzip

Browse Source 
clientStream.requestedGzip is set from clientStream.writeRequest,
and examined by clientConn.readLoop. I'm not sure if there's
any possible way for an actual data race to happen here in
practice, since the read loop should only examine the field
after the request is sent by writeRequest, but it's enough
for the race detector to complain.

Set the field in ClientConn.roundTrip instead, before
the clientStream has become accessible to any other goroutines.

No test, but a following CL has race detector failures without
this change.

Change-Id: Id30f1b95bcfcc35c213440e0e47cce3feaaff06d
Reviewed-on: https://go-review.googlesource.com/c/net/+/586245
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
This commit is contained in:
Damien Neil
2024-05-17 14:29:29 -07:00
parent 332fe235e6
commit 410d19ee56
2 changed files with 21 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
http2/transport.go
View File

@@ -1301,6 +1301,26 @@ func (cc *ClientConn) roundTrip(req *http.Request, streamf func(*clientStream))
donec: make(chan struct{}),
}
// TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere?
if !cc.t.disableCompression() &&
req.Header.Get("Accept-Encoding") == "" &&
req.Header.Get("Range") == "" &&
!cs.isHead {
// Request gzip only, not deflate. Deflate is ambiguous and
// not as universally supported anyway.
// See: https://zlib.net/zlib_faq.html#faq39
//
// Note that we don't request this for HEAD requests,
// due to a bug in nginx:
// http://trac.nginx.org/nginx/ticket/358
// https://golang.org/issue/5522
//
// We don't request gzip if the request is for a range, since
// auto-decoding a portion of a gzipped document will just fail
// anyway. See https://golang.org/issue/8923
cs.requestedGzip = true
}
go cs.doRequest(req, streamf)
waitDone := func() error {
@@ -1449,26 +1469,6 @@ func (cs *clientStream) writeRequest(req *http.Request, streamf func(*clientStre
streamf(cs)
}
// TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere?
if !cc.t.disableCompression() &&
req.Header.Get("Accept-Encoding") == "" &&
req.Header.Get("Range") == "" &&
!cs.isHead {
// Request gzip only, not deflate. Deflate is ambiguous and
// not as universally supported anyway.
// See: https://zlib.net/zlib_faq.html#faq39
//
// Note that we don't request this for HEAD requests,
// due to a bug in nginx:
// http://trac.nginx.org/nginx/ticket/358
// https://golang.org/issue/5522
//
// We don't request gzip if the request is for a range, since
// auto-decoding a portion of a gzipped document will just fail
// anyway. See https://golang.org/issue/8923
cs.requestedGzip = true
}
continueTimeout := cc.t.expectContinueTimeout()
if continueTimeout != 0 {
if !httpguts.HeaderValuesContainsToken(req.Header["Expect"], "100-continue") {

1
http2/transport_test.go
View File

@@ -2895,6 +2895,7 @@ func TestRoundTripDoesntConsumeRequestBodyEarly(t *testing.T) {
cc := &ClientConn{
closed: true,
reqHeaderMu: make(chan struct{}, 1),
t: &Transport{},
}
_, err := cc.RoundTrip(req)
if err != errClientConnUnusable {