diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index f7e91046279ea7e5245af372a9c5d89bb91f11b7..e3d5c8a6221bd8fad230481a63520904495eb34f 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,18 @@ +*. Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations. + + `respond_to` can match multiple types and lead to undefined behavior when + multiple invocations are made and the types do not match: + + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.html { render body: "HTML" } + end + end + end + + *Patrick Toomey* + * `ActionDispatch::Http::UploadedFile` now delegates `to_path` to its tempfile. This allows uploaded file objects to be passed directly to `File.read` diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index ce9eb209fe6cbb3e54394bf87c93d9e19c0a303f..30034be018cb9c9cb81afb08e9bcb4373223af4b 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -51,6 +51,24 @@ class UnknownHttpMethod < ActionControllerError #:nodoc: class UnknownFormat < ActionControllerError #:nodoc: end + # Raised when a nested respond_to is triggered and the content types of each + # are incompatible. For exampe: + # + # respond_to do |outer_type| + # outer_type.js do + # respond_to do |inner_type| + # inner_type.html { render body: "HTML" } + # end + # end + # end + class RespondToMismatchError < ActionControllerError + DEFAULT_MESSAGE = "respond_to was called multiple times and matched with conflicting formats in this action. Please note that you may only call respond_to and match on a single format per action." + + def initialize(message = nil) + super(message || DEFAULT_MESSAGE) + end + end + class MissingExactTemplate < UnknownFormat #:nodoc: end end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 2233b93406887a0afee3c32c46f46008a9000358..2b55b9347ce62b6be96142489faf7577604f1bdf 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -197,6 +197,9 @@ def respond_to(*mimes) yield collector if block_given? if format = collector.negotiate_format(request) + if content_type && content_type != format + raise ActionController::RespondToMismatchError + end _process_format(format) _set_rendered_content_type format response = collector.response diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 771eccb29b13aa279dd1a35b1a07aefdc97d2f5e..1163775d3c5c8914d9754e242ded559e13c50c2d 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -102,6 +102,26 @@ def made_for_content_type end end + def using_conflicting_nested_js_then_html + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.html { render body: "HTML" } + end + end + end + end + + def using_non_conflicting_nested_js_then_js + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.js { render body: "JS" } + end + end + end + end + def custom_type_handling respond_to do |type| type.html { render body: "HTML" } @@ -430,6 +450,20 @@ def test_using_defaults_with_type_list assert_equal "

Hello world!

\n", @response.body end + def test_using_conflicting_nested_js_then_html + @request.accept = "*/*" + assert_raises(ActionController::RespondToMismatchError) do + get :using_conflicting_nested_js_then_html + end + end + + def test_using_non_conflicting_nested_js_then_js + @request.accept = "*/*" + get :using_non_conflicting_nested_js_then_js + assert_equal "text/javascript", @response.content_type + assert_equal "JS", @response.body + end + def test_with_atom_content_type @request.accept = "" @request.env["CONTENT_TYPE"] = "application/atom+xml"