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

JSON requests with Chunked encoding are not handled correctly #15079

Open
xaviershay opened this issue May 12, 2014 · 10 comments
Open

JSON requests with Chunked encoding are not handled correctly #15079

xaviershay opened this issue May 12, 2014 · 10 comments

Comments

@xaviershay
Copy link
Contributor

(Continuation of #7556, since that ticket became confusing.)

On Rails 4.1.1 (and current master), posting a JSON body with chunked transfer encoding, the body is ignored and no params are accessible in the controller.

Here is a repro: https://github.com/xaviershay/chunked_repro (see README)
Here is a proof-of-concept fix: https://github.com/xaviershay/rails/tree/chunked-fix

If the fix is directionally correct, I can try to clean it up into a PR.

@rails-bot
Copy link

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@eval
Copy link
Contributor

eval commented Sep 24, 2014

On Rails 4.2.0.beta1 this issue still exists.

@rails-bot
Copy link

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@mattbornski
Copy link

bump

@trostli
Copy link

trostli commented Feb 21, 2015

This still seems to be an issue, I can reproduce it on Rails 4.2

@mchapman17
Copy link

Any progress on this? Currently having the same problem with Rails 4.2.0 as well.

@trostli
Copy link

trostli commented May 12, 2015

Bump. This is still an issue...

@coorasse
Copy link
Contributor

I created a PR but I'm not sure about security implications

@thom-nic
Copy link

I'm not confident the proposed PR fixes the underlying problem(s).

First off the chunked data, which looks like this needs to be reassembled before it can possibly be decoded as valid JSON. This could be done through a Rack middleware like this however I'm running into additional problems...

When I attempt to parse the chunked POST body from my middleware (which is near the beginning of the middlware chain,) I only get the first TCP packet of POST data - the rest is truncated. At that point my env['rack.input'] is a StringIO so seems something upstream is doing the basic parsing of the HTTP request off the socket and losing the remainder of the body. From wireshark I can see the complete raw request arrive at my machine.

I'm not familiar enough with Rack to know - does Rack or Thin/Puma/Webrick do the initial HTTP parse?

@thom-nic
Copy link

Update: the truncated body appears to be an issue with thin. When I switched to Webrick I get the full request body. Oddly, Webrick appears to properly decode the body but leaves the Transfer-Encoding: chunked header in there which is confusing. Which I think is where the proposed fix mentioned here comes from - I think it will work for Webrick but definitely not thin. In the process of investigating others.

So - it seems ruby HTTP servers are inconsistent in this area. But I think, depending on which server you are running, you could get around this issue via a rack middleware.

Edit: From my testing, webrick and unicorn/rainbows appear to decode the chunks and leave the header (so the given patch would I think work for them.) Thin gives one (still-encoded) TCP packet and Puma gives an empty body.

Edit 2: For anyone interested and/or stumbling upon this issue from the internet, I've updated the middleware I wrote to work around this issue if you are using Webrick/Unicorn/Rainbows/Zbatery as your HTTP server. e.g. when the chunked body is actually decoded by the HTTP server but the Content-Length is missing. It basically reads the body and inserts a Content-Length header so that ActionDispatch::Request can process the request as if it were an HTTP 1.0 request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants