x/net/http2: Transport hangs forever if using a pipe that has not been written to yet as req.Body #17480

luna-duclos · 2016-10-17T14:37:04Z

### What version of Go are you using (go version)?
Go 1.7.1

### What operating system and processor architecture are you using (go env)?

set GOARCH=amd64                                                                                                                                                
set GOBIN=                                                                                                                                                      
set GOEXE=.exe                                                                                                                                                  
set GOHOSTARCH=amd64                                                                                                                                            
set GOHOSTOS=windows                                                                                                                                            
set GOOS=windows                                                                                                                                                
set GOPATH=D:\src\psg\server;D:\src\psg\server\vendor                                                                                                           
set GORACE=                                                                                                                                                     
set GOROOT=C:\apps\Go                                                                                                                                           
set GOTOOLDIR=C:\apps\Go\pkg\tool\windows_amd64                                                                                                                 
set CC=gcc                                                                                                                                                      
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\lunad\AppData\Local\Temp\go-build906126950=/tmp/go-build -gno-record-gcc-switches  
set CXX=g++                                                                                                                                                     
set CGO_ENABLED=1

### What did you do?
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

I ran the following program:

package main

import (
    "crypto/rand"
    "io"
    "log"
    "net/http"
    "runtime/debug"
    "time"

    "golang.org/x/net/http2"
)

func main() {
    // set a 10 second timer for success
    go func() {
        timer := time.NewTimer(10 * time.Second)
        <-timer.C
        debug.SetTraceback("all")
        panic("10 second test period expired")
    }()

    transport := &http2.Transport{}
    c := &http.Client{Transport: transport}

    // Get a set of pipes, we'll use these to send data to the server
    pipeReader, pipeWriter := io.Pipe()

    // Initialize the request
    req, err := http.NewRequest("PUT", "https://http2.golang.org/bidirectional-stream", pipeReader)
    if err != nil {
        pipeWriter.Close()
        log.Fatalf("Error while initializing http request: %v", err)
    }

    // Set the body of the request to the reader end of our pipe, and initiate the request.
    req.Body = pipeReader
    resp, err := c.Do(req)
    if err != nil {
        pipeWriter.Close()
        log.Fatalf("Error during http call: %v", err)
    }

    // Check that we actually got a 200 OK, if not: error out
    if resp.StatusCode != http.StatusOK {
        resp.Body.Close()
        pipeWriter.Close()
        log.Fatalf("http call got non-200 status code %d", resp.StatusCode)
    }

    // Use a 10 byte buffer
    buffer := make([]byte, 10)

    // Read some data
    dataLen, err := resp.Body.Read(buffer)
    if err != nil {
        log.Fatalf("Error while reading data: %v", err)
    }

    log.Printf("Got data: %+v", buffer[:dataLen])

    // Reply
    if _, err := rand.Read(buffer); err != nil {
        log.Fatalf("Error while reading random data: %v", err)
    }

    if _, err := pipeWriter.Write(buffer); err != nil {
        log.Fatalf("Error while writing data: %v", err)
    }
}

### What did you expect to see?
I expected to see a bidirectional http-2 stream, with the server talking first, and the client then replying to the server.

### What did you see instead?
The program hangs within http2/transport.go, in the bodyAndLength function, due to that function doing a Read(), which ofcourse hangs.

The text was updated successfully, but these errors were encountered:

bradfitz · 2016-10-17T14:44:21Z

Workaround: set your Request.ContentLength to -1 after http.NewRequest, before Client.Do.

luna-duclos · 2016-10-17T14:52:31Z

The workaround works :) Thank you

bradfitz · 2016-10-17T15:28:06Z

The larger issue is to remind myself why we need to distinguish between explicitly-zero and unset-so-zero in Request.ContentLength. Presumably there was a reason. Or maybe there was only a reason for HTTP/1 and it's less important for HTTP/2.

In any case, investigate.

gopherbot · 2016-10-18T09:06:13Z

CL https://golang.org/cl/31326 mentions this issue.

…Length Updates golang/go#17480 (fixes after vendor to std) Updates golang/go#17071 (a better fix than the original one) Change-Id: Ibb49d2cb825e28518e688b5c3e0952956a305050 Reviewed-on: https://go-review.googlesource.com/31326 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org>

gopherbot · 2016-10-18T19:20:40Z

CL https://golang.org/cl/31361 mentions this issue.

…rt to determine ContentLength Updates golang/go#17480 (fixes after vendor to std) Updates golang/go#17071 (a better fix than the original one) Change-Id: Ibb49d2cb825e28518e688b5c3e0952956a305050 Reviewed-on: https://go-review.googlesource.com/31326 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org> Reviewed-on: https://go-review.googlesource.com/31361 Run-TryBot: Chris Broadfoot <cbro@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

gopherbot · 2016-10-18T19:55:13Z

CL https://golang.org/cl/31432 mentions this issue.

gopherbot · 2016-10-18T21:45:24Z

CL https://golang.org/cl/31434 mentions this issue.

The way to send an explicitly-zero Content-Length is to set a nil Body. Fix this test to do that, rather than relying on type sniffing. Updates #17480 Updates #17071 Change-Id: I6a38e20f17013c88ec4ea69d73c507e4ed886947 Reviewed-on: https://go-review.googlesource.com/31434 TryBot-Result: Gobot Gobot <gobot@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org>

gopherbot · 2016-10-18T23:40:11Z

CL https://golang.org/cl/31437 mentions this issue.

…0 Body more reliably The way to send an explicitly-zero Content-Length is to set a nil Body. Fix this test to do that, rather than relying on type sniffing. Updates #17480 Updates #17071 Change-Id: I6a38e20f17013c88ec4ea69d73c507e4ed886947 Reviewed-on: https://go-review.googlesource.com/31434 TryBot-Result: Gobot Gobot <gobot@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org> Reviewed-on: https://go-review.googlesource.com/31437 Run-TryBot: Chris Broadfoot <cbro@golang.org>

Updates bundled http2 for x/net/http2 git rev d4c55e66 for: [release-branch.go1.7] http2: never Read from Request.Body in Transport to determine ContentLength https://golang.org/cl/31361 Updates #17480 Updates #17071 Change-Id: I2231adaed3cb5b368927a9654dcf7e69a8b664b6 Reviewed-on: https://go-review.googlesource.com/31432 Run-TryBot: Chris Broadfoot <cbro@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org>

gopherbot · 2016-10-19T12:05:32Z

CL https://golang.org/cl/31445 mentions this issue.

… Transport This CL makes NewRequest set Body nil for known-zero bodies, and makes the http1 Transport not peek-Read a byte to determine whether there's a body. Background: Many fields of the Request struct have different meanings for whether they're outgoing (via the Transport) or incoming (via the Server). For outgoing requests, ContentLength and Body are documented as: // Body is the request's body. // // For client requests a nil body means the request has no // body, such as a GET request. The HTTP Client's Transport // is responsible for calling the Close method. Body io.ReadCloser // ContentLength records the length of the associated content. // The value -1 indicates that the length is unknown. // Values >= 0 indicate that the given number of bytes may // be read from Body. // For client requests, a value of 0 with a non-nil Body is // also treated as unknown. ContentLength int64 Because of the ambiguity of what ContentLength==0 means, the http1 and http2 Transports previously Read the first byte of a non-nil Body when the ContentLength was 0 to determine whether there was an actual body (with a non-zero length) and ContentLength just wasn't populated, or it was actually empty. That byte-sniff has been problematic and gross (see #17480, #17071) and was removed for http2 in a previous commit. That means, however, that users doing: req, _ := http.NewRequest("POST", url, strings.NewReader("")) ... would not send a Content-Length header in their http2 request, because the size of the reader (even though it was known, being one of the three common recognized types from NewRequest) was zero, and so the HTTP Transport thought it was simply unset. To signal explicitly-zero vs unset-zero, this CL changes NewRequest to signal explicitly-zero by setting the Body to nil, instead of the strings.NewReader("") or other zero-byte reader. This CL also removes the byte sniff from the http1 Transport, like https://golang.org/cl/31326 did for http2. Updates #17480 Updates #17071 Change-Id: I329f02f124659bf7d8bc01e2c9951ebdd236b52a Reviewed-on: https://go-review.googlesource.com/31445 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

gopherbot · 2016-10-21T21:00:38Z

CL https://golang.org/cl/31737 mentions this issue.

…0 Body more reliably The way to send an explicitly-zero Content-Length is to set a nil Body. Fix this test to do that, rather than relying on type sniffing. Updates golang#17480 Updates golang#17071 Change-Id: I6a38e20f17013c88ec4ea69d73c507e4ed886947 Reviewed-on: https://go-review.googlesource.com/31434 TryBot-Result: Gobot Gobot <gobot@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org> Reviewed-on: https://go-review.googlesource.com/31437 Run-TryBot: Chris Broadfoot <cbro@golang.org>

Updates bundled http2 for x/net/http2 git rev d4c55e66 for: [release-branch.go1.7] http2: never Read from Request.Body in Transport to determine ContentLength https://golang.org/cl/31361 Updates golang#17480 Updates golang#17071 Change-Id: I2231adaed3cb5b368927a9654dcf7e69a8b664b6 Reviewed-on: https://go-review.googlesource.com/31432 Run-TryBot: Chris Broadfoot <cbro@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org>

gopherbot · 2016-12-22T06:01:01Z

CL https://golang.org/cl/34668 mentions this issue.

In Go 1.8, we'd removed the Transport's Request.Body one-byte-Read-sniffing to disambiguate between non-nil Request.Body with a ContentLength of 0 or -1. Previously, we tried to see whether a ContentLength of 0 meant actually zero, or just an unset by reading a single byte of the Request.Body and then stitching any read byte back together with the original Request.Body. That historically has caused many problems due to either data races, blocking forever (#17480), or losing bytes (#17071). Thus, we removed it in both HTTP/1 and HTTP/2 in Go 1.8. Unfortunately, during the Go 1.8 beta, we've found that a few people have gotten bitten by the behavior change on requests with methods typically not containing request bodies (e.g. GET, HEAD, DELETE). The most popular example is the aws-go SDK, which always set http.Request.Body to a non-nil value, even on such request methods. That was causing Go 1.8 to send such requests with Transfer-Encoding chunked bodies, with zero bytes, confusing popular servers (including but limited to AWS). This CL partially reverts the no-byte-sniffing behavior and restores it only for GET/HEAD/DELETE/etc requests, and only when there's no Transfer-Encoding set, and the Content-Length is 0 or -1. Updates #18257 (aws-go) bug And also private bug reports about non-AWS issues. Updates #18407 also, but haven't yet audited things enough to declare it fixed. Change-Id: Ie5284d3e067c181839b31faf637eee56e5738a6a Reviewed-on: https://go-review.googlesource.com/34668 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

luna-duclos changed the title ~~x/net/http2: Client tries a Read() to determine body length~~ x/net/http2: Transport hangs forever if using a pipe that has not been written to yet as req.Body Oct 17, 2016

bradfitz self-assigned this Oct 17, 2016

bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 17, 2016

bradfitz added this to the Go1.8Maybe milestone Oct 17, 2016

bradfitz mentioned this issue Oct 18, 2016

net/http: first byte of outgoing request body is sometimes lost #17071

Closed

bradfitz modified the milestones: Go1.8, Go1.8Maybe Oct 18, 2016

bradfitz removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 18, 2016

quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 21, 2016

gopherbot closed this as completed in a50fbcd Oct 21, 2016

bradfitz mentioned this issue May 5, 2017

net/http: client do not set Content-Length: 0 with an io.Reader with no bytes. #20257

Closed

golang locked and limited conversation to collaborators Dec 22, 2017

gopherbot added the FrozenDueToAge label Dec 22, 2017

rsc unassigned bradfitz Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/net/http2: Transport hangs forever if using a pipe that has not been written to yet as req.Body #17480

x/net/http2: Transport hangs forever if using a pipe that has not been written to yet as req.Body #17480

luna-duclos commented Oct 17, 2016

bradfitz commented Oct 17, 2016

luna-duclos commented Oct 17, 2016 •

edited

bradfitz commented Oct 17, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 19, 2016

gopherbot commented Oct 21, 2016

gopherbot commented Dec 22, 2016

x/net/http2: Transport hangs forever if using a pipe that has not been written to yet as req.Body #17480

x/net/http2: Transport hangs forever if using a pipe that has not been written to yet as req.Body #17480

Comments

luna-duclos commented Oct 17, 2016

bradfitz commented Oct 17, 2016

luna-duclos commented Oct 17, 2016 • edited

bradfitz commented Oct 17, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 18, 2016

gopherbot commented Oct 19, 2016

gopherbot commented Oct 21, 2016

gopherbot commented Dec 22, 2016

luna-duclos commented Oct 17, 2016 •

edited