Skip to content

Commit 20ed279

Browse files
neilddmitshur
authored andcommitted
[internal-branch.go1.16-vendor] http2: avoid race in TestTransportReqBodyAfterResponse_403.
This test sends a request with a 10MiB body, reads a 403 response while the body is still being written, and closes the response body. It then verifies that the full request body was not written, since reading a response code >=300 interrupts body writes. This can be racy: We process the status code and interrupt the body write in RoundTrip, but it is possible for the body write to complete before RoundTrip looks at the response. Adjust the test to have more control over the request body: Only provide half the Request.Body until after the response headers have been received. Updates golang/go#49076 Change-Id: Id4802b04a50f34f6af28f4eb93e37ef70a33a068 Reviewed-on: https://go-review.googlesource.com/c/net/+/354130 Trust: Damien Neil <[email protected]> Run-TryBot: Damien Neil <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/net/+/357091 Reviewed-by: Dmitri Shuralyov <[email protected]> TryBot-Result: Go Bot <[email protected]>
1 parent d585ef0 commit 20ed279

File tree

1 file changed

+21
-5
lines changed

1 file changed

+21
-5
lines changed

http2/transport_test.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ func testTransportReqBodyAfterResponse(t *testing.T, status int) {
889889
const bodySize = 10 << 20
890890
clientDone := make(chan struct{})
891891
ct := newClientTester(t)
892+
recvLen := make(chan int64, 1)
892893
ct.client = func() error {
893894
defer ct.cc.(*net.TCPConn).CloseWrite()
894895
if runtime.GOOS == "plan9" {
@@ -897,32 +898,35 @@ func testTransportReqBodyAfterResponse(t *testing.T, status int) {
897898
}
898899
defer close(clientDone)
899900

900-
var n int64 // atomic
901-
req, err := http.NewRequest("PUT", "https://dummy.tld/", io.LimitReader(countingReader{&n}, bodySize))
901+
body := &pipe{b: new(bytes.Buffer)}
902+
io.Copy(body, io.LimitReader(neverEnding('A'), bodySize/2))
903+
req, err := http.NewRequest("PUT", "https://dummy.tld/", body)
902904
if err != nil {
903905
return err
904906
}
905907
res, err := ct.tr.RoundTrip(req)
906908
if err != nil {
907909
return fmt.Errorf("RoundTrip: %v", err)
908910
}
909-
defer res.Body.Close()
910911
if res.StatusCode != status {
911912
return fmt.Errorf("status code = %v; want %v", res.StatusCode, status)
912913
}
914+
io.Copy(body, io.LimitReader(neverEnding('A'), bodySize/2))
915+
body.CloseWithError(io.EOF)
913916
slurp, err := ioutil.ReadAll(res.Body)
914917
if err != nil {
915918
return fmt.Errorf("Slurp: %v", err)
916919
}
917920
if len(slurp) > 0 {
918921
return fmt.Errorf("unexpected body: %q", slurp)
919922
}
923+
res.Body.Close()
920924
if status == 200 {
921-
if got := atomic.LoadInt64(&n); got != bodySize {
925+
if got := <-recvLen; got != bodySize {
922926
return fmt.Errorf("For 200 response, Transport wrote %d bytes; want %d", got, bodySize)
923927
}
924928
} else {
925-
if got := atomic.LoadInt64(&n); got == 0 || got >= bodySize {
929+
if got := <-recvLen; got == 0 || got >= bodySize {
926930
return fmt.Errorf("For %d response, Transport wrote %d bytes; want (0,%d) exclusive", status, got, bodySize)
927931
}
928932
}
@@ -948,6 +952,7 @@ func testTransportReqBodyAfterResponse(t *testing.T, status int) {
948952
}
949953
}
950954
//println(fmt.Sprintf("server got frame: %v", f))
955+
ended := false
951956
switch f := f.(type) {
952957
case *WindowUpdateFrame, *SettingsFrame:
953958
case *HeadersFrame:
@@ -985,13 +990,24 @@ func testTransportReqBodyAfterResponse(t *testing.T, status int) {
985990
return err
986991
}
987992
}
993+
994+
if f.StreamEnded() {
995+
ended = true
996+
}
988997
case *RSTStreamFrame:
989998
if status == 200 {
990999
return fmt.Errorf("Unexpected client frame %v", f)
9911000
}
1001+
ended = true
9921002
default:
9931003
return fmt.Errorf("Unexpected client frame %v", f)
9941004
}
1005+
if ended {
1006+
select {
1007+
case recvLen <- dataRecv:
1008+
default:
1009+
}
1010+
}
9951011
}
9961012
}
9971013
ct.run()

0 commit comments

Comments
 (0)