Make HEAD work / convert to GET once more #9032

Merged
merged 2 commits into from Jan 28, 2013

Conversation

Projects
None yet
2 participants
Contributor

michiels commented Jan 22, 2013

While testing a Rails 4 app in semi-production, I noticed that CSRF protection threw exceptions on HEAD requests by crawler bots.

TL;DR: Shall I make a pull request that adds support for a HEAD request in the csrf protection code? Should we find a way to convert HEAD -> GET just like in Rails <4.

After discovering, I read in the CHANGELOG that ActionDispatch::Head was replaced in favor of Rack::Head by Santiago Pastorino. ActionDispatch::Head converted a HEAD to a GET request like so:

    def call(env)
      if env["REQUEST_METHOD"] == "HEAD"
        env["REQUEST_METHOD"] = "GET"
        env["rack.methodoverride.original_method"] = "HEAD"
        status, headers, _ = @app.call(env)
        [status, headers, []]
      else
        @app.call(env)
      end
    end

However. Rack::Head middleware does not do this HEAD -> GET conversion in https://github.com/rack/rack/blob/master/lib/rack/head.rb.

    if env["REQUEST_METHOD"] == "HEAD"
      body.close if body.respond_to? :close
      [status, headers, []]
    else
      [status, headers, body]
    end

We could easily add a request.head? in valid_request? to support this. But somehow I feel there are more reasons and code that depends on HEAD requests being converted to GETs.

Added a failing test for now.

@spastorino spastorino added a commit that referenced this pull request Jan 28, 2013

@spastorino spastorino Merge pull request #9032 from firmhouse/head-breaks-csrf
Make HEAD work / convert to GET once more
5f5a43e

@spastorino spastorino merged commit 5f5a43e into rails:master Jan 28, 2013

Contributor

michiels commented Jan 28, 2013

Sure thing! Will put them up as new pull requests. I am thinking about abstracting the GET/HEAD check into a request.like_get? or something. However, doing something like request.get? || request.head? is more explicit in most cases, and I don't see HTTP spec change that soon ;)

michiels deleted the firmhouse:head-breaks-csrf branch Jan 28, 2013

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