aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorPatrick Toomey <ptoomey3@biasedcoin.com>2018-07-26 10:29:57 -0600
committerPatrick Toomey <ptoomey3@biasedcoin.com>2018-07-30 16:33:34 -0600
commit84e8b350a3c3b9dbf4333fb21979688b3eb1f19e (patch)
treea3d89d4186e7f39936b0527c7107c4c970d53192 /actionpack
parent15a72c6c05cfc5250ee04742b4f45463f937d3f7 (diff)
downloadrails-84e8b350a3c3b9dbf4333fb21979688b3eb1f19e.tar.gz
rails-84e8b350a3c3b9dbf4333fb21979688b3eb1f19e.tar.bz2
rails-84e8b350a3c3b9dbf4333fb21979688b3eb1f19e.zip
Raises exception when respond_to called multiple times in incompatible way
Nesting respond_to calls can lead to unexpected behavior, so it should be avoided. Currently, the first respond_to format match sets the content-type for the resulting response. But, if a nested respond_to occurs, it is possible to match on a different format. For example: respond_to do |outer_type| outer_type.js do respond_to do |inner_type| inner_type.html { render body: "HTML" } end end end Browsers will often include */* in their Accept headers. In the above example, such a request would result in the outer_type.js match setting the content- type of the response to text/javascript, while the inner_type.html match will cause the actual response to return "HTML". This change tries to minimize potential breakage by only raising an exception if the nested respond_to calls are in conflict with each other. So, something like the following example would not raise an exception: respond_to do |outer_type| outer_type.js do respond_to do |inner_type| inner_type.js { render body: "JS" } end end end While the above is nested, it doesn't affect the content-type of the response.
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md15
-rw-r--r--actionpack/lib/action_controller/metal/exceptions.rb18
-rw-r--r--actionpack/lib/action_controller/metal/mime_responds.rb3
-rw-r--r--actionpack/test/controller/mime/respond_to_test.rb34
4 files changed, 70 insertions, 0 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index f7e9104627..e3d5c8a622 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 ce9eb209fe..30034be018 100644
--- a/actionpack/lib/action_controller/metal/exceptions.rb
+++ b/actionpack/lib/action_controller/metal/exceptions.rb
@@ -51,6 +51,24 @@ module ActionController
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 2233b93406..2b55b9347c 100644
--- a/actionpack/lib/action_controller/metal/mime_responds.rb
+++ b/actionpack/lib/action_controller/metal/mime_responds.rb
@@ -197,6 +197,9 @@ module ActionController #:nodoc:
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 771eccb29b..1163775d3c 100644
--- a/actionpack/test/controller/mime/respond_to_test.rb
+++ b/actionpack/test/controller/mime/respond_to_test.rb
@@ -102,6 +102,26 @@ class RespondToController < ActionController::Base
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 @@ class RespondToControllerTest < ActionController::TestCase
assert_equal "<p>Hello world!</p>\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"