From ea5f509643d6d9c468a9b26f6c45bd4e40fd67cf Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Wed, 17 Apr 2019 15:37:16 +0900 Subject: Change `ActionDispatch::Response#content_type` returning Content-Type header as it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #35709, `Response#conten_type` returns only MIME type correctly. It is a documented behavior that this method only returns MIME type, so this change seems appropriate. https://github.com/rails/rails/blob/39de7fac0507070e3c5f8b33fbad6fced84d97ed/actionpack/lib/action_dispatch/http/response.rb#L245-L249 But unfortunately, some users expect this method to return all Content-Type that does not contain charset. This seems to be breaking changes. We can change this behavior with the deprecate cycle. But, in that case, a method needs that include Content-Type with additional parameters. And that method name is probably the `content_type` seems to properly. So I changed the new behavior to more appropriate `media_type` method. And `Response#content_type` changed (as the method name) to return Content-Type header as it is. Fixes #35709. [Rafael Mendonça França & Yuuji Yaginuma ] --- actionpack/test/dispatch/debug_exceptions_test.rb | 30 +++++++++---------- actionpack/test/dispatch/response_test.rb | 36 ++++++++++++++++++----- 2 files changed, 43 insertions(+), 23 deletions(-) (limited to 'actionpack/test/dispatch') diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 3e57e8f4d9..68817ccdea 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -208,7 +208,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_response 500 assert_no_match(/

/, body) assert_no_match(//, body) - assert_equal "text/plain", response.content_type + assert_equal "text/plain", response.media_type assert_match(/RuntimeError\npuke/, body) Rails.stub :root, Pathname.new(".") do @@ -222,31 +222,31 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest get "/not_found", headers: xhr_request_env assert_response 404 assert_no_match(//, body) - assert_equal "text/plain", response.content_type + assert_equal "text/plain", response.media_type assert_match(/#{AbstractController::ActionNotFound.name}/, body) get "/method_not_allowed", headers: xhr_request_env assert_response 405 assert_no_match(//, body) - assert_equal "text/plain", response.content_type + assert_equal "text/plain", response.media_type assert_match(/ActionController::MethodNotAllowed/, body) get "/unknown_http_method", headers: xhr_request_env assert_response 405 assert_no_match(//, body) - assert_equal "text/plain", response.content_type + assert_equal "text/plain", response.media_type assert_match(/ActionController::UnknownHttpMethod/, body) get "/bad_request", headers: xhr_request_env assert_response 400 assert_no_match(//, body) - assert_equal "text/plain", response.content_type + assert_equal "text/plain", response.media_type assert_match(/ActionController::BadRequest/, body) get "/parameter_missing", headers: xhr_request_env assert_response 400 assert_no_match(//, body) - assert_equal "text/plain", response.content_type + assert_equal "text/plain", response.media_type assert_match(/ActionController::ParameterMissing/, body) end @@ -257,37 +257,37 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_response 500 assert_no_match(/

/, body) assert_no_match(//, body) - assert_equal "application/json", response.content_type + assert_equal "application/json", response.media_type assert_match(/RuntimeError: puke/, body) get "/not_found", headers: { "action_dispatch.show_exceptions" => true }, as: :json assert_response 404 assert_no_match(//, body) - assert_equal "application/json", response.content_type + assert_equal "application/json", response.media_type assert_match(/#{AbstractController::ActionNotFound.name}/, body) get "/method_not_allowed", headers: { "action_dispatch.show_exceptions" => true }, as: :json assert_response 405 assert_no_match(//, body) - assert_equal "application/json", response.content_type + assert_equal "application/json", response.media_type assert_match(/ActionController::MethodNotAllowed/, body) get "/unknown_http_method", headers: { "action_dispatch.show_exceptions" => true }, as: :json assert_response 405 assert_no_match(//, body) - assert_equal "application/json", response.content_type + assert_equal "application/json", response.media_type assert_match(/ActionController::UnknownHttpMethod/, body) get "/bad_request", headers: { "action_dispatch.show_exceptions" => true }, as: :json assert_response 400 assert_no_match(//, body) - assert_equal "application/json", response.content_type + assert_equal "application/json", response.media_type assert_match(/ActionController::BadRequest/, body) get "/parameter_missing", headers: { "action_dispatch.show_exceptions" => true }, as: :json assert_response 400 assert_no_match(//, body) - assert_equal "application/json", response.content_type + assert_equal "application/json", response.media_type assert_match(/ActionController::ParameterMissing/, body) end @@ -298,7 +298,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_response 500 assert_match(/

/, body) assert_match(//, body) - assert_equal "text/html", response.content_type + assert_equal "text/html", response.media_type assert_match(/puke/, body) end @@ -307,7 +307,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest get "/index.xml", headers: { "action_dispatch.show_exceptions" => true } assert_response 500 - assert_equal "application/xml", response.content_type + assert_equal "application/xml", response.media_type assert_match(/RuntimeError: puke/, body) end @@ -321,7 +321,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest get "/index", headers: { "action_dispatch.show_exceptions" => true }, as: :wibble assert_response 500 - assert_equal "application/json", response.content_type + assert_equal "application/json", response.media_type assert_match(/RuntimeError: puke/, body) ensure diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 7758b0406a..33cf86a081 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -290,8 +290,8 @@ class ResponseTest < ActiveSupport::TestCase resp.to_a assert_equal("utf-16", resp.charset) - assert_equal(Mime[:xml], resp.content_type) - + assert_equal(Mime[:xml], resp.media_type) + assert_equal("application/xml; charset=utf-16", resp.content_type) assert_equal("application/xml; charset=utf-16", resp.headers["Content-Type"]) end @@ -503,8 +503,8 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_response :success assert_equal("utf-16", @response.charset) - assert_equal(Mime[:xml], @response.content_type) - + assert_equal(Mime[:xml], @response.media_type) + assert_equal("application/xml; charset=utf-16", @response.content_type) assert_equal("application/xml; charset=utf-16", @response.headers["Content-Type"]) end @@ -519,8 +519,8 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_response :success assert_equal("utf-16", @response.charset) - assert_equal(Mime[:xml], @response.content_type) - + assert_equal(Mime[:xml], @response.media_type) + assert_equal("application/xml; charset=utf-16", @response.content_type) assert_equal("application/xml; charset=utf-16", @response.headers["Content-Type"]) end @@ -553,7 +553,26 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_response :success assert_equal("text/csv; charset=utf-16; header=present", @response.headers["Content-Type"]) - assert_equal("text/csv", @response.content_type) + assert_equal("text/csv; charset=utf-16; header=present", @response.content_type) + assert_equal("text/csv", @response.media_type) + assert_equal("utf-16", @response.charset) + end + + test "response Content-Type with optional parameters that set before charset" do + @app = lambda { |env| + [ + 200, + { "Content-Type" => "text/csv; header=present; charset=utf-16" }, + ["Hello"] + ] + } + + get "/" + assert_response :success + + assert_equal("text/csv; header=present; charset=utf-16", @response.headers["Content-Type"]) + assert_equal("text/csv; header=present; charset=utf-16", @response.content_type) + assert_equal("text/csv", @response.media_type) assert_equal("utf-16", @response.charset) end @@ -570,7 +589,8 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_response :success assert_equal('text/csv; header=present; charset="utf-16"', @response.headers["Content-Type"]) - assert_equal("text/csv", @response.content_type) + assert_equal('text/csv; header=present; charset="utf-16"', @response.content_type) + assert_equal("text/csv", @response.media_type) assert_equal("utf-16", @response.charset) end end -- cgit v1.2.3