HTTP status code "405 Method Not Allowed" (http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.6) is used when a client tries to use a verb on a resource and it is not supported. Along with the 405, the client should receive a list of support verbs in the "Allow" header of the response. If the controller only has "show" and "update" actions and someone tries to call destroy with the "DELETE" verb then they should receive a 405 with "Allow: GET, PUT" header. I''ve be working on a patch (http://pastie.caboo.se/31676) and it seems to work fine. I just don''t like the implementation of it. Seems like it could be done much cleaner. Could someone clean up my patch a little or is this whole "Method Not Allowed" thing a bad idea? I''ve be workin http://pastie.caboo.se/31676 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
HTTP status code "405 Method Not Allowed" (http://www.w3.org/ Protocols/rfc2616/rfc2616-sec10.html#sec10.4.6) is used when a client tries to use a verb on a resource and it is not supported. Along with the 405, the client should receive a list of support verbs in the "Allow" header of the response. If the controller only has "show" and "update" actions and someone tries to call destroy with the "DELETE" verb then they should receive a 405 with "Allow: GET, PUT" header. I''ve be working on a patch (http://pastie.caboo.se/31676) and it seems to work fine. I just don''t like the implementation of it. Seems like it could be done much cleaner. Could someone clean up my patch a little or is this whole "Method Not Allowed" thing a bad idea? -- Josh Peek --~--~---------~--~----~------------~-------~--~----~ 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 1/7/07, Josh Peek <josh@joshpeek.com> wrote:> > > Could someone clean up my patch a little or is this whole "Method Not > Allowed" thing a bad idea?I think it''s a good idea, actually. The patch needs slight reformatting, though. If you''ve gotten this far, why put it on someone else''s back when you can simply finish it up? As Core guys would say - "be a hero, implement it!" :) --~--~---------~--~----~------------~-------~--~----~ 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 1/7/07, Josh Peek <josh@joshpeek.com> wrote:> > HTTP status code "405 Method Not Allowed" (http://www.w3.org/ > Protocols/rfc2616/rfc2616-sec10.html#sec10.4.6) is used when a client > tries to use a verb on a resource and it is not supported. Along with > the 405, the client should receive a list of support verbs in the > "Allow" header of the response. If the controller only has "show" and > "update" actions and someone tries to call destroy with the "DELETE" > verb then they should receive a 405 with "Allow: GET, PUT" header. > > I''ve be working on a patch (http://pastie.caboo.se/31676) and it > seems to work fine. I just don''t like the implementation of it. Seems > like it could be done much cleaner.+1, it goes with the use of 406 Not Acceptable too. -- Rick Olson http://weblog.techno-weenie.net http://mephistoblog.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Mislav Marohni wrote:> I think it''s a good idea, actually. The patch needs slight reformatting, > though. If you''ve gotten this far, why put it on someone else''s back when > you can simply finish it up?Its not what you think, I haven''t given up. Just looking for some pointers in the right direction. Rick Olson wrote:> +1, it goes with the use of 406 Not Acceptable too.406''s are already in there. $ curl -I http://localhost:3000/posts.technoweenie HTTP/1.1 406 Not Acceptable I was thinking to replace all "ActionController::UnknownAction" with "501 Not Implemented" errors. I think i could make the non-restful goers mad. It could just be restricted to restful controllers. Another issue I had was determining if a controller is a resource or not. With that patch, I just check if there is a named route for the controller following resource conventions. Its not that full prove. Maybe I need to write a patch for this first. "PostsController.resource?" would be useful. Still need to investigate if there is a nice way to grab the map routes from a controller. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> 406''s are already in there.I know, that was my point.> I was thinking to replace all "ActionController::UnknownAction" with > "501 Not Implemented" errors. I think i could make the non-restful > goers mad. It could just be restricted to restful controllers.What about 404 like it is currently?> Another issue I had was determining if a controller is a resource or > not. With that patch, I just check if there is a named route for the > controller following resource conventions. Its not that full prove. > Maybe I need to write a patch for this first. > "PostsController.resource?" would be useful. Still need to investigate > if there is a nice way to grab the map routes from a controller.I don''t think that''s necessary. It should just be a special exception routing raises when a request doesn''t match the correct request method condition. -- Rick Olson http://weblog.techno-weenie.net http://mephistoblog.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Rick Olson wrote:> > I was thinking to replace all "ActionController::UnknownAction" with > > "501 Not Implemented" errors. I think i could make the non-restful > > goers mad. It could just be restricted to restful controllers. > > What about 404 like it is currently?It would go allow more with an unknown aspect like "GET /posts/1;unknown".> > Another issue I had was determining if a controller is a resource or > > not. With that patch, I just check if there is a named route for the > > controller following resource conventions. Its not that full prove. > > Maybe I need to write a patch for this first. > > "PostsController.resource?" would be useful. Still need to investigate > > if there is a nice way to grab the map routes from a controller. > > I don''t think that''s necessary. It should just be a special exception > routing raises when a request doesn''t match the correct request method > condition.Thats why I didn''t really like my current implementation either. I have to dig into routes more. Another problem is map.resource creates routes to all the actions whether you had them or not. Thats why I hijacked the UnknownAction call and checked if it was a restful controller. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Working on a much cleaner patch (http://pastie.caboo.se/31798). Still not done, I just wanted to post my work before I feel asleep. I''ve modified the routing lib instead of the rescue lib. Works much nicer. I''ve added some unit tests that still aren''t passing, this time its the allowed methods. One question Rick, could I make (I mean let me) map.resource only map to actions that exist? So if there were no destroy action, that route would never exist. That way it would call a routing error and I''d be able to pick it up and determine if that were other http verbs that should be used instead. More work tomorrow, :) --~--~---------~--~----~------------~-------~--~----~ 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 1/7/07, Josh Peek <josh@joshpeek.com> wrote:> > Working on a much cleaner patch (http://pastie.caboo.se/31798). Still > not done, I just wanted to post my work before I feel asleep. > > I''ve modified the routing lib instead of the rescue lib. Works much > nicer. I''ve added some unit tests that still aren''t passing, this time > its the allowed methods. > > One question Rick, could I make (I mean let me) map.resource only map > to actions that exist? So if there were no destroy action, that route > would never exist. That way it would call a routing error and I''d be > able to pick it up and determine if that were other http verbs that > should be used instead. > > More work tomorrow, :)I don''t think that''s necessary. UnknownAction should map to a 404 which should be sufficient I think. -- Rick Olson http://weblog.techno-weenie.net http://mephistoblog.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Latest: http://pastie.caboo.se/31975 Just need to add the allow headers into the rescue actions. More references * http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.1 * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.7 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Okay, Rick, ticket was created. http://dev.rubyonrails.org/ticket/6953 The code is nice and clean but I still have a tiny complaint about the allow headers. I don''t really think "PUT, DELETE" should be returned if the resource is a list/collection. This is one of those debatable things. The patch is the core functionally and we can allows make it more proper later. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
You could refactor the "allows" portion a bit:
allows = []
%w{get post put delete}.each do |verb|
allows << verb.upcase if routes.find { |r| r.recognize(path, { :method
=>
verb.to_sym }) }
end
regards,
e.
On 1/8/07, Josh Peek <josh@joshpeek.com> wrote:>
>
> Okay, Rick, ticket was created.
>
> http://dev.rubyonrails.org/ticket/6953
>
> The code is nice and clean but I still have a tiny complaint about the
> allow headers. I don''t really think "PUT, DELETE" should
be returned if
> the resource is a list/collection. This is one of those debatable
> things.
>
> The patch is the core functionally and we can allows make it more
> proper later.
>
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
Awesome! If you wanted you could make the change and reattach the patch in the ticket. That way you''d get official credit. Also line #15 should read "assert_equal "GET, POST", e.message" instead of "assert_equal "GET, POST, PUT, DELETE", e.message". The only problem is that the test fails. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
done -- new .diff attached to http://dev.rubyonrails.org/ticket/6953 On 1/11/07, Josh Peek <josh@joshpeek.com> wrote:> > Awesome! If you wanted you could make the change and reattach the patch > in the ticket. That way you''d get official credit. > > Also line #15 should read "assert_equal "GET, POST", e.message" instead > of "assert_equal "GET, POST, PUT, DELETE", e.message". The only problem > is that the test fails. > > > > >--~--~---------~--~----~------------~-------~--~----~ 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 be workin http://pastie.caboo.se/31676Why don''t you just move this to a plugin for now? I''m all for supporting HTTP and all that. I''m just afraid this wouldn''t get used much in Rails, which begs the question: Why include it? At the very least, this will give you freedom to make modifications at your leisure. A series of patches through pastie or trac does not make a very good SCM substitute. Believe me, I know :) http://dev.rubyonrails.org/ticket/1758 Though sometimes it takes a bit to drill that notion into my head... http://dev.rubyonrails.org/ticket/1974 -- Rick Olson http://weblog.techno-weenie.net http://mephistoblog.com --~--~---------~--~----~------------~-------~--~----~ 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 think your right. It might have some undesirable affects if you don''t really care about resources or http. I still think its a "core" feature but I think I''m overlooking more situations I haven''t tested yet. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---