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.

Sign up for GitHub

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

Jump to bottom

Fixes raw_post being empty for "Transfer-Encoding: chunked" #37423

Closed
wants to merge 1 commit into from
Closed

Fixes raw_post being empty for "Transfer-Encoding: chunked" #37423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion actionpack/lib/action_dispatch/http/request.rb
View file
Open in desktop
Expand Up @@ -321,12 +321,23 @@ def server_software
def raw_post
unless has_header? "RAW_POST_DATA"
raw_post_body = body
set_header("RAW_POST_DATA", raw_post_body.read(content_length))
set_header("RAW_POST_DATA",
if chunked?
raw_post_body.rewind if raw_post_body.respond_to?(:rewind)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for chunked requests had to rewind again before reading for some reason

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented out this line, and ran the tests in test_case_test.rb and they all passed. Are you sure this rewind is necessary? And if so, could a test be added to ensure it doesn't regress?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rewind is necessary, in the rails app I linked, if this line is commented out, it never worked.

Will test this out again and make sure it's working as it should (perhaps this test itself is not properly testing out the feature), will test and confirm.

Thanks for the reminder on this PR, been a while since I looked at it. 👍

raw_post_body.read
else
raw_post_body.read(content_length)
end
)
raw_post_body.rewind if raw_post_body.respond_to?(:rewind)
end
get_header "RAW_POST_DATA"
end

def chunked?
headers["Transfer-Encoding"] =~ /chunked/i
end

# The request body is an IO input stream. If the RAW_POST_DATA environment
# variable is already set, wrap it in a StringIO.
def body
Expand Down
6 changes: 6 additions & 0 deletions actionpack/test/controller/test_case_test.rb
View file
Open in desktop
Expand Up @@ -990,6 +990,12 @@ def test_parsed_body_with_as_option
post :render_json, body: { foo: "heyo" }.to_json, as: :json
assert_equal({ "foo" => "heyo" }, response.parsed_body)
end

def test_chunked_request
@request.headers["Transfer-Encoding"] = "chunked"
post :test_params, params: { foo: "heyo" }, xhr: true
assert_equal("foo=heyo", @request.raw_post)
end
end

class ResponseDefaultHeadersTest < ActionController::TestCase
Expand Down