Skip to content
This repository

Make ActionController#head pass rack-link #6148

Merged
merged 2 commits into from almost 2 years ago

3 participants

Adam Hawkins Kunal Shah Jon Leighton
Adam Hawkins

This commit makes head responses pass rack-lint. I came across this bug while implementing HTTP caching in our application. According to rack-lint the content-type header must not be present for 1xx, 204, 205, and 304. I was getting weird 500s in our app because responses were blowing up in rack-lint. This commit fixes that. All the other tests pass.

Jon Leighton jonleighton merged commit 8edd21c into from May 04, 2012
Jon Leighton jonleighton closed this May 04, 2012
Kunal Shah

@twinturbo I commented in #3436 but I'm also wondering why rack-lint has this requirement. From RFC2616:

14.17 Content-Type

The Content-Type entity-header field indicates the media type of the entity-body sent to the recipient or,
in the case of the HEAD method, the media type that would have been sent had the request been a GET.

I'm not sure why rack-lint has this requirement

Kunal Shah

rack-link references RFC3875 which specifies:

6.3.1. Content-Type

The Content-Type response field sets the Internet Media Type [6] of
the entity body.

  Content-Type = "Content-Type:" media-type NL

If an entity body is returned, the script MUST supply a Content-Type
field in the response. If it fails to do so, the server SHOULD NOT
attempt to determine the correct content type. The value SHOULD be
sent unmodified to the client, except for any charset parameter
changes.

If I'm reading this correctly the issue is with rack-lint.

L472 of https://github.com/rack/rack/blob/master/lib/rack/lint.rb
Rack::Utils::STATUS_WITH_NO_ENTITY_BODY

I'm going to open an issue in that project...

Adam Hawkins

@whistlerbrk

If an entity body is returned

That's the key. Those codes should have no body.

Kunal Shah

@twinturbo Aha! I see it now. My reading of "If it fails to do so" was incorrect. Sorry for the noise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

May 03, 2012
Make ActionController#head pass rack-link 8eedd1a
Remove content-length as well 8edd21c
This page is out of date. Refresh to see the latest.
22  actionpack/lib/action_controller/metal/head.rb
@@ -27,8 +27,28 @@ def head(status, options = {})
27 27
 
28 28
       self.status = status
29 29
       self.location = url_for(location) if location
30  
-      self.content_type = Mime[formats.first] if formats
  30
+
  31
+      if include_content_headers?(self.status)
  32
+        self.content_type = Mime[formats.first] if formats
  33
+      else
  34
+        headers.delete('Content-Type')
  35
+        headers.delete('Content-Length')
  36
+      end
  37
+
31 38
       self.response_body = " "
32 39
     end
  40
+
  41
+    private
  42
+    # :nodoc:
  43
+    def include_content_headers?(status)
  44
+      case status
  45
+      when 100..199
  46
+        false
  47
+      when 204, 205, 304
  48
+        false
  49
+      else
  50
+        true
  51
+      end
  52
+    end
33 53
   end
34 54
 end
66  actionpack/test/controller/new_base/bare_metal_test.rb
@@ -37,6 +37,36 @@ class HeadController < ActionController::Metal
37 37
     def index
38 38
       head :not_found
39 39
     end
  40
+
  41
+    def continue
  42
+      self.content_type = "text/html"
  43
+      head 100
  44
+    end
  45
+
  46
+    def switching_protocols
  47
+      self.content_type = "text/html"
  48
+      head 101
  49
+    end
  50
+
  51
+    def processing
  52
+      self.content_type = "text/html"
  53
+      head 102
  54
+    end
  55
+
  56
+    def no_content
  57
+      self.content_type = "text/html"
  58
+      head 204
  59
+    end
  60
+
  61
+    def reset_content
  62
+      self.content_type = "text/html"
  63
+      head 205
  64
+    end
  65
+
  66
+    def not_modified
  67
+      self.content_type = "text/html"
  68
+      head 304
  69
+    end
40 70
   end
41 71
 
42 72
   class HeadTest < ActiveSupport::TestCase
@@ -44,6 +74,42 @@ class HeadTest < ActiveSupport::TestCase
44 74
       status = HeadController.action(:index).call(Rack::MockRequest.env_for("/")).first
45 75
       assert_equal 404, status
46 76
     end
  77
+
  78
+    test "head :continue (100) does not return a content-type header" do
  79
+      headers = HeadController.action(:continue).call(Rack::MockRequest.env_for("/")).second
  80
+      assert_nil headers['Content-Type']
  81
+      assert_nil headers['Content-Length']
  82
+    end
  83
+
  84
+    test "head :continue (101) does not return a content-type header" do
  85
+      headers = HeadController.action(:continue).call(Rack::MockRequest.env_for("/")).second
  86
+      assert_nil headers['Content-Type']
  87
+      assert_nil headers['Content-Length']
  88
+    end
  89
+
  90
+    test "head :processing (102) does not return a content-type header" do
  91
+      headers = HeadController.action(:processing).call(Rack::MockRequest.env_for("/")).second
  92
+      assert_nil headers['Content-Type']
  93
+      assert_nil headers['Content-Length']
  94
+    end
  95
+
  96
+    test "head :no_content (204) does not return a content-type header" do
  97
+      headers = HeadController.action(:no_content).call(Rack::MockRequest.env_for("/")).second
  98
+      assert_nil headers['Content-Type']
  99
+      assert_nil headers['Content-Length']
  100
+    end
  101
+
  102
+    test "head :reset_content (205) does not return a content-type header" do
  103
+      headers = HeadController.action(:reset_content).call(Rack::MockRequest.env_for("/")).second
  104
+      assert_nil headers['Content-Type']
  105
+      assert_nil headers['Content-Length']
  106
+    end
  107
+
  108
+    test "head :not_modified (304) does not return a content-type header" do
  109
+      headers = HeadController.action(:not_modified).call(Rack::MockRequest.env_for("/")).second
  110
+      assert_nil headers['Content-Type']
  111
+      assert_nil headers['Content-Length']
  112
+    end
47 113
   end
48 114
 
49 115
   class BareControllerTest < ActionController::TestCase
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.