A few days ago I posted a patch that adds :only and :except style conditions to route generation for resources. I''ve had a couple of +1s on Trac but someone suggested that the functionality should be discussed here. I''ve just added a new patch that now works with r7416 so I''d like some feedback on whether this goes against the principles of RESTful routing or is it functionality people would like to see included. http://dev.rubyonrails.org/ticket/9460 Andrew White --~--~---------~--~----~------------~-------~--~----~ 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 9/7/07, Andrew White <andyw@pixeltrix.co.uk> wrote:> > > A few days ago I posted a patch that adds :only and :except style > conditions to route generation for resources. I''ve had a couple of > +1s on Trac but someone suggested that the functionality should be > discussed here.I don''t really believe in this kind of security. You implement a destructive controller method like "destroy", and then you want to prevent access to it by cutting of the route? Why not solve it with proper access control, or simply declaring the method private? If the method is implemented in the controller then it must be because someone has to use it. And if not allpeople should be allowed to use it, implement authorization. Imagine having dynamite in your house and then explaining the wife it is safe because you cut off the fuse. --~--~---------~--~----~------------~-------~--~----~ 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 7 Sep 2007, at 14:19, Mislav Marohnić wrote:> I don''t really believe in this kind of security. You implement a > destructive controller method like "destroy", and then you want to > prevent access to it by cutting of the route? Why not solve it with > proper access control, or simply declaring the method private? If > the method is implemented in the controller then it must be because > someone has to use it. And if not all people should be allowed to > use it, implement authorization. > > Imagine having dynamite in your house and then explaining the wife > it is safe because you cut off the fuse.Whilst it''s true that you should remove the destroy action to be entirely safe, it''s not beyond the realms of possibility that in a multi-person team someone might accidentally add a destroy action which shouldn''t be there. If you don''t think it should be applied on security grounds what about efficiency? If your site consists of mainly read-only resources then you''ll have 3-4 times as many routes as you really need. Andrew White --~--~---------~--~----~------------~-------~--~----~ 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 R, 2007-09-07 at 15:19 +0200, Mislav Marohnić wrote:> I don''t really believe in this kind of security. You implement a > destructive controller method like "destroy", and then you want to > prevent access to it by cutting of the route? Why not solve it with > proper access control, or simply declaring the method private? If the > method is implemented in the controller then it must be because > someone has to use it. And if not all people should be allowed to use > it, implement authorization. > > Imagine having dynamite in your house and then explaining the wife it > is safe because you cut off the fuse.I agree that this should not be considered a security feature, but the ability to only generate the routes that you are going to implement does seem somewhat useful. It would reduce the chance of accidentally generating an url that leads nowhere. Yes, it adds a bit of overhead, but only if you actually want to use the feature. --~--~---------~--~----~------------~-------~--~----~ 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 9/7/07, Andrew White <andyw@pixeltrix.co.uk> wrote:> > > If you don''t think it should be applied on security grounds what > about efficiency? If your site consists of mainly read-only resources > then you''ll have 3-4 times as many routes as you really need.So, is route matching that faster after this patch? --~--~---------~--~----~------------~-------~--~----~ 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 am on the middle ground with this patch. It''d be good to remove the
routes you''re not using.
But at the same time, it''d make the complex routing code even more
complex. Plus, the counter argument is, if you have read-only
resources, why can''t you just use named routes like :
map.with_options(:controller => ''whatever'', :conditions
=> {:method
=> :get}) do |foo|
[''all'', ''your'',
''readonly'', ''actions''].each do |act|
foo.send("#{something}_#{act}", "the/path/you/want",
:action => act)
end
end
Untested. Should work I''d like to think.
So yeah, My vote is ±0 :P
On 9/7/07, Andrew White <andyw@pixeltrix.co.uk>
wrote:>
> A few days ago I posted a patch that adds :only and :except style
> conditions to route generation for resources. I''ve had a couple of
> +1s on Trac but someone suggested that the functionality should be
> discussed here.
>
> I''ve just added a new patch that now works with r7416 so
I''d like
> some feedback on whether this goes against the principles of RESTful
> routing or is it functionality people would like to see included.
>
> http://dev.rubyonrails.org/ticket/9460
>
>
> Andrew White
>
> >
>
--
Cheers!
- Pratik
http://m.onkey.org
--~--~---------~--~----~------------~-------~--~----~
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 7 Sep 2007, at 15:36, Pratik wrote:> But at the same time, it''d make the complex routing code even more > complex. Plus, the counter argument is, if you have read-only > resources, why can''t you just use named routes like : > > map.with_options(:controller => ''whatever'', :conditions => {:method > => :get}) do |foo| > [''all'', ''your'', ''readonly'', ''actions''].each do |act| > foo.send("#{something}_#{act}", "the/path/you/want", :action > => act) > end > end > > Untested. Should work I''d like to think.So here''s an excerpt from my application''s routes.rb file with my patch: map.namespace :shop do |shop| ... shop.resources ''products'', :only => [:index, :show], :collection => { :search => :get } do |product| product.resources ''images'', :only => :show do |image| image.resources ''thumbnails'', :only => :show end end ... end I don''t think you can exactly replicate in named routes without basically writing it out longhand e.g: map.with_options :controller => ''shop/products'', :conditions => { :method => :get } do |products| products.shop_search_products ''shop/products/search'', :action => ''search'' products.shop_products ''shop/products'', :action => ''index'' products.shop_product ''shop/products/:id'', :action => ''show'' end map.with_options :controller => ''shop/images'', :conditions => { :method => :get } do |images| products.shop_product_image ''shop/products/:product_id/ images/:id.:format'', :action => ''show'' end map.with_options :controller => ''shop/thumbnails'', :conditions => { :method => :get } do |thumbnails| thumbnails.shop_product_image_thumbnail ''shop/products/:product_id/ images/:image_id/:id.:format'', :action => ''show'' end Doing it this way would wear out my s, h, o & p keys I fear. :-) Andrew White Pixeltrix T: 024 7625 6244 12 Turbine Hall M: 07973 190 907 Electric Wharf F: 024 7625 6266 Coventry CV1 4JB www.pixeltrix.co.uk --~--~---------~--~----~------------~-------~--~----~ 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 7 Sep 2007, at 15:25, Mislav Marohnić wrote:> If you don''t think it should be applied on security grounds what > about efficiency? If your site consists of mainly read-only resources > then you''ll have 3-4 times as many routes as you really need. > > So, is route matching that faster after this patch?Probably not, but it''s not slower. :-) Andrew White --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> If your site consists of mainly read-only resources > then you''ll have 3-4 times as many routes as you really need. > > Andrew WhiteExcellent. Entia non sunt multiplicanda sine necessitate! :) --~--~---------~--~----~------------~-------~--~----~ 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 agree that this should not be considered a security feature, but > the ability to only generate the routes that you are going to implement > does seem somewhat useful. > > It would reduce the chance of accidentally generating an url that leads > nowhere. Yes, it adds a bit of overhead, but only if you actually want > to use the feature.Rails already deals with resource routes that leads to nowhere. We give you a 406 Not Supported back. Which is a lot better than the 404 Not Found that you''d get if this patch went into effect. I''m -1 on this patch. --~--~---------~--~----~------------~-------~--~----~ 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 9 Sep 2007, at 16:47, DHH wrote:> Rails already deals with resource routes that leads to nowhere. We > give you a 406 Not Supported back. Which is a lot better than the 404 > Not Found that you''d get if this patch went into effect.FYI, this appears to be incorrect - without the patch and no action defined you''ll get a not found exception in every case. With the patch applied you''ll at least get at method not allowed for update and delete. Andrew White --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> > Rails already deals with resource routes that leads to nowhere. We > > give you a 406 Not Supported back. Which is a lot better than the 404 > > Not Found that you''d get if this patch went into effect. > > FYI, this appears to be incorrect - without the patch and no action > defined you''ll get a not found exception in every case. With the > patch applied you''ll at least get at method not allowed for update > and delete.Ah, you''re right. This is me wishing that it''d return a 405. It''s on missing respond_to''s that we return a 406 if the requested content type is not available. We should figure out a way to automatically return 405 if the verb doesn''t correspond to anything. This could be as simple as ApplicationController having default methods for all the verbs that return 405, but of course will mostly be overwritten. Or there may be a cleaner way to do it. In any case, I think 405 is what we should be doing and it should be happening automatically. If anyone wants to PDI that, it''d be great. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Andrew White
2007-Sep-16 15:39 UTC
Missing resource action response (was Re: Comments requested on #9460)
On 14 Sep 2007, at 02:59, DHH wrote:> We should figure out a way to automatically > return 405 if the verb doesn''t correspond to anything. > > This could be as simple as ApplicationController having default > methods for all the verbs that return 405, but of course will mostly > be overwritten. Or there may be a cleaner way to do it.Rather than putting it directly in ApplicationController the patch I''ve uploaded in #9569 includes the seven basic actions into ActionController::Base and then subtracts those actions from the default list added to the hidden actions list. The only issue I''ve yet to resolve in the patch is how to collect the full list of overridden actions. The current patch will list the ones implemented in the current subclass, but any overridden in the ancestor chain between ActionController::Base and subclasses parent will be missed out. Since this only affects the accuracy of the error message in development mode I''m not too worried but if someone who''s Ruby fu is a bit stronger than mine can come up with a nice, simple way of getting all the overridden methods from an arbitrary position in the ancestor chain I''ll amend the patch. Andrew White --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---