I've changed my mind a bit... see below.
> > This does sound like a bug or misconfiguration somewhere along the
> > line. The request verification logic should trigger *everything*
> > that isn't a :get and doesn't have a content-type of one of these:
> > @@unverifiable_types = Set.new [:text, :json, :csv, :xml, :rss,
> > :atom, :yaml]
I see the issue now. ActionController:Base includes
RequestForgeryProtection, which defines this method:
def verifiable_request_format?
request.content_type.nil? || request.content_type.verify_request?
end
That first part (request.content_type.nil?) always returns true for
DELETE requests, because ActiveResource only sends an Accept header:
# connection.rb
HTTP_FORMAT_HEADER_NAMES = { :get => 'Accept',
:put => 'Content-Type',
:post => 'Content-Type',
:delete => 'Accept'
}
I think the Delete request needs to send both headers, since I think
ActiveResource wants to receive the deleted content as well.
I'm now thinking that ActiveResource is doing the right thing: DELETE
requests should not be sending a Content-Type. It's the
RequestForgeryProtection module that needs a tweak. Instead of this:
def verifiable_request_format?
request.content_type.nil? ||
request.content_type.verify_request?
end
it should be
def verifiable_request_format?
(request.content_type.nil? && request.method != :delete) ||
request.content_type.verify_request?
end
I think the only reason the request.content_type.nil? check is there
is to prevent someone from evading verification by omitting the
content type entirely. However, I think that's fine for DELETE
requests. So if the content type is nil, and it's a DELETE request,
then verify_request? will get a chance to do it's thing (currently,
that side of the expression is short-circuited and never runs).
If this sounds good to everyone, I'll submit a patch this weekend. Not
sure if the door is already closed on 2.2, but it would be good get
this slipped in if possible.
Thanks
Jeff
