Skip to content

Commit

Permalink
[internal-branch.go1.17-vendor] net/http2: Fix handling of expect con…
Browse files Browse the repository at this point in the history
…tinue

Recent refactoring in the http2 package broke handling of ExpectContinueTimeout, where the client was always waiting for the timeout to pass before sending the body.

For golang/go#49677
Fixes golang/go#49905

Change-Id: I565299e48313bb905b2655bd52084ea49ab742f3
GitHub-Last-Rev: 587b484
GitHub-Pull-Request: #118
Reviewed-on: https://go-review.googlesource.com/c/net/+/363914
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
Trust: Damien Neil <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
(cherry picked from commit 0a0e4e1)
Reviewed-on: https://go-review.googlesource.com/c/net/+/368475
Reviewed-by: Michael Knyszek <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
TryBot-Result: Go Bot <[email protected]>
  • Loading branch information
mhr3 authored and mknyszek committed Dec 1, 2021
1 parent 1dc0aec commit 85e122b
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 6 deletions.
12 changes: 6 additions & 6 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -1161,12 +1161,12 @@ func (cs *clientStream) writeRequest(req *http.Request) (err error) {
}

continueTimeout := cc.t.expectContinueTimeout()
if continueTimeout != 0 &&
!httpguts.HeaderValuesContainsToken(
req.Header["Expect"],
"100-continue") {
continueTimeout = 0
cs.on100 = make(chan struct{}, 1)
if continueTimeout != 0 {
if !httpguts.HeaderValuesContainsToken(req.Header["Expect"], "100-continue") {
continueTimeout = 0
} else {
cs.on100 = make(chan struct{}, 1)
}
}

// Past this point (where we send request headers), it is possible for
Expand Down
90 changes: 90 additions & 0 deletions http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5109,6 +5109,96 @@ func TestTransportServerResetStreamAtHeaders(t *testing.T) {
res.Body.Close()
}

type trackingReader struct {
rdr io.Reader
wasRead uint32
}

func (tr *trackingReader) Read(p []byte) (int, error) {
atomic.StoreUint32(&tr.wasRead, 1)
return tr.rdr.Read(p)
}

func (tr *trackingReader) WasRead() bool {
return atomic.LoadUint32(&tr.wasRead) != 0
}

func TestTransportExpectContinue(t *testing.T) {
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/reject":
w.WriteHeader(403)
default:
io.Copy(io.Discard, r.Body)
}
}, optOnlyServer)
defer st.Close()

tr := &http.Transport{
TLSClientConfig: tlsConfigInsecure,
MaxConnsPerHost: 1,
ExpectContinueTimeout: 10 * time.Second,
}

err := ConfigureTransport(tr)
if err != nil {
t.Fatal(err)
}
client := &http.Client{
Transport: tr,
}

testCases := []struct {
Name string
Path string
Body *trackingReader
ExpectedCode int
ShouldRead bool
}{
{
Name: "read-all",
Path: "/",
Body: &trackingReader{rdr: strings.NewReader("hello")},
ExpectedCode: 200,
ShouldRead: true,
},
{
Name: "reject",
Path: "/reject",
Body: &trackingReader{rdr: strings.NewReader("hello")},
ExpectedCode: 403,
ShouldRead: false,
},
}

for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
startTime := time.Now()

req, err := http.NewRequest("POST", st.ts.URL+tc.Path, tc.Body)
if err != nil {
t.Fatal(err)
}
req.Header.Set("Expect", "100-continue")
res, err := client.Do(req)
if err != nil {
t.Fatal(err)
}
res.Body.Close()

if delta := time.Since(startTime); delta >= tr.ExpectContinueTimeout {
t.Error("Request didn't finish before expect continue timeout")
}
if res.StatusCode != tc.ExpectedCode {
t.Errorf("Unexpected status code, got %d, expected %d", res.StatusCode, tc.ExpectedCode)
}
if tc.Body.WasRead() != tc.ShouldRead {
t.Errorf("Unexpected read status, got %v, expected %v", tc.Body.WasRead(), tc.ShouldRead)
}
})
}
}

type closeChecker struct {
io.ReadCloser
closed chan struct{}
Expand Down

0 comments on commit 85e122b

Please sign in to comment.