Hi, (I didn''t get any answers over on rails-talk; I''m hoping it''s ok that I escalate it to this list instead.) I have a Rails 2.1.1 web app, and a Rails 2.1.1 app acting as a client by using ActiveResource. From the client, I can find, create, and update resources owned by the web app. However, I can not delete any. Calling the .destroy method in ActiveResource generates a 422 from the web app. Not sure why this would be the case, since I thought protect_from_forgery only protects HTML and JS requests, whereas ActiveResource should be sending XML requests. Any idea if this is a bug in ActiveResource that I should dig into/ submit a patch for, or is this actually by design and I''m not understanding something about how to achieve deletes via ActiveResource? 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Michael Koziarski
2008-Sep-30 08:43 UTC
Re: Bug with ActiveResource/ActionPack/protect_from_forgery
On Mon, Sep 29, 2008 at 4:38 AM, Jeff <cohen.jeff@gmail.com> wrote:> > Hi, > > (I didn''t get any answers over on rails-talk; I''m hoping it''s ok that > I escalate it to this list instead.) > > I have a Rails 2.1.1 web app, and a Rails 2.1.1 app acting as a client > by using ActiveResource. > > From the client, I can find, create, and update resources owned by the > web app. > > However, I can not delete any. Calling the .destroy method in > ActiveResource generates a 422 from the web app. > > Not sure why this would be the case, since I thought > protect_from_forgery only protects HTML and JS requests, whereas > ActiveResource should be sending XML requests. > > Any idea if this is a bug in ActiveResource that I should dig into/ > submit a patch for, or > is this actually by design and I''m not understanding something about > how to achieve deletes via ActiveResource?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]> Thanks! > Jeff > > >-- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Sep 30, 3:43 am, "Michael Koziarski" <mich...@koziarski.com> wrote:> On Mon, Sep 29, 2008 at 4:38 AM, Jeff <cohen.j...@gmail.com> wrote: > > However, I can not delete any. Calling the .destroy method in > > ActiveResource generates a 422 from the web app.> 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]Thanks for that - I''ll try to dig into it this week and see if I can figure out what''s going on. 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> On Sep 30, 3:43 am, "Michael Koziarski" <mich...@koziarski.com> 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. So far my attempts to send both headers are breaking existing tests, so I''m working on updating the tests. 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
I''ve changed my mind a bit... see below. On Sep 30, 12:03 pm, Jeff <cohen.j...@gmail.com> wrote:> > On Sep 30, 3:43 am, "Michael Koziarski" <mich...@koziarski.com> 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Oct 1, 2:13 pm, Jeff <cohen.j...@gmail.com> wrote:> 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....Aaaaand now I see someone beat me to it :-) http://rails.lighthouseapp.com/projects/8994/tickets/1145-bug-invalidauthenticitytoken-incorrectly-raised-for-xml-controllerdestroy-request#ticket-1145-5 I''ll let the people on that thread submit a patch instead. Sorry for the noise. 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Hello again, Looking for +1''s and/or feedback on this long-standing ticket, that I submitted a patch for: http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1145 Executive summary: currently in Rails 2.1, ActiveResource DELETEs won''t work if the server app is using forgery protection. This should now be fixed by virtue of tightening up the rules that we use for forgery proteection (only worry about html/url-encoded content requests). Most of the insight came from Matthjs and DHH confirmed the aproach - the question is whether my implementation works for anyone else besides me :-) 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---