aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorCarlos Antonio da Silva <carlosantoniodasilva@gmail.com>2013-12-19 16:30:48 -0200
committerCarlos Antonio da Silva <carlosantoniodasilva@gmail.com>2013-12-23 16:16:49 -0200
commit618d5317d3ce3d40a22293149f79ff9843bbdb35 (patch)
tree9daf6bf41adf5bbbd43271cca11ad244a62dd66f
parented334d9a334c48582c2f94cfc48c89ba36242422 (diff)
downloadrails-618d5317d3ce3d40a22293149f79ff9843bbdb35.tar.gz
rails-618d5317d3ce3d40a22293149f79ff9843bbdb35.tar.bz2
rails-618d5317d3ce3d40a22293149f79ff9843bbdb35.zip
Move the null mime type to request.format
TLDR: always return an object that responds to the query methods from request.format, and do not touch Mime::Type[] lookup to avoid bugs. --- Long version: The initial issue was about being able to do checks like request.format.html? for request with an unknown format, where request.format would be nil. This is where the issue came from at first in #7837 and #8085 (merged in cba05887dc3b56a46a9fe2779b6b228880b49622), but the implementation went down the path of adding this to the mime type lookup logic. This unfortunately introduced subtle bugs, for instance in the merged commit a test related to send_file had to be changed to accomodate the introduction of the NullType. Later another bug was found in #13064, related to the content-type being shown as #<Mime::NullType:...> for templates with localized extensions but no format included. This one was fixed in #13133, merged in 43962d6ec50f918c9970bd3cd4b6ee5c7f7426ed. Besides that, custom handlers were not receiving the proper template formats anymore when passing through the rendering process, because of the NullType addition. That was found while migrating an application from 3.2 to 4.0 that uses the Markerb gem (a custom handler that generates both text and html emails from a markdown template). --- This changes the implementation moving away from returning this null object from the mime lookup, and still fixes the initial issue where request.format.zomg? would raise an exception for unknown formats due to request.format being nil.
-rw-r--r--actionpack/CHANGELOG.md9
-rw-r--r--actionpack/lib/abstract_controller/rendering.rb2
-rw-r--r--actionpack/lib/action_controller/metal/rendering.rb3
-rw-r--r--actionpack/lib/action_dispatch/http/mime_negotiation.rb2
-rw-r--r--actionpack/lib/action_dispatch/http/mime_type.rb2
-rw-r--r--actionpack/test/controller/send_file_test.rb2
6 files changed, 10 insertions, 10 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index d696656521..30b643c791 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -82,14 +82,15 @@
*Łukasz Strzałkowski*
-* Fix header `Content-Type: #<Mime::NullType:...>` in localized template.
+* Fix render of localized templates without an explicit format using wrong
+ content header and not passing correct formats to template due to the
+ introduction of the `NullType` for mimes.
- When localized template has no format in the template name,
- the response now has the default and correct `content-type`.
+ Templates like `hello.it.erb` were subject to this issue.
Fixes #13064.
- *Angelo Capilleri*
+ *Angelo Capilleri*, *Carlos Antonio da Silva*
* Try to escape each part of a url correctly when using a redirect route.
diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb
index 6f17e3fcd9..7be61d94c9 100644
--- a/actionpack/lib/abstract_controller/rendering.rb
+++ b/actionpack/lib/abstract_controller/rendering.rb
@@ -22,7 +22,7 @@ module AbstractController
def render(*args, &block)
options = _normalize_render(*args, &block)
self.response_body = render_to_body(options)
- _process_format(rendered_format)
+ _process_format(rendered_format) if rendered_format
self.response_body
end
diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb
index 66d34f3b67..5c48b4ab98 100644
--- a/actionpack/lib/action_controller/metal/rendering.rb
+++ b/actionpack/lib/action_controller/metal/rendering.rb
@@ -34,8 +34,7 @@ module ActionController
def _process_format(format)
super
- # format is a Mime::NullType instance here then this condition can't be changed to `if format`
- self.content_type ||= format.to_s unless format.nil?
+ self.content_type ||= format.to_s
end
# Normalize arguments by catching blocks and setting them on :update.
diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb
index 346598b6de..c33ba201e1 100644
--- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb
+++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb
@@ -50,7 +50,7 @@ module ActionDispatch
# GET /posts/5 | request.format => Mime::HTML or MIME::JS, or request.accepts.first
#
def format(view_path = [])
- formats.first
+ formats.first || Mime::NullType.instance
end
def formats
diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb
index 2a8ff0a5d2..3d2dd2d632 100644
--- a/actionpack/lib/action_dispatch/http/mime_type.rb
+++ b/actionpack/lib/action_dispatch/http/mime_type.rb
@@ -28,7 +28,7 @@ module Mime
class << self
def [](type)
return type if type.is_a?(Type)
- Type.lookup_by_extension(type) || NullType.instance
+ Type.lookup_by_extension(type)
end
def fetch(type)
diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb
index 8ecc1c7d73..a4c84c29d7 100644
--- a/actionpack/test/controller/send_file_test.rb
+++ b/actionpack/test/controller/send_file_test.rb
@@ -144,7 +144,7 @@ class SendFileTest < ActionController::TestCase
}
@controller.headers = {}
- assert !@controller.send(:send_file_headers!, options)
+ assert_raise(ArgumentError) { @controller.send(:send_file_headers!, options) }
end
def test_send_file_headers_guess_type_from_extension