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

Regression in integration tests when providing as: :html #50345

Open
inkstak opened this issue Dec 13, 2023 · 4 comments · May be fixed by #50390
Open

Regression in integration tests when providing as: :html #50345

inkstak opened this issue Dec 13, 2023 · 4 comments · May be fixed by #50390

Comments

@inkstak
Copy link

inkstak commented Dec 13, 2023

Steps to reproduce

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "main"
end

require "action_controller/railtie"

class TestApp < Rails::Application
  config.hosts << "www.example.com"
  routes.draw do
    resources :characters
  end

  config.root   = __dir__
  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger
end

class CharactersController < ActionController::Base
  def create
    render head: :no_content
  end
end

require "minitest/autorun"
require "rack/test"

class BugTest < ActionDispatch::IntegrationTest
  def test_params_without_content_type
    post "/characters", params: { name: "Muad'Dib" }
  end

  def test_params_with_htm_content_type
    post "/users", as: :html, params: { name: "Muad'Dib" }
  end

  private

  def app
    Rails.application
  end
end

Expected behavior

In Rails 7.0, the second test works.

Actual behavior

In Rails 7.1, the second test will raise an error :

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'
    .../actionpack/lib/action_dispatch/testing/integration.rb:22:in `post'
    .../actionpack/lib/action_dispatch/testing/integration.rb:379:in `post'
    issue_bug.rb:39:in `test_params_with_htm_content_type'

System configuration

Rails version:

Rails 7.1.2 & main

Ruby version:

Ruby 3.2.2

@inkstak
Copy link
Author

inkstak commented Dec 13, 2023

The issue comes from this new line in action_dispatch/testing/request_encoder.rb

+   register_encoder :html, response_parser: -> body { Rails::Dom::Testing.html_document.parse(body) }

Without HTML encoder, the default encoderIdentityEncoder returned the params as passed.

Now that an HTML encoder has been registered, it tries to call :"to_#{@mime.symbol}" on params by default.

My first thought was to register a param_encoder :

ActionDispatch::IntegrationTest.register_encoder :html,
  response_parser: ->(body) { Rails::Dom::Testing.html_document.parse(body) },
  param_encoder: ->(params) { params }

But it doesn't seem to work : the request is now performed but params are not passed to the controller.

EDIT: The patch suggested is working. I'll try to pull a request asap.

@mguan2020
Copy link
Contributor

Thank you very much @inkstak for the detailed explanations. I have attached a PR that implements your suggested change as well as the corresponding tests.

Please let me know if there is anything you would like me to fix.

@seanpdoyle
Copy link
Contributor

seanpdoyle commented Dec 18, 2023

In the case you've shared above, 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 either application/x-www-form-urlencoded or multipart/form-data.

In the short-term, you might be better off omitting the as: :html entirely and letting Action Dispatch's test harness determine the proper content type and parameter encoding.

In the long-term, the as: :html keyword argument might require some special treatment. I've opened #50390 as an alternative to #50375.

seanpdoyle added a commit to seanpdoyle/rails that referenced this issue Dec 18, 2023
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'
```

When an `ActionDispatch::Testing::RequestEncoder` registered through a
call to `register_encoder` does not define a `:param_encoder` keyword
argument **and** the params do not respond to a `to_$MIME` method,
fallback to returning the `params` themselves, unchanged. This behavior
is what's implemented by the private `IdentityEncoder` class, so mimic
it for defined encoders.

Next, special case handling requests with `as: :html`. 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`.

[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
@seanpdoyle seanpdoyle linked a pull request Dec 18, 2023 that will close this issue
4 tasks
@inkstak
Copy link
Author

inkstak commented Dec 21, 2023

In the case you've shared above, 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 either application/x-www-form-urlencoded or multipart/form-data.

@seanpdoyle I couldn't agree more. Using as: :html seems unnecessary when you're testing endpoints that are intended to be used in a browser.

I used to use it as RSpec metadata to explicitly test responses to unexpected formats against API endpoints.
Because Rails default format for */* is HTML, I used as: :html :

RSpec.describe "API::ResourcesController#create" do
  subject do
    post "/resources", as:, headers:, params:
  end

  let(:as) { |e| e.metadata[:as] }
  
  context "when requesting JSON", as: :json do
    it_behaves_like "it requires authentication"
    it_behaves_like "it responds successfully when signed in"
  end

  context "when requesting HTML", as: :html do
    it_behaves_like "it responds with not acceptable"
  end
end

seanpdoyle added a commit to seanpdoyle/rails that referenced this issue Mar 8, 2024
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'
```

When an `ActionDispatch::Testing::RequestEncoder` registered through a
call to `register_encoder` does not define a `:param_encoder` keyword
argument **and** the params do not respond to a `to_$MIME` method,
fallback to returning the `params` themselves, unchanged. This behavior
is what's implemented by the private `IdentityEncoder` class, so mimic
it for defined encoders.

Next, special case handling requests with `as: :html`. 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`.

[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
seanpdoyle added a commit to seanpdoyle/rails that referenced this issue Mar 8, 2024
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
seanpdoyle added a commit to seanpdoyle/rails that referenced this issue Mar 24, 2024
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
seanpdoyle added a commit to seanpdoyle/rails that referenced this issue Apr 18, 2024
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
seanpdoyle added a commit to seanpdoyle/rails that referenced this issue Apr 24, 2024
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
seanpdoyle added a commit to seanpdoyle/rails that referenced this issue May 15, 2024
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
seanpdoyle added a commit to seanpdoyle/rails that referenced this issue May 16, 2024
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 a pull request may close this issue.

4 participants