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

Treat as: :html tests request params as :url_encoded_form #50390

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Dec 18, 2023

Motivation / Background

Closes #50345

Detail

First, handle the exception mentioned in #50345:

BugTest#test_params_with_htm_content_type:
NoMethodError: undefined method `to_html' for {:name=>"Muad'Dib"}:Hash
    .../actionpack/lib/action_dispatch/testing/request_encoder.rb:39:in `encode_params'
    .../actionpack/lib/action_dispatch/testing/integration.rb:251:in `process'

Calls with as: :html result in a NoMethodError because
Hash#to_html does not exist.

Passing as: :html implies that the request parameters will come from a
POST body encoded as text/html. That isn't entirely true -- browsers
will encode POST parameters as with the Content-Type: header set to
either application/x-www-form-urlencoded or multipart/form-data.
This commit skips setting the CONTENT_TYPE Rack header when processed
with as: :html.

To account for that, extend the RequestEncoder constructor to accept a
content_type argument to use when provided. When omitted, continue to
fall back to the provided MIME type. Extend the default :html encoder
configuration to default to submitting with Content-Type: x-www-form-urlencoded.

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@seanpdoyle
Copy link
Contributor Author

@rafaelfranca could you review this, since you've reviewed and merged #47144?

@seanpdoyle seanpdoyle force-pushed the issue-50345 branch 2 times, most recently from fed510e to ff91bc3 Compare March 8, 2024 15:52
@seanpdoyle seanpdoyle changed the title Handle as: :html in IntegrationTest Treat as: :html tests request params as :url_encoded_form Mar 8, 2024
@seanpdoyle seanpdoyle force-pushed the issue-50345 branch 2 times, most recently from 091e925 to 68d409f Compare April 24, 2024 13:23
Closes [rails#50345][]

First, handle the exception mentioned in [rails#50345][]:

```
BugTest#test_params_with_htm_content_type:
NoMethodError: undefined method `to_html' for {:name=>"Muad'Dib"}:Hash
    .../actionpack/lib/action_dispatch/testing/request_encoder.rb:39:in `encode_params'
    .../actionpack/lib/action_dispatch/testing/integration.rb:251:in `process'
```

Calls with `as: :html` result in a `NoMethodError` because
`Hash#to_html` does not exist.

Passing `as: :html` implies that the request parameters will come from a
`POST` body encoded as `text/html`. That isn't entirely true -- browsers
will encode `POST` parameters as with the `Content-Type:` header set to
either [application/x-www-form-urlencoded][] or [multipart/form-data][].
This commit skips setting the `CONTENT_TYPE` Rack header when processed
with `as: :html`.

To account for that, extend the `RequestEncoder` constructor to accept a
`content_type `argument to use when provided. When omitted, continue to
fall back to the provided MIME type. Extend the default `:html` encoder
configuration to default to submitting with `Content-Type:
x-www-form-urlencoded`.

[rails#50345]: rails#50345
[application/x-www-form-urlencoded]: https://developer.mozilla.org/en-US/docs/Learn/Forms/Sending_and_retrieving_form_data#the_post_method
[multipart/form-data]: https://developer.mozilla.org/en-US/docs/Learn/Forms/Sending_and_retrieving_form_data#the_enctype_attribute
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in integration tests when providing as: :html
1 participant