From 08dda57501ce6ef3fd63b9a912de135c050e8b0d Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 7 Mar 2023 11:27:33 -0800 Subject: [PATCH 1/4] html: fix package doc typo Change-Id: Ic16f297e936cf10bafe0656f5db68cd422c430aa Reviewed-on: https://go-review.googlesource.com/c/net/+/474217 Reviewed-by: Ian Lance Taylor Auto-Submit: Roland Shoemaker Run-TryBot: Roland Shoemaker TryBot-Result: Gopher Robot --- html/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/html/doc.go b/html/doc.go index 7a96eae331..5ff8480cf5 100644 --- a/html/doc.go +++ b/html/doc.go @@ -106,7 +106,7 @@ and as such does not resolve issues that may exist in the processed HTML, producing a literal interpretation of the input. If your use case requires semantically well-formed HTML, as defined by the -WHATWG specifiction, the parser should be used rather than the tokenizer. +WHATWG specification, the parser should be used rather than the tokenizer. */ package html // import "golang.org/x/net/html" From 9f24bb44e6dfa4fadbda1cd143a46d288ba89ae5 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Thu, 9 Mar 2023 12:33:56 -0800 Subject: [PATCH 2/4] http2: properly discard data received after request/response body is closed A server handler can close an inbound Request.Body to indicate that it is not interested in the remainder of the request body. Equivalently, a client can close a Response.Body indicate that it is not interesed in the remainder of the response body. In both cases, if we receive DATA frames from the peer for the stream, we should return connection-level flow control credit for the discarded data. We do not return stream-level flow control, since we don't want to unblock further sends of data that we're just going to discard. Closing either a Response.Body or an inbound Request.Body results in a pipe.BreakWithError. Reads from a broken pipe fail immediately. Previously, writes to a broken pipe would succeed, discarding the written data and incrementing the pipe's unread count. Silently discarding data written to a broken pipe results in both the Transport and Server failing to detect the condition where data has been discarded. Change pipes to return an error when writing to a broken pipe. Change transportResponseBody.Close to break the response body before returning flow control credit for unread data in the pipe, avoiding a race condition where data is added to the pipe in between the return of flow control credit and the pipe breaking. Change the Server to treat an error writing to the inbound request body as an expected condition (since this only happens when a handler closes the request body), returning connection-level flow control credit for the discarded data. Fixes golang/go#57578 Change-Id: I1ed4ea9865818f9c7d7eb4500edfd7556e3cbcbf Reviewed-on: https://go-review.googlesource.com/c/net/+/475135 Run-TryBot: Damien Neil TryBot-Result: Gopher Robot Reviewed-by: Roland Shoemaker --- http2/pipe.go | 6 +----- http2/pipe_test.go | 8 ++++---- http2/server.go | 7 +++++-- http2/server_test.go | 26 ++++++++++++++++++++++++++ http2/transport.go | 6 +++--- 5 files changed, 39 insertions(+), 14 deletions(-) diff --git a/http2/pipe.go b/http2/pipe.go index c15b8a7719..684d984fd9 100644 --- a/http2/pipe.go +++ b/http2/pipe.go @@ -88,13 +88,9 @@ func (p *pipe) Write(d []byte) (n int, err error) { p.c.L = &p.mu } defer p.c.Signal() - if p.err != nil { + if p.err != nil || p.breakErr != nil { return 0, errClosedPipeWrite } - if p.breakErr != nil { - p.unread += len(d) - return len(d), nil // discard when there is no reader - } return p.b.Write(d) } diff --git a/http2/pipe_test.go b/http2/pipe_test.go index 83d2dfd27e..67562a92a1 100644 --- a/http2/pipe_test.go +++ b/http2/pipe_test.go @@ -125,14 +125,14 @@ func TestPipeBreakWithError(t *testing.T) { if p.Len() != 3 { t.Errorf("pipe should have 3 unread bytes") } - // Write should succeed silently. - if n, err := p.Write([]byte("abc")); err != nil || n != 3 { - t.Errorf("Write(abc) after break\ngot %v, %v\nwant 0, nil", n, err) + // Write should fail. + if n, err := p.Write([]byte("abc")); err != errClosedPipeWrite || n != 0 { + t.Errorf("Write(abc) after break\ngot %v, %v\nwant 0, errClosedPipeWrite", n, err) } if p.b != nil { t.Errorf("buffer should be nil after Write") } - if p.Len() != 6 { + if p.Len() != 3 { t.Errorf("pipe should have 6 unread bytes") } // Read should fail. diff --git a/http2/server.go b/http2/server.go index 8cb14f3c97..cd057f3982 100644 --- a/http2/server.go +++ b/http2/server.go @@ -1822,15 +1822,18 @@ func (sc *serverConn) processData(f *DataFrame) error { } if len(data) > 0 { + st.bodyBytes += int64(len(data)) wrote, err := st.body.Write(data) if err != nil { + // The handler has closed the request body. + // Return the connection-level flow control for the discarded data, + // but not the stream-level flow control. sc.sendWindowUpdate(nil, int(f.Length)-wrote) - return sc.countError("body_write_err", streamError(id, ErrCodeStreamClosed)) + return nil } if wrote != len(data) { panic("internal error: bad Writer") } - st.bodyBytes += int64(len(data)) } // Return any padded flow control now, since we won't diff --git a/http2/server_test.go b/http2/server_test.go index d32b2d85bd..40ab750fca 100644 --- a/http2/server_test.go +++ b/http2/server_test.go @@ -3906,6 +3906,32 @@ func TestUnreadFlowControlReturned_Server(t *testing.T) { } } +func TestServerReturnsStreamAndConnFlowControlOnBodyClose(t *testing.T) { + unblockHandler := make(chan struct{}) + defer close(unblockHandler) + + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + r.Body.Close() + w.WriteHeader(200) + w.(http.Flusher).Flush() + <-unblockHandler + }) + defer st.Close() + + st.greet() + st.writeHeaders(HeadersFrameParam{ + StreamID: 1, + BlockFragment: st.encodeHeader(), + EndHeaders: true, + }) + st.wantHeaders() + const size = inflowMinRefresh // enough to trigger flow control return + st.writeData(1, false, make([]byte, size)) + st.wantWindowUpdate(0, size) // conn-level flow control is returned + unblockHandler <- struct{}{} + st.wantData() +} + func TestServerIdleTimeout(t *testing.T) { if testing.Short() { t.Skip("skipping in short mode") diff --git a/http2/transport.go b/http2/transport.go index 05ba23d3d9..c9e1115a5d 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -2555,6 +2555,9 @@ func (b transportResponseBody) Close() error { cs := b.cs cc := cs.cc + cs.bufPipe.BreakWithError(errClosedResponseBody) + cs.abortStream(errClosedResponseBody) + unread := cs.bufPipe.Len() if unread > 0 { cc.mu.Lock() @@ -2573,9 +2576,6 @@ func (b transportResponseBody) Close() error { cc.wmu.Unlock() } - cs.bufPipe.BreakWithError(errClosedResponseBody) - cs.abortStream(errClosedResponseBody) - select { case <-cs.donec: case <-cs.ctx.Done(): From 6960703597adf5b8919a13c3c0ce585a274fd405 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 20 Mar 2023 16:10:06 -0400 Subject: [PATCH 3/4] http2: log the correct error when retrying in (*Transport).RoundTripOpt On the shouldRetryRequest path, err is invariantly nil, and therefore meaningless to log with vlogf. Instead, log the original error returned by the call to cc.RoundTrip. For golang/go#59155. Change-Id: I82c00a6033d0e92c28a5ccf60a87eec1c8b41886 Reviewed-on: https://go-review.googlesource.com/c/net/+/477876 TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills Reviewed-by: Damien Neil --- http2/transport.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/http2/transport.go b/http2/transport.go index c9e1115a5d..f965579f7d 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -560,10 +560,11 @@ func (t *Transport) RoundTripOpt(req *http.Request, opt RoundTripOpt) (*http.Res traceGotConn(req, cc, reused) res, err := cc.RoundTrip(req) if err != nil && retry <= 6 { + roundTripErr := err if req, err = shouldRetryRequest(req, err); err == nil { // After the first retry, do exponential backoff with 10% jitter. if retry == 0 { - t.vlogf("RoundTrip retrying after failure: %v", err) + t.vlogf("RoundTrip retrying after failure: %v", roundTripErr) continue } backoff := float64(uint(1) << (uint(retry) - 1)) @@ -572,7 +573,7 @@ func (t *Transport) RoundTripOpt(req *http.Request, opt RoundTripOpt) (*http.Res timer := backoffNewTimer(d) select { case <-timer.C: - t.vlogf("RoundTrip retrying after failure: %v", err) + t.vlogf("RoundTrip retrying after failure: %v", roundTripErr) continue case <-req.Context().Done(): timer.Stop() From 694cff8668bac64e0864b552bffc280cd27f21b1 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Thu, 6 Apr 2023 09:44:16 +0000 Subject: [PATCH 4/4] go.mod: update golang.org/x dependencies Update golang.org/x dependencies to their latest tagged versions. Once this CL is submitted, and post-submit testing succeeds on all first-class ports across all supported Go versions, this repository will be tagged with its next minor version. Change-Id: I2fd2e05ca8edb122059be1918e555952de4941e8 Reviewed-on: https://go-review.googlesource.com/c/net/+/482776 Reviewed-by: Dmitri Shuralyov Run-TryBot: Gopher Robot Reviewed-by: Dmitri Shuralyov Reviewed-by: Carlos Amedee Auto-Submit: Gopher Robot TryBot-Result: Gopher Robot --- go.mod | 6 +++--- go.sum | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 553fef1486..d906d99812 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module golang.org/x/net go 1.17 require ( - golang.org/x/sys v0.6.0 - golang.org/x/term v0.6.0 - golang.org/x/text v0.8.0 + golang.org/x/sys v0.7.0 + golang.org/x/term v0.7.0 + golang.org/x/text v0.9.0 ) diff --git a/go.sum b/go.sum index 5847b9f424..4179727853 100644 --- a/go.sum +++ b/go.sum @@ -16,19 +16,19 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= +golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= -golang.org/x/term v0.6.0 h1:clScbb1cHjoCkyRbWwBEUZ5H/tIFu5TAXIqaZD0Gcjw= -golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= +golang.org/x/term v0.7.0 h1:BEvjmm5fURWqcfbSKTdpkDXYBrUS1c0m8agp14W48vQ= +golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/text v0.8.0 h1:57P1ETyNKtuIjB4SRd15iJxuhj8Gc416Y78H3qgMh68= -golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE= +golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=