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

Chunked request bodies are not supported #620

Closed
javanthropus opened this issue Dec 11, 2014 · 11 comments

Comments

Projects
None yet
7 participants
@javanthropus
Copy link

commented Dec 11, 2014

If I understand the HTTP RFC correctly, an HTTP 1.1 web server must support chunked request bodies. As of version 2.10.2, Puma does not and instead treats any such request as having no body at all. This inability of Puma was originally raised as issue #80 which has been closed without resolution. Please see that issue for the reasons. My hope is that this issue addresses those reasons better so that we can add support for this.

A simple Rack app demonstrates the problem:

run proc { |env| [200, {}, [env['rack.input'].read.bytesize.to_s]] }

Run that with Puma and use curl to upload some data in a request using the chunked transfer encoding:

dd if=/dev/zero bs=1 count=10240 |
  curl -v -X POST -H 'Transfer-Encoding: chunked' --data-binary @- localhost:9292

Under Puma, the response reports that the request body contained 0 bytes. This works correctly under Unicorn and reports 10240 bytes.

Interestingly, sending 1024 bytes or less in the request causes Puma to complain on the server side that the request could not be parsed even though the Rack app returns a response code of 200. Larger requests do not trigger the server side exception.

127.0.0.1 - - [11/Dec/2014 08:55:04] "POST / HTTP/1.1" 200 - 0.0001
2014-12-11 08:55:04 -0600: HTTP parse error, malformed request (): #<Puma::HttpParserError: Invalid HTTP format, parsing fails.>
2014-12-11 08:55:04 -0600: ENV: {"rack.version"=>[1, 2], "rack.errors"=>#<IO:<STDERR>>, "rack.multithread"=>true, "rack.multiprocess"=>false, "rack.run_once"=>false, "SCRIPT_NAME"=>"", "CONTENT_TYPE"=>"text/plain", "QUERY_STRING"=>"", "SERVER_PROTOCOL"=>"HTTP/1.1", "SERVER_SOFTWARE"=>"2.10.2", "GATEWAY_INTERFACE"=>"CGI/1.2"}

I discovered this issue while attempting to host grack with Puma. After a certain threshold (somewhere less than 7MB), git will chunk commits during a push operation in order to enable streaming, so the fix for this needs to avoid attempting to dump the entire request body into a temporary file prior to handing the content to the app since those requests are unbounded in size. It needs to wrap the socket content in a streaming decoder for chunked data and pass that up to the app.

@evanphx

This comment has been minimized.

Copy link
Member

commented Dec 13, 2014

Yes, that's correct. I've had chunked request handling on the todo list, but I haven't gotten to it yet.

@bbozo

This comment has been minimized.

Copy link

commented Jun 10, 2015

I'd like to give this a go if it's not overly complicated, or plan B switch to Trinidad and cross fingers and I hate Tomcat with equal passion with which I love puma <3 Not being able to upload from Android is a kind of a big issue for us, argh

@evanphx can you maybe point me in a good direction how to tackle this?

@thom-nic

This comment has been minimized.

Copy link

commented Jun 10, 2015

@bbozo wow interesting that you just commented on this. I recently ran into issues with chunked Transfter-Encoding across a number of different Ruby HTTP servers (Puma being one of them.)

I want to reference this rails issue for context then make a suggestion...

If you follow the (somewhat confusing) precedent that Webrick/ Unicorn/ Rainbows set (see linked issue above) you'll be passing along a request that ActionDispatch::Request fails to handle correctly if the Content-Length header is still missing.

So I'd suggest taking one of the two approaches:

  1. Parse & reassemble the chunked data, reassign env['rack.input'] to the decoded body, and add a Content-Length header. I'd also remove the Transfer-Encoding header, at which point rack/rails can be oblivious to the fact that the request was ever chunked.
  2. Simply read the full request body but don't decode it, and pass it along to rack. This is more of a "hands-off" approach which IMO is still valid but requires the user to add rack middleware to actually decode the request. Another example which I think validates this approach is this middleware for handling Content-Encoding.

That said, I want to say thanks for tackling this! I was rather surprised when I figured out thin doesn't support this then I tried Puma which didn't work either :)

@thom-nic

This comment has been minimized.

Copy link

commented Jun 11, 2015

@bbozo if you want to try your hand at this, it looks like this is where you need to start. Unfortunately I'm not familiar with Ragel so it's all a bit of a mystery to me.

@javanthropus

This comment has been minimized.

Copy link
Author

commented Jun 11, 2015

Since Unicorn already does this, it may also be useful to check out how this is done in its fork of the parser: http://bogomips.org/unicorn.git/tree/ext/unicorn_http.

@thom-nic

This comment has been minimized.

Copy link

commented Jun 11, 2015

Well that's certainly helpful. I didn't realize the projects share a common code ancestor. Unfortunately ragel still seems way too far over my head for me to contribute a patch in the near future.

@bbozo

This comment has been minimized.

Copy link

commented Jun 11, 2015

Indeed :) defunkt/unicorn@81026ea#diff-35c8f85f334e5bc83c3623ac6e9f4f94

I'll take a look, maybe we can beg for help some of the guys from unicorn too ^_^

@fedesoria

This comment has been minimized.

Copy link

commented Jul 15, 2015

Is there any workaround for this?

@bbozo

This comment has been minimized.

Copy link

commented Jul 17, 2015

Hi :) for the moment, it seems this is the only solution: http://atnan.com/blog/2008/08/08/transfer-encoding-chunked-or-chunky-http/

We "have" a workaround for our use case with a free tier Amazon EC2 VM to fix the requests - it's not implemented yet mind you, so I can not guarantee 100%, our plan is to make uploads go directly to S3 in the future - which is really the better option for us anyway

I'm interested in playing with this, I just need to get some slack of my work to be able to tackle it and learn some ragel :)

@chewi

This comment has been minimized.

Copy link
Contributor

commented Oct 20, 2015

This is something I'd like to have but I'm not desperately needing it right now. If it were to be done, I'd like to have the option for Puma not to dechunk the request in the middle because otherwise you'd lose some of the benefit. I'd be putting this in front of Grape, which might handle this better than Rails. I used to fetch large CSV files over SFTP and load them on the fly, sometimes even decompressing them and decrypting them in the process. It would be nice to be able to do the same over HTTP.

@marianandre42

This comment has been minimized.

Copy link

commented Mar 22, 2016

+1

@evanphx evanphx closed this in 5ec232d Jul 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.