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

raw_post being empty for "Transfer-Encoding: chunked" #46784

Closed
tooooooooomy opened this issue Dec 21, 2022 · 4 comments
Closed

raw_post being empty for "Transfer-Encoding: chunked" #46784

tooooooooomy opened this issue Dec 21, 2022 · 4 comments

Comments

@tooooooooomy
Copy link
Contributor

raw_post of chunked request is empty. I would like to open a feature request to handle chunked request by action pack.
I found the same issue was raised by @hahmed and he sent the PR to fix it but was closed at that time.
The reason why the PR closed is the problem was solved with Puma by adding CONTENT_LENGTH request header at the Puma layer by this PR.
However, the same problem still happens with unicorn, thin and webrick at least.
It is helpful to us application developers if this problem would be taken care by rails so that we won't have to check neither the request is chunked nor which rack http server is used.

Steps to reproduce

I prepared environments to reproduce the problem via docker. Please download it and follow the instruction of README.
https://github.com/kazu9su/rails-experiment

Expected behavior

If the chunked request is sent to the rails server, application can read the raw_post with any rack http server.

Actual behavior

We can access to the request body with raw_post with only the rack http server which buffers the request body upfront like Puma.

  • Reproduced the problem with...
@hahmed
Copy link
Contributor

hahmed commented Jan 16, 2023

Hey @kazu9su, thanks for reporting this issue. I'm going to take a look at this some time this week, apologies its taken a while to get around to it!

pedro108 added a commit to pedro108/rails that referenced this issue Feb 10, 2023
@hahmed
Copy link
Contributor

hahmed commented Feb 15, 2023

@kazu9su a fix was merged upstream, any chance you could retest?

@hahmed hahmed added more-information-needed When reporter needs to provide more information actionpack labels Feb 15, 2023
@tooooooooomy
Copy link
Contributor Author

tooooooooomy commented Feb 27, 2023

@hahmed First of all, thank you for letting me know it and thank you all to work on this issue!

I tested this version of rails 097ead2 with unicorn 6.1.0 app, and found out all things work well except for 'CONTENT_LENGTH' value of request.headers.

With Puma 5.6.5 app, I can get correct CONTENT_LENGTH value from request.headers hash like this but it showed as nil on unicorn 6.1.0 app.

I suppose that it would solve this problem that setting CONTENT_LENGTH at the same time. How do you think?

@rails-bot rails-bot bot removed the more-information-needed When reporter needs to provide more information label Feb 27, 2023
@tooooooooomy
Copy link
Contributor Author

tooooooooomy commented Mar 12, 2023

@hahmed
I confirmed what I addressed again about request.headers property and I believe that rails handles them as raw request headers sent through the request. Given that, providing content_length or raw_post methods seems enough to satisfy what I raised as this issue. I close this.

Again, thank you for the great work!

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

No branches or pull requests

2 participants