From c62abed8ca480216b3b6386c95935bfc24683216 Mon Sep 17 00:00:00 2001 From: Armand du Plessis Date: Thu, 2 Aug 2012 15:58:45 -0700 Subject: [PATCH] Collapsed dual checks (one for content headers and one for content) into a single check. Rails includes a single character body to a head(:no_content) response to work around an old Safari bug where headers were ignored if no body sent. This patch brings the behavior slightly closer to spec if :no_content/204 is explicity requested via a head only response. Status comparison done on symbolic and numeric values Not returning any content when responding with head and limited to a status code that explicitly states no content will be returned - 100..199, 204, 205, 304. --- .../lib/action_controller/metal/head.rb | 8 ++--- .../test/controller/mime_responds_test.rb | 8 ++--- .../controller/new_base/bare_metal_test.rb | 30 +++++++++++++++++++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index 2fcd933d32d9f..747e1273be5e2 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -29,19 +29,19 @@ def head(status, options = {}) self.status = status self.location = url_for(location) if location - if include_content_headers?(self.status) + if include_content?(self.status) self.content_type = content_type || (Mime[formats.first] if formats) + self.response_body = " " else headers.delete('Content-Type') headers.delete('Content-Length') + self.response_body = "" end - - self.response_body = " " end private # :nodoc: - def include_content_headers?(status) + def include_content?(status) case status when 100..199 false diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index d093e31b3d0ad..6f9dd44e210a1 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -850,7 +850,7 @@ def test_using_resource_for_put_with_xml_yields_no_content_on_success put :using_resource assert_equal "application/xml", @response.content_type assert_equal 204, @response.status - assert_equal " ", @response.body + assert_equal "", @response.body end def test_using_resource_for_put_with_json_yields_no_content_on_success @@ -859,7 +859,7 @@ def test_using_resource_for_put_with_json_yields_no_content_on_success put :using_resource assert_equal "application/json", @response.content_type assert_equal 204, @response.status - assert_equal " ", @response.body + assert_equal "", @response.body end def test_using_resource_for_put_with_xml_yields_unprocessable_entity_on_failure @@ -901,7 +901,7 @@ def test_using_resource_for_delete_with_xml_yields_no_content_on_success delete :using_resource assert_equal "application/xml", @response.content_type assert_equal 204, @response.status - assert_equal " ", @response.body + assert_equal "", @response.body end def test_using_resource_for_delete_with_json_yields_no_content_on_success @@ -911,7 +911,7 @@ def test_using_resource_for_delete_with_json_yields_no_content_on_success delete :using_resource assert_equal "application/json", @response.content_type assert_equal 204, @response.status - assert_equal " ", @response.body + assert_equal "", @response.body end def test_using_resource_for_delete_with_html_redirects_on_failure diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index 5bcd79ebecfc7..7396c850adc37 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -110,6 +110,36 @@ class HeadTest < ActiveSupport::TestCase assert_nil headers['Content-Type'] assert_nil headers['Content-Length'] end + + test "head :no_content (204) does not return any content" do + content = HeadController.action(:no_content).call(Rack::MockRequest.env_for("/")).third.first + assert_empty content + end + + test "head :reset_content (205) does not return any content" do + content = HeadController.action(:reset_content).call(Rack::MockRequest.env_for("/")).third.first + assert_empty content + end + + test "head :not_modified (304) does not return any content" do + content = HeadController.action(:not_modified).call(Rack::MockRequest.env_for("/")).third.first + assert_empty content + end + + test "head :continue (100) does not return any content" do + content = HeadController.action(:continue).call(Rack::MockRequest.env_for("/")).third.first + assert_empty content + end + + test "head :switching_protocols (101) does not return any content" do + content = HeadController.action(:switching_protocols).call(Rack::MockRequest.env_for("/")).third.first + assert_empty content + end + + test "head :processing (102) does not return any content" do + content = HeadController.action(:processing).call(Rack::MockRequest.env_for("/")).third.first + assert_empty content + end end class BareControllerTest < ActionController::TestCase