Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: requests with "Transfer-Encoding: chunked" body may buffer small chunks #6574

Closed
benburkert opened this issue Oct 12, 2013 · 10 comments
Milestone

Comments

@benburkert
Copy link
Contributor

Hello,

I am attempting to implement a client for a http fanout service in go. The utility
establishes an http connection and makes a POST request with a "Transfer-Encoding:
chunked" body, then reads data from stdin and writes them as individual chunks to
the request body.

I have noticed that small chunks are buffered instead of flushed to the wire
immediately. This seems to happen because the io.Reader initialized in the Request.Body
is wrapped in a bufio.Reader when building the persistent connection:
http://golang.org/src/pkg/net/http/transport.go#L509

The bufio.Reader is initialized with the default buffer size (defaultBufSize = 4096) so
chunks smaller than this size are buffered internally before sent to the wire. In my
opinion these chunks should be flushed to the wire no matter the size.

What steps will reproduce the problem?
1. run ngrep to sniff the TCP packets sent to port 8080: "ngrep -W byline -d lo0
port 8080"
2. start a basic web server on port 8080: "{ sleep 5 ; echo -ne "HTTP/1.1 200
OK\r\nContent-Length: 0\r\n\r\n" ; } | nc -l 8080"
3. run http://play.golang.org/p/CZaWdr4oft

What is the expected output?

The expected output from ngrep should have 3 separate TCP packets sent from the go
process to the server:

ngrep -W byline -d lo0 port 8080
interface: lo0 (127.0.0.0/255.0.0.0)
filter: (ip) and ( port 8080 )
#####
T 127.0.0.1:52415 -> 127.0.0.1:8080 [AP]
POST / HTTP/1.1.
Host: 127.0.0.1:8080.
User-Agent: Go 1.1 package http.
Transfer-Encoding: chunked.
Accept-Encoding: gzip.
.
b.
First Chunk
#
T 127.0.0.1:52415 -> 127.0.0.1:8080 [AP]
.
c.
Second Chunk
#
T 127.0.0.1:52415 -> 127.0.0.1:8080 [AP]
.
0.
.

####
T 127.0.0.1:8080 -> 127.0.0.1:52415 [AP]
HTTP/1.1 200 OK.
Content-Length: 0.
.

######

What do you see instead?

The output from ngrep only contains a single TCP packet from the go process to the
server:

ngrep -W byline -d lo0 port 8080
interface: lo0 (127.0.0.0/255.0.0.0)
filter: (ip) and ( port 8080 )
#####
T 127.0.0.1:52641 -> 127.0.0.1:8080 [AP]
POST / HTTP/1.1.
Host: 127.0.0.1:8080.
User-Agent: Go 1.1 package http.
Transfer-Encoding: chunked.
Accept-Encoding: gzip.
.
b.
First Chunk.
c.
Second Chunk.
0.
.

##
T 127.0.0.1:8080 -> 127.0.0.1:52641 [AP]
HTTP/1.1 200 OK.
Content-Length: 0.
.

######

Which operating system are you using?

OSX

Which version are you using?  (run 'go version')

go version go1.1.2 darwin/amd64
@benburkert
Copy link
Contributor Author

Comment 1:

My proposed fix is to flush the wire after writing each chunk if the wire is a
bufio.Writer: https://golang.org/cl/14438065

@bradfitz
Copy link
Contributor

Comment 2:

Too late for Go 1.2 (not a regression, either), and probably not the right change.  Best
might be an option on the Transport, since there's nowhere to add an optional Flush
method, and changing this behavior for all bufio.Writers isn't correct.

Labels changed: added go1.3maybe, removed priority-triage.

Status changed to Thinking.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 3:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 4:

Labels changed: added repo-main.

@cyx
Copy link

cyx commented May 13, 2015

Is there a known workaround for this issue?

@bradfitz bradfitz modified the milestones: Go1.5Maybe, Unplanned May 13, 2015
@bradfitz
Copy link
Contributor

No. There are several easy fixes. I'm debating which I like the most. I'll try to get one into 1.5.

@cyx
Copy link

cyx commented May 13, 2015

Ok thanks. So I take it I can't do any workarounds for this that doesn't involve patching the stdlib then?
Currently I've devolved into shelling into curl to make tightly streamed text output work -- but looking forward to the fix.

@bradfitz
Copy link
Contributor

There are no workarounds.

@bradfitz bradfitz removed the Thinking label May 13, 2015
@gopherbot
Copy link

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

@cyx
Copy link

cyx commented May 13, 2015

Awesome! I'll be sure to try HEAD if this gets merged in :)

On Wed, May 13, 2015 at 1:00 PM, GopherBot notifications@github.com wrote:

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


Reply to this email directly or view it on GitHub
#6574 (comment).

@mikioh mikioh modified the milestones: Go1.5, Go1.5Maybe May 15, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants