Skip to content

Commit

Permalink
Collapsed dual checks (one for content headers and one for content) i…
Browse files Browse the repository at this point in the history
…nto 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.
  • Loading branch information
Armand du Plessis committed Aug 2, 2012
1 parent 6e52376 commit c62abed
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 8 deletions.
8 changes: 4 additions & 4 deletions actionpack/lib/action_controller/metal/head.rb
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions actionpack/test/controller/mime_responds_test.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
30 changes: 30 additions & 0 deletions actionpack/test/controller/new_base/bare_metal_test.rb
Expand Up @@ -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
Expand Down

0 comments on commit c62abed

Please sign in to comment.