aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md29
-rw-r--r--actionpack/lib/action_controller/metal/basic_implicit_render.rb2
-rw-r--r--actionpack/lib/action_controller/metal/implicit_render.rb91
-rw-r--r--actionpack/test/controller/mime/respond_to_test.rb93
-rw-r--r--actionpack/test/controller/render_test.rb25
-rw-r--r--actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb1
-rw-r--r--actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb1
-rw-r--r--actionpack/test/fixtures/implicit_render_test/some_action_2.html+zomg.erb1
-rw-r--r--actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb (renamed from actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb)0
9 files changed, 190 insertions, 53 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index da96aef98b..5b762a8f17 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,32 @@
+* Update default rendering policies when the controller action did
+ not explicitly indicate a response.
+
+ For API controllers, the implicit render always renders "204 No Content"
+ and does not account for any templates.
+
+ For other controllers, the following conditions are checked:
+
+ First, if a template exists for the controller action, it is rendered.
+ This template lookup takes into account the action name, locales, format,
+ variant, template handlers, etc. (see +render+ for details).
+
+ Second, if other templates exist for the controller action but is not in
+ the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt>
+ is raised. The list of available templates is assumed to be a complete
+ enumeration of all the possible formats (or variants, etc.); that is,
+ having only HTML and JSON templates indicate that the controller action is
+ not meant to handle XML requests.
+
+ Third, if the current request is an "interactive" browser request (the user
+ navigated here by entering the URL in the address bar, submiting a form,
+ clicking on a link, etc. as opposed to an XHR or non-browser API request),
+ <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error
+ message.
+
+ Finally, it falls back to the same "204 No Content" behavior as API controllers.
+
+ *Godfrey Chan*, *Jon Moss*, *Kasper Timm Hansen*, *Mike Clark*, *Matthew Draper*
+
## Rails 5.0.0.beta3 (February 24, 2016) ##
* Update session to have indifferent access across multiple requests.
diff --git a/actionpack/lib/action_controller/metal/basic_implicit_render.rb b/actionpack/lib/action_controller/metal/basic_implicit_render.rb
index 6c6f8381ff..cef65a362c 100644
--- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb
+++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb
@@ -1,5 +1,5 @@
module ActionController
- module BasicImplicitRender
+ module BasicImplicitRender # :nodoc:
def send_action(method, *args)
super.tap { default_render unless performed? }
end
diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb
index 17fcc2fa02..1f0179a287 100644
--- a/actionpack/lib/action_controller/metal/implicit_render.rb
+++ b/actionpack/lib/action_controller/metal/implicit_render.rb
@@ -1,29 +1,80 @@
+require 'active_support/core_ext/string/strip'
+
module ActionController
+ # Handles implicit rendering for a controller action when it did not
+ # explicitly indicate an appropiate response via methods such as +render+,
+ # +respond_to+, +redirect+ or +head+.
+ #
+ # For API controllers, the implicit render always renders "204 No Content"
+ # and does not account for any templates.
+ #
+ # For other controllers, the following conditions are checked:
+ #
+ # First, if a template exists for the controller action, it is rendered.
+ # This template lookup takes into account the action name, locales, format,
+ # variant, template handlers, etc. (see +render+ for details).
+ #
+ # Second, if other templates exist for the controller action but is not in
+ # the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt>
+ # is raised. The list of available templates is assumed to be a complete
+ # enumeration of all the possible formats (or variants, etc.); that is,
+ # having only HTML and JSON templates indicate that the controller action is
+ # not meant to handle XML requests.
+ #
+ # Third, if the current request is an "interactive" browser request (the user
+ # navigated here by entering the URL in the address bar, submiting a form,
+ # clicking on a link, etc. as opposed to an XHR or non-browser API request),
+ # <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error
+ # message.
+ #
+ # Finally, it falls back to the same "204 No Content" behavior as API controllers.
module ImplicitRender
+ # :stopdoc:
include BasicImplicitRender
- # Renders the template corresponding to the controller action, if it exists.
- # The action name, format, and variant are all taken into account.
- # For example, the "new" action with an HTML format and variant "phone"
- # would try to render the <tt>new.html+phone.erb</tt> template.
- #
- # If no template is found <tt>ActionController::BasicImplicitRender</tt>'s implementation is called, unless
- # a block is passed. In that case, it will override the super implementation.
- #
- # default_render do
- # head 404 # No template was found
- # end
def default_render(*args)
if template_exists?(action_name.to_s, _prefixes, variants: request.variant)
render(*args)
- else
- if block_given?
- yield(*args)
- else
- logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger
- super
+ elsif any_templates?(action_name.to_s, _prefixes)
+ message = "#{self.class.name}\##{action_name} does not know how to repond " \
+ "to this request. There are other templates available for this controller " \
+ "action but none of them were suitable for this request.\n\n" \
+ "This usually happens when the client requested an unsupported format " \
+ "(e.g. requesting HTML content from a JSON endpoint or vice versa), but " \
+ "it might also be failing due to other constraints, such as locales or" \
+ "variants.\n"
+
+ if request.formats.any?
+ message << "\nRequested format(s): #{request.formats.join(", ")}"
end
+
+ if request.variant.any?
+ message << "\nRequested variant(s): #{request.variant.join(", ")}"
+ end
+
+ raise ActionController::UnknownFormat, message
+ elsif interactive_browser_request?
+ message = "You did not define any templates for #{self.class.name}\##{action_name}. " \
+ "This is not necessarily a problem (e.g. you might be building an API endpoint " \
+ "that does not require any templates), and the controller would usually respond " \
+ "with `head :no_content` for your convenience.\n\n" \
+ "However, you appear to have navigated here from an interactive browser request – " \
+ "such as by navigating to this URL directly, clicking on a link or submitting a form. " \
+ "Rendering a `head :no_content` in this case could have resulted in unexpected UI " \
+ "behavior in the browser.\n\n" \
+ "If you expected the `head :no_content` response, you do not need to take any " \
+ "actions – requests coming from an XHR (AJAX) request or other non-browser clients " \
+ "will receive the \"204 No Content\" response as expected.\n\n" \
+ "If you did not expect this behavior, you can resolve this error by adding a " \
+ "template for this controller action (usually `#{action_name}.html.erb`) or " \
+ "otherwise indicate the appropriate response in the action using `render`, " \
+ "`redirect_to`, `head`, etc.\n"
+
+ raise ActionController::UnknownFormat, message
+ else
+ logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger
+ super
end
end
@@ -32,5 +83,11 @@ module ActionController
"default_render"
end
end
+
+ private
+
+ def interactive_browser_request?
+ request.format == Mime[:html] && !request.xhr?
+ end
end
end
diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb
index 76e2d3ff43..d0c7b2e06a 100644
--- a/actionpack/test/controller/mime/respond_to_test.rb
+++ b/actionpack/test/controller/mime/respond_to_test.rb
@@ -160,7 +160,14 @@ class RespondToController < ActionController::Base
end
end
- def variant_with_implicit_rendering
+ def variant_with_implicit_template_rendering
+ # This has exactly one variant template defined in the file system (+mobile.html.erb),
+ # which raises the regular MissingTemplate error for other variants.
+ end
+
+ def variant_without_implicit_template_rendering
+ # This differs from the above in that it does not have any templates defined in the file
+ # system, which triggers the ImplicitRender (204 No Content) behavior.
end
def variant_with_format_and_custom_render
@@ -272,6 +279,8 @@ class RespondToController < ActionController::Base
end
class RespondToControllerTest < ActionController::TestCase
+ NO_CONTENT_WARNING = "No template found for RespondToController#variant_without_implicit_template_rendering, rendering head :no_content"
+
def setup
super
@request.host = "www.example.com"
@@ -616,30 +625,69 @@ class RespondToControllerTest < ActionController::TestCase
end
def test_invalid_variant
+ assert_raises(ActionController::UnknownFormat) do
+ get :variant_with_implicit_template_rendering, params: { v: :invalid }
+ end
+ end
+
+ def test_variant_not_set_regular_unknown_format
+ assert_raises(ActionController::UnknownFormat) do
+ get :variant_with_implicit_template_rendering
+ end
+ end
+
+ def test_variant_with_implicit_template_rendering
+ get :variant_with_implicit_template_rendering, params: { v: :mobile }
+ assert_equal "text/html", @response.content_type
+ assert_equal "mobile", @response.body
+ end
+
+ def test_variant_without_implicit_rendering_from_browser
+ assert_raises(ActionController::UnknownFormat) do
+ get :variant_without_implicit_template_rendering, params: { v: :does_not_matter }
+ end
+ end
+
+ def test_variant_variant_not_set_and_without_implicit_rendering_from_browser
+ assert_raises(ActionController::UnknownFormat) do
+ get :variant_without_implicit_template_rendering
+ end
+ end
+
+ def test_variant_without_implicit_rendering_from_xhr
logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new
old_logger, ActionController::Base.logger = ActionController::Base.logger, logger
- get :variant_with_implicit_rendering, params: { v: :invalid }
+ get :variant_without_implicit_template_rendering, xhr: true, params: { v: :does_not_matter }
assert_response :no_content
- assert_equal 1, logger.logged(:info).select{ |s| s =~ /No template found/ }.size, "Implicit head :no_content not logged"
+
+ assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged"
ensure
ActionController::Base.logger = old_logger
end
- def test_variant_not_set_regular_template_missing
- get :variant_with_implicit_rendering
+ def test_variant_without_implicit_rendering_from_api
+ logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new
+ old_logger, ActionController::Base.logger = ActionController::Base.logger, logger
+
+ get :variant_without_implicit_template_rendering, format: 'json', params: { v: :does_not_matter }
assert_response :no_content
+
+ assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged"
+ ensure
+ ActionController::Base.logger = old_logger
end
- def test_variant_with_implicit_rendering
- get :variant_with_implicit_rendering, params: { v: :implicit }
+ def test_variant_variant_not_set_and_without_implicit_rendering_from_xhr
+ logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new
+ old_logger, ActionController::Base.logger = ActionController::Base.logger, logger
+
+ get :variant_without_implicit_template_rendering, xhr: true
assert_response :no_content
- end
- def test_variant_with_implicit_template_rendering
- get :variant_with_implicit_rendering, params: { v: :mobile }
- assert_equal "text/html", @response.content_type
- assert_equal "mobile", @response.body
+ assert_equal 1, logger.logged(:info).select { |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged"
+ ensure
+ ActionController::Base.logger = old_logger
end
def test_variant_with_format_and_custom_render
@@ -778,24 +826,3 @@ class RespondToControllerTest < ActionController::TestCase
assert_equal "phone", @response.body
end
end
-
-class RespondToWithBlockOnDefaultRenderController < ActionController::Base
- def show
- default_render do
- render body: 'default_render yielded'
- end
- end
-end
-
-class RespondToWithBlockOnDefaultRenderControllerTest < ActionController::TestCase
- def setup
- super
- @request.host = "www.example.com"
- end
-
- def test_default_render_uses_block_when_no_template_exists
- get :show
- assert_equal "default_render yielded", @response.body
- assert_equal "text/plain", @response.content_type
- end
-end
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 60c6518c62..83d7405e4d 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -26,6 +26,9 @@ end
class ImplicitRenderTestController < ActionController::Base
def empty_action
end
+
+ def empty_action_with_template
+ end
end
class TestController < ActionController::Base
@@ -537,10 +540,28 @@ end
class ImplicitRenderTest < ActionController::TestCase
tests ImplicitRenderTestController
- def test_implicit_no_content_response
- get :empty_action
+ def test_implicit_no_content_response_as_browser
+ assert_raises(ActionController::UnknownFormat) do
+ get :empty_action
+ end
+ end
+
+ def test_implicit_no_content_response_as_xhr
+ get :empty_action, xhr: true
assert_response :no_content
end
+
+ def test_implicit_success_response_with_right_format
+ get :empty_action_with_template
+ assert_equal "<h1>Empty action rendered this implicitly.</h1>\n", @response.body
+ assert_response :success
+ end
+
+ def test_implicit_unknown_format_response
+ assert_raises(ActionController::UnknownFormat) do
+ get :empty_action_with_template, format: 'json'
+ end
+ end
end
class HeadRenderTest < ActionController::TestCase
diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb
new file mode 100644
index 0000000000..ded99ba52d
--- /dev/null
+++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb
@@ -0,0 +1 @@
+mobile
diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb
new file mode 100644
index 0000000000..dd294f8cf6
--- /dev/null
+++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb
@@ -0,0 +1 @@
+<h1>Empty action rendered this implicitly.</h1>
diff --git a/actionpack/test/fixtures/implicit_render_test/some_action_2.html+zomg.erb b/actionpack/test/fixtures/implicit_render_test/some_action_2.html+zomg.erb
new file mode 100644
index 0000000000..7d8187321c
--- /dev/null
+++ b/actionpack/test/fixtures/implicit_render_test/some_action_2.html+zomg.erb
@@ -0,0 +1 @@
+zomg
diff --git a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb
index 317801ad30..317801ad30 100644
--- a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb
+++ b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb