Skip to content

Commit

Permalink
Change ActionDispatch::Response#content_type to return the full Con…
Browse files Browse the repository at this point in the history
…tent-Type header

And deprecate the config to keep the previous behavior.
  • Loading branch information
rafaelfranca committed Oct 30, 2020
1 parent 62bda90 commit 64efe50
Show file tree
Hide file tree
Showing 12 changed files with 26 additions and 121 deletions.
8 changes: 8 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
* Deprecate `config.action_dispatch.return_only_media_type_on_content_type`.

*Rafael Mendonça França*

* Change `ActionDispatch::Response#content_type` to return the full Content-Type header.

*Rafael Mendonça França*

* Remove deprecated `ActionDispatch::Http::ParameterFilter`.

*Rafael Mendonça França*
Expand Down
25 changes: 13 additions & 12 deletions actionpack/lib/action_dispatch/http/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,18 @@ def each(&block)

cattr_accessor :default_charset, default: "utf-8"
cattr_accessor :default_headers
cattr_accessor :return_only_media_type_on_content_type, default: false

def self.return_only_media_type_on_content_type=(*)
ActiveSupport::Deprecation.warn(
".return_only_media_type_on_content_type= is dreprecated with no replacement and will be removed in 6.2."
)
end

def self.return_only_media_type_on_content_type
ActiveSupport::Deprecation.warn(
".return_only_media_type_on_content_type is dreprecated with no replacement and will be removed in 6.2."
)
end

include Rack::Response::Helpers
# Aliasing these off because AD::Http::Cache::Response defines them.
Expand Down Expand Up @@ -243,17 +254,7 @@ def content_type=(content_type)

# Content type of response.
def content_type
if self.class.return_only_media_type_on_content_type
ActiveSupport::Deprecation.warn(
"Rails 6.1 will return Content-Type header without modification." \
" If you want just the MIME type, please use `#media_type` instead."
)

content_type = super
content_type ? content_type.split(/;\s*charset=/)[0].presence : content_type
else
super.presence
end
super.presence
end

# Media type of response.
Expand Down
3 changes: 1 addition & 2 deletions actionpack/lib/action_dispatch/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ class Railtie < Rails::Railtie # :nodoc:
config.action_dispatch.use_authenticated_cookie_encryption = false
config.action_dispatch.use_cookies_with_metadata = false
config.action_dispatch.perform_deep_munge = true
config.action_dispatch.return_only_media_type_on_content_type = true

config.action_dispatch.default_headers = {
"X-Frame-Options" => "SAMEORIGIN",
Expand All @@ -43,10 +42,10 @@ class Railtie < Rails::Railtie # :nodoc:
ActionDispatch::Http::URL.tld_length = app.config.action_dispatch.tld_length
ActionDispatch::Request.ignore_accept_header = app.config.action_dispatch.ignore_accept_header
ActionDispatch::Request::Utils.perform_deep_munge = app.config.action_dispatch.perform_deep_munge

ActiveSupport.on_load(:action_dispatch_response) do
self.default_charset = app.config.action_dispatch.default_charset || app.config.encoding
self.default_headers = app.config.action_dispatch.default_headers
self.return_only_media_type_on_content_type = app.config.action_dispatch.return_only_media_type_on_content_type
end

ActionDispatch::ExceptionWrapper.rescue_responses.merge!(config.action_dispatch.rescue_responses)
Expand Down
9 changes: 0 additions & 9 deletions actionpack/test/controller/render_js_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,4 @@ def test_should_render_js_partial
get :show_partial, format: "js", xhr: true
assert_equal "partial js", @response.body
end

def test_should_not_trigger_content_type_deprecation
original = ActionDispatch::Response.return_only_media_type_on_content_type
ActionDispatch::Response.return_only_media_type_on_content_type = true

assert_not_deprecated { get :render_vanilla_js_hello, xhr: true }
ensure
ActionDispatch::Response.return_only_media_type_on_content_type = original
end
end
18 changes: 0 additions & 18 deletions actionpack/test/controller/render_json_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,22 +133,4 @@ def test_render_json_calls_to_json_from_object
get :render_json_without_options
assert_equal '{"a":"b"}', @response.body
end

def test_should_not_trigger_content_type_deprecation
original = ActionDispatch::Response.return_only_media_type_on_content_type
ActionDispatch::Response.return_only_media_type_on_content_type = true

assert_not_deprecated { get :render_json_hello_world }
ensure
ActionDispatch::Response.return_only_media_type_on_content_type = original
end

def test_should_not_trigger_content_type_deprecation_with_callback
original = ActionDispatch::Response.return_only_media_type_on_content_type
ActionDispatch::Response.return_only_media_type_on_content_type = true

assert_not_deprecated { get :render_json_hello_world_with_callback, xhr: true }
ensure
ActionDispatch::Response.return_only_media_type_on_content_type = original
end
end
9 changes: 0 additions & 9 deletions actionpack/test/controller/render_xml_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,4 @@ def test_should_use_implicit_content_type
get :implicit_content_type, format: "atom"
assert_equal Mime[:atom], @response.media_type
end

def test_should_not_trigger_content_type_deprecation
original = ActionDispatch::Response.return_only_media_type_on_content_type
ActionDispatch::Response.return_only_media_type_on_content_type = true

assert_not_deprecated { get :render_with_to_xml }
ensure
ActionDispatch::Response.return_only_media_type_on_content_type = original
end
end
9 changes: 0 additions & 9 deletions actionpack/test/controller/request_forgery_protection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -616,15 +616,6 @@ def test_should_only_allow_cross_origin_js_get_without_xhr_header_if_protection_
end
end

def test_should_not_trigger_content_type_deprecation
original = ActionDispatch::Response.return_only_media_type_on_content_type
ActionDispatch::Response.return_only_media_type_on_content_type = true

assert_not_deprecated { get :same_origin_js, xhr: true }
ensure
ActionDispatch::Response.return_only_media_type_on_content_type = original
end

def test_should_not_raise_error_if_token_is_not_a_string
assert_blocked do
patch :index, params: { custom_authenticity_token: { foo: "bar" } }
Expand Down
29 changes: 0 additions & 29 deletions actionpack/test/dispatch/response_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -604,33 +604,4 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest
assert_equal("text/csv; header=present", @response.media_type)
assert_equal("utf-16", @response.charset)
end

test "`content type` returns header that excludes `charset` when specified `return_only_media_type_on_content_type`" do
original = ActionDispatch::Response.return_only_media_type_on_content_type
ActionDispatch::Response.return_only_media_type_on_content_type = true

@app = lambda { |env|
if env["PATH_INFO"] == "/with_parameters"
[200, { "Content-Type" => "text/csv; header=present; charset=utf-16" }, [""]]
else
[200, { "Content-Type" => "text/csv; charset=utf-16" }, [""]]
end
}

get "/"
assert_response :success

assert_deprecated do
assert_equal("text/csv", @response.content_type)
end

get "/with_parameters"
assert_response :success

assert_deprecated do
assert_equal("text/csv; header=present", @response.content_type)
end
ensure
ActionDispatch::Response.return_only_media_type_on_content_type = original
end
end
4 changes: 4 additions & 0 deletions guides/source/6_1_release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,12 @@ Please refer to the [Changelog][action-pack] for detailed changes.

### Deprecations

* Deprecate `config.action_dispatch.return_only_media_type_on_content_type`.

### Notable changes

* Change `ActionDispatch::Response#content_type` to return the full Content-Type header.

Action View
-----------

Expand Down
5 changes: 0 additions & 5 deletions guides/source/configuring.md
Original file line number Diff line number Diff line change
Expand Up @@ -625,10 +625,6 @@ Defaults to `'signed cookie'`.

Any exceptions that are not configured will be mapped to 500 Internal Server Error.

* `config.action_dispatch.return_only_media_type_on_content_type` change the
return value of `ActionDispatch::Response#content_type` to the Content-Type
header without modification. Defaults to `false`.

* `config.action_dispatch.cookies_same_site_protection` configures the default
value of the `SameSite` attribute when setting cookies. When set to `nil`, the
`SameSite` attribute is not added. To allow the value of the `SameSite` attribute
Expand Down Expand Up @@ -1024,7 +1020,6 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla
- `config.autoloader`: `:zeitwerk`
- `config.action_view.default_enforce_utf8`: `false`
- `config.action_dispatch.use_cookies_with_metadata`: `true`
- `config.action_dispatch.return_only_media_type_on_content_type`: `false`
- `config.action_mailer.delivery_job`: `"ActionMailer::MailDeliveryJob"`
- `config.active_storage.queues.analysis`: `:active_storage_analysis`
- `config.active_storage.queues.purge`: `:active_storage_purge`
Expand Down
1 change: 0 additions & 1 deletion railties/lib/rails/application/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ def load_defaults(target_version)

if respond_to?(:action_dispatch)
action_dispatch.use_cookies_with_metadata = true
action_dispatch.return_only_media_type_on_content_type = false
end

if respond_to?(:action_mailer)
Expand Down
27 changes: 0 additions & 27 deletions railties/test/application/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2453,33 +2453,6 @@ class ::DummySerializer < ActiveJob::Serializers::ObjectSerializer; end
assert_nil ActiveStorage.queues[:purge]
end

test "ActionDispatch::Response.return_only_media_type_on_content_type is false by default" do
app "development"

assert_equal false, ActionDispatch::Response.return_only_media_type_on_content_type
end

test "ActionDispatch::Response.return_only_media_type_on_content_type is true in the 5.x defaults" do
remove_from_config '.*config\.load_defaults.*\n'
add_to_config 'config.load_defaults "5.2"'

app "development"

assert_equal true, ActionDispatch::Response.return_only_media_type_on_content_type
end

test "ActionDispatch::Response.return_only_media_type_on_content_type can be configured in the new framework defaults" do
remove_from_config '.*config\.load_defaults.*\n'

app_file "config/initializers/new_framework_defaults_6_0.rb", <<-RUBY
Rails.application.config.action_dispatch.return_only_media_type_on_content_type = false
RUBY

app "development"

assert_equal false, ActionDispatch::Response.return_only_media_type_on_content_type
end

test "ActionMailbox.logger is Rails.logger by default" do
app "development"

Expand Down

0 comments on commit 64efe50

Please sign in to comment.