From b65bb377fe43b90af68b01e62f0dcd2717eaace4 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 23 Aug 2019 03:34:39 +0900 Subject: [PATCH] Fix `content_type=` to not discard extra part In #36996, the behavior of `content_type=` has been changed to treat a `content_type` itself as a mime type if a `content_type` doesn't contain `charset=...` part. i.e. ```ruby response.content_type = "text/html; fragment" response.media_type # => "text/html; fragment" response.content_type = "text/html; fragment; charset=utf-16" response.media_type # => "text/html" ``` That is tricky and strange. I think that we cannot distinguish whether extra part is a part of mime type or not for now. So at least in Rails 6.0, we should not discard extra part conservatively as before. --- actionpack/lib/action_dispatch/http/response.rb | 15 ++++++++++----- actionpack/test/controller/content_type_test.rb | 11 +++++++++++ actionpack/test/dispatch/response_test.rb | 4 ++-- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 08411f6f8d..cb8cf40d8d 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -82,7 +82,6 @@ module ActionDispatch # :nodoc: SET_COOKIE = "Set-Cookie" LOCATION = "Location" NO_CONTENT_CODES = [100, 101, 102, 204, 205, 304] - CONTENT_TYPE_PARSER = /\A(?[^;\s]+)?\s*(?:;\s*(?.+))?(?:;\s*charset=(?"?)(?[^;\s]+)\k)/ # :nodoc: cattr_accessor :default_charset, default: "utf-8" cattr_accessor :default_headers @@ -419,14 +418,20 @@ module ActionDispatch # :nodoc: end private - ContentTypeHeader = Struct.new :mime_type, :extra, :charset - NullContentTypeHeader = ContentTypeHeader.new nil, nil, nil + ContentTypeHeader = Struct.new :mime_type, :charset + NullContentTypeHeader = ContentTypeHeader.new nil, nil + + CONTENT_TYPE_PARSER = / + \A + (?[^;\s]+\s*(?:;\s*(?:(?!charset)[^;\s])+)*)? + (?:;\s*charset=(?"?)(?[^;\s]+)\k)? + /x # :nodoc: def parse_content_type(content_type) if content_type && match = CONTENT_TYPE_PARSER.match(content_type) - ContentTypeHeader.new(match[:type], match[:extra], match[:charset]) + ContentTypeHeader.new(match[:mime_type], match[:charset]) else - ContentTypeHeader.new(content_type, nil) + NullContentTypeHeader end end diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index fcf767b706..4c2bd8c745 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -50,6 +50,11 @@ class OldContentTypeController < ActionController::Base format.rss { render body: "hello world!", content_type: Mime[:xml] } end end + + def render_content_type_with_charset + response.content_type = "text/html; fragment; charset=utf-16" + render body: "hello world!" + end end class ContentTypeTest < ActionController::TestCase @@ -131,6 +136,12 @@ class ContentTypeTest < ActionController::TestCase assert_equal "utf-8", @response.charset end + def test_content_type_with_charset + get :render_content_type_with_charset + assert_equal "text/html; fragment", @response.media_type + assert_equal "utf-16", @response.charset + end + private def with_default_charset(charset) old_default_charset = ActionDispatch::Response.default_charset diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index ed64d89902..0c9735f917 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -572,7 +572,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest 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("text/csv; header=present", @response.media_type) assert_equal("utf-16", @response.charset) end @@ -590,7 +590,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest 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("text/csv; header=present", @response.media_type) assert_equal("utf-16", @response.charset) end