net/http: fix hang in probing for a zero-length request body

Fix a hang that occurs when making a request and all of the following apply:
* The request method is one of GET, HEAD, DELETE, OPTIONS, PROPFIND, or SEARCH.
* The Request.Body is non-nil.
* The content length is not set, or is set to -1.
* Transfer-Encoding: chunked is not set.
* The request body does not respond to a read within 200ms.

In this case, we give up on probing for a zero-length body and send the
request while the probe completes in the background. Fix a bug in the
io.Reader wrapping the in-flight probe: It should return io.EOF after
the probe completes, but does not.

Fixes #47568.

Change-Id: I7f9188c96e1210055df68424081af927006e4816
Reviewed-on: https://go-review.googlesource.com/c/go/+/340256
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index 01d605c35194f88e954c2b348458f5e7aae9c478..05ed2268b59ff24fe95b5c40c23724628c794ddd 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -2082,3 +2082,47 @@ func (b *issue40382Body) Close() error {
        }
        return nil
 }
+
+func TestProbeZeroLengthBody(t *testing.T) {
+       setParallel(t)
+       defer afterTest(t)
+       reqc := make(chan struct{})
+       cst := newClientServerTest(t, false, HandlerFunc(func(w ResponseWriter, r *Request) {
+               close(reqc)
+               if _, err := io.Copy(w, r.Body); err != nil {
+                       t.Errorf("error copying request body: %v", err)
+               }
+       }))
+       defer cst.close()
+
+       bodyr, bodyw := io.Pipe()
+       var gotBody string
+       var wg sync.WaitGroup
+       wg.Add(1)
+       go func() {
+               defer wg.Done()
+               req, _ := NewRequest("GET", cst.ts.URL, bodyr)
+               res, err := cst.c.Do(req)
+               b, err := io.ReadAll(res.Body)
+               if err != nil {
+                       t.Error(err)
+               }
+               gotBody = string(b)
+       }()
+
+       select {
+       case <-reqc:
+               // Request should be sent after trying to probe the request body for 200ms.
+       case <-time.After(60 * time.Second):
+               t.Errorf("request not sent after 60s")
+       }
+
+       // Write the request body and wait for the request to complete.
+       const content = "body"
+       bodyw.Write([]byte(content))
+       bodyw.Close()
+       wg.Wait()
+       if gotBody != content {
+               t.Fatalf("server got body %q, want %q", gotBody, content)
+       }
+}

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index 85c2e5a360d68676f2f6c4574b0e5833fc68ca24..5ff89cc17f10cbd83a0dfcd8da9455f0ef54ae68 100644 (file)
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -212,6 +212,7 @@ func (t *transferWriter) probeRequestBody() {
                        rres.b = buf[0]
                }
                t.ByteReadCh <- rres
+               close(t.ByteReadCh)
        }(t.Body)
        timer := time.NewTimer(200 * time.Millisecond)
        select {
@@ -1072,6 +1073,9 @@ func (fr finishAsyncByteRead) Read(p []byte) (n int, err error) {
        if n == 1 {
                p[0] = rres.b
        }
+       if err == nil {
+               err = io.EOF
+       }
        return
 }