From a960fc703245df735787a3fcc05eb66701e150ba Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Mon, 29 Aug 2016 17:34:39 -0400 Subject: [PATCH 1/3] Allow `send_file` to declare a charset Removed my patch in favor of @tenderlove's less invasive approach. [Aaron Patterson & Jon Moss] --- actionpack/lib/action_controller/metal/data_streaming.rb | 1 + actionpack/lib/action_dispatch/http/response.rb | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index b598dd4d77cc2..6a1acb64f7974 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -70,6 +70,7 @@ def send_file(path, options = {}) #:doc: send_file_headers! options self.status = options[:status] || 200 + self.content_type = options[:type] if options.key?(:type) self.content_type = options[:content_type] if options.key?(:content_type) response.send_file path end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 9788431943291..c8f0644ef578a 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -224,8 +224,10 @@ def status=(status) # Sets the HTTP content type. def content_type=(content_type) - header_info = parse_content_type - set_content_type content_type.to_s, header_info.charset || self.class.default_charset + return unless content_type + new_header_info = parse_content_type(content_type.to_s) + prev_header_info = parse_content_type(get_header(CONTENT_TYPE)) + set_content_type new_header_info.mime_type, new_header_info.charset || prev_header_info.charset || self.class.default_charset end # Sets the HTTP response's content MIME type. For example, in the controller @@ -403,8 +405,7 @@ def cookies ContentTypeHeader = Struct.new :mime_type, :charset NullContentTypeHeader = ContentTypeHeader.new nil, nil - def parse_content_type - content_type = get_header CONTENT_TYPE + def parse_content_type(content_type = get_header(CONTENT_TYPE)) if content_type type, charset = content_type.split(/;\s*charset=/) type = nil if type.empty? From bed91f842109c715903ebbe3ce9754629b3594fd Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Mon, 29 Aug 2016 17:35:15 -0400 Subject: [PATCH 2/3] Add regression tests --- actionpack/test/controller/send_file_test.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 25420ead3bb5c..8dc565ac8dfe5 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -233,4 +233,18 @@ def test_send_file_with_action_controller_live response = process("file") assert_equal 200, response.status end + + def test_send_file_charset_with_type_options_key + @controller = SendFileWithActionControllerLive.new + @controller.options = { type: "text/calendar; charset=utf-8" } + response = process("file") + assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"] + end + + def test_send_file_charset_with_content_type_options_key + @controller = SendFileWithActionControllerLive.new + @controller.options = { content_type: "text/calendar" } + response = process("file") + assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"] + end end From 92f8b99ff74cee43f56ff336838b3a776e763136 Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Mon, 29 Aug 2016 17:57:40 -0400 Subject: [PATCH 3/3] Remove default argument, and extract internal convenience method --- .../lib/action_dispatch/http/response.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index c8f0644ef578a..e8173e2a9979d 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -226,7 +226,7 @@ def status=(status) def content_type=(content_type) return unless content_type new_header_info = parse_content_type(content_type.to_s) - prev_header_info = parse_content_type(get_header(CONTENT_TYPE)) + prev_header_info = parsed_content_type_header set_content_type new_header_info.mime_type, new_header_info.charset || prev_header_info.charset || self.class.default_charset end @@ -240,7 +240,7 @@ def content_type=(content_type) # information. def content_type - parse_content_type.mime_type + parsed_content_type_header.mime_type end def sending_file=(v) @@ -255,7 +255,7 @@ def sending_file=(v) # response.charset = 'utf-16' # => 'utf-16' # response.charset = nil # => 'utf-8' def charset=(charset) - header_info = parse_content_type + header_info = parsed_content_type_header if false == charset set_header CONTENT_TYPE, header_info.mime_type else @@ -267,7 +267,7 @@ def charset=(charset) # The charset of the response. HTML wants to know the encoding of the # content you're giving them, so we need to send that along. def charset - header_info = parse_content_type + header_info = parsed_content_type_header header_info.charset || self.class.default_charset end @@ -405,7 +405,7 @@ def cookies ContentTypeHeader = Struct.new :mime_type, :charset NullContentTypeHeader = ContentTypeHeader.new nil, nil - def parse_content_type(content_type = get_header(CONTENT_TYPE)) + def parse_content_type(content_type) if content_type type, charset = content_type.split(/;\s*charset=/) type = nil if type.empty? @@ -415,6 +415,12 @@ def parse_content_type(content_type = get_header(CONTENT_TYPE)) end end + # Small internal convenience method to get the parsed version of the current + # content type header. + def parsed_content_type_header + parse_content_type(get_header(CONTENT_TYPE)) + end + def set_content_type(content_type, charset) type = (content_type || "").dup type << "; charset=#{charset}" if charset @@ -451,7 +457,7 @@ def munge_body_object(body) def assign_default_content_type_and_charset! return if content_type - ct = parse_content_type + ct = parsed_content_type_header set_content_type(ct.mime_type || Mime[:html].to_s, ct.charset || self.class.default_charset) end