I've changed my mind a bit... see below.
On Sep 30, 12:03 pm, Jeff <[EMAIL PROTECTED]> wrote:
> > On Sep 30, 3:43 am, "Michael Koziarski" <[EMAIL PROTECTED]> wrote:
> > > 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
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Ruby
on Rails: Core" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---
