Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't issue HEAD requests to a GET endpoint #190

Closed
stephencelis opened this issue Jun 26, 2012 · 5 comments
Closed

Can't issue HEAD requests to a GET endpoint #190

stephencelis opened this issue Jun 26, 2012 · 5 comments

Comments

@stephencelis
Copy link
Contributor

I'm not sure if this bug belongs here or upstream with rack-mount (https://github.com/josh/rack-mount), but HTTP spec is pretty clear about responses to HEAD being identical to GET (less the response body):

The metainformation contained in the HTTP headers in response to a HEAD request SHOULD be identical to the information sent in response to a GET request. This method can be used for obtaining metainformation about the entity implied by the request without transferring the entity-body itself. This method is often used for testing hypertext links for validity, accessibility, and recent modification.

http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.4

Below is a basic diff with a spec that fails (not sure if the response should be be nil or an empty string, though).

diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb
index 486b4b6..7541a26 100644
--- a/spec/grape/api_spec.rb
+++ b/spec/grape/api_spec.rb
@@ -296,6 +296,15 @@ describe Grape::API do
       last_response.status.should eql 201
       last_response.body.should eql 'Created'
     end
+
+    it 'should allow a HEAD request against a GET method' do
+      subject.get '/example' do
+        verb
+      end
+      head '/example'
+      last_response.status.should eql 200
+      last_response.body.should be_empty
+    end
   end

   describe 'filters' do

A potential fix (without modifying rack-mount) would be this diff:

diff --git a/lib/grape/api.rb b/lib/grape/api.rb
index 152e6a5..acdc61f 100644
--- a/lib/grape/api.rb
+++ b/lib/grape/api.rb
@@ -293,7 +293,7 @@ module Grape
         imbue(:afters, [block])
       end

-      def get(paths = ['/'], options = {}, &block); route('GET', paths, options, &block) end
+      def get(paths = ['/'], options = {}, &block); route(%w(GET HEAD), paths, options, &block) end
       def post(paths = ['/'], options = {}, &block); route('POST', paths, options, &block) end
       def put(paths = ['/'], options = {}, &block); route('PUT', paths, options, &block) end
       def head(paths = ['/'], options = {}, &block); route('HEAD', paths, options, &block) end

This diff doesn't address the issue raised in #189, but we could fix that, as well. This diff also breaks a lot of specs.

Any thoughts on how/where the logic should be implemented? I've pinged @josh to see if he thinks any of it belongs in rack-mount.

@dblock
Copy link
Member

dblock commented Jun 27, 2012

Lets close this and let everyone comment on #189 which fixes the issue.

@dblock dblock closed this as completed Jun 27, 2012
@stephencelis
Copy link
Contributor Author

Can we reopen this? #189 and #190 are separate issues. #189 fixes the fact that HEAD requests can return a body. #190 brings up the fact that you can't issue a HEAD request to a GET endpoint and get identical responses. E.g.,

# in a grape api
get '/test' do
  'hello world'
end

The GET will return a 200 and the HEAD will return a 404:

127.0.0.1 - - [27/Jun/2012 09:40:49] "GET /test HTTP/1.1" 200 11 0.0089
127.0.0.1 - - [27/Jun/2012 09:40:55] "HEAD /test HTTP/1.1" 404 - 0.0005

This means that issuing curl -I to see the headers of a GET endpoint will not work.

@dblock dblock reopened this Jun 27, 2012
@dblock
Copy link
Member

dblock commented Jun 27, 2012

You're right, reopening.

@dblock
Copy link
Member

dblock commented Dec 16, 2012

I read the spec and I think you're right. If you want to make a proper pull request that has all specs passing (including your new one), will be happy to merge. Don't forget to update CHANGELOG.

dblock added a commit that referenced this issue Feb 10, 2013
@dblock
Copy link
Member

dblock commented Feb 10, 2013

When you add a GET route for a resource, a route for the HEAD method will now also be added automatically. You can disable this with do_not_route_head!, which isn't the most amazing name for a method or option - I am open to changing it if someone can please suggest something better.

@dblock dblock closed this as completed Feb 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants