merb

Comments and changes to this ticket

• 

  Michael D. Ivey (ivey) June 7th, 2008 @ 02:30 AM

  Can we make this more generic, like I did with params["_method"] so headers for other platforms don't need to be hardcoded?

  Is there a way to do it to support both? (ie w/ closures?}

• 

  mde June 7th, 2008 @ 07:07 AM

  Michael,

  I may be 'tainted' because X-HTTP-Method-Override is what we used during my time at OSAF for dealing with crappy browsers like Safari2. But (even after some Google querying followup) it would seem that X-HTTP-Method-Override is a de facto naming convention for overriding in the header.

  It does seem a little more likely to me that people are going to be coming up with ad hoc param names than header names to override the request method. (I couldn't find any other naming conventions in searching the Web.) I guess if you wanted to support more you could take the exact same approach you did with the browser_method_workarounds (a bit of misnomer -- our problem child in this case is Flash), but that seems like a lot of overhead for what appears to be a more unlikely scenario.

  By supporting both, what exactly were you thinking of? I was assuming what you'd want would be an order-of-precedence thing where it looks for any params first, then for an X-HTTP-Method-Override, then just falls back to normal POST. (That's what the attached patch does.)

  There can only be a single method picked, right? Maybe I'm misunderstanding your question.

  Thanks.

  Matthew

• 

  Michael D. Ivey (ivey) June 7th, 2008 @ 07:10 AM

  Rails hardcodes the params["_method"} behavior, and we used to as well.

  Facebook uses params["fb_sig_requested_method"], so instead of adding that too, we made it flexible.

  I don't have a problem adding X-HTTP-Method-Override to the default list, like _method, but I'd like to see it be flexible so Framework N that uses X-N-Requested-Method can be easily supported without a change to core.

  What I was kinda thinking was instead of registering a list of params, register a list of blocks that take the controller, and call them in turn until you either find one or none. This way plugins could support any number of "hack them request method before route identification" methods.

  Make sense?

• 

  mde June 9th, 2008 @ 05:53 AM

  Michael,

  Attached is another patch that uses a list of procs that get called in the Request object context.

  I also changed the name of the variable used to store the tests. I'm not totally sold on http_method_overrides, but overriding the method definitely has much broader applicability than just to browsers. Most modern browsers (even IE6, believe it or not) are actually totally capable of arbitrary request methods (via XHR). Safari2 and Flash are the only two user-agents I have experience with that have this limitation. Apparently some firewalls limit requests to GET/POST, which would mean that all clients would have to use this override to participate in REST.

  For some reason the tests need the params method called directly rather than the instance variable. I guess that has something to do with how the request is mocked up? If you think this might be a bug, I could take a stab at fixing it.

  Let me know if this patch works, or if I need to iterate.

  Thanks.

  Matthew

  • 0001-Made-HTTP-method-override-proc-based-and-pluggable.patch 3 KB
• 

  Yehuda Katz (wycats) June 11th, 2008 @ 11:51 PM

  • → State changed from “new” to “resolved”

  (from [e7792452a120cdc582b7a1cd826e47eded459d3d]) Made HTTP method override proc-based and pluggable. [#364]

  Changed the check for specific params to procs that execute

  in the request scope, allowing checks for method overrides to

  be based on params, headers -- or pretty much anything in the

  request. Also changed the name of the array that stores these

  checks from browser_method_workarounds to the more accurate

  and descriptive http_method_overrides.

  Signed-off-by: Michael D. Ivey

  (I changed this to yield the controller to the block, instead of

  using instance_eval, as it seemed cleaner and was significantly

  faster. See benchmark attachment on ticket #364 --ivey)

  [#364 state:resolved]

  http://github.com/wycats/merb-co...

• 

  Michael D. Ivey (ivey) June 11th, 2008 @ 11:54 PM

  Here's the Benchwarmer script I ran to test param lookups vs calling blocks

  • blocks.bench.rb 2.2 KB
• 

  Yehuda Katz (wycats) June 12th, 2008 @ 07:59 AM

  (from [1fd7a9ccc0d4ff11a961038d882d724a5725595b]) Merge branch 'master' of git@github.com:wycats/merb-core

  • 'master' of git@github.com:wycats/merb-core:

  make the default rake task 'specs' so running rake in merb-core just runs the specs

  Rename @status to @_status

  Avoid adding nil values in place of missing keys for Hash#only

  Fix issue with null segments

  Fixed session cookie expires value [#366 state:resolved]

  Made HTTP method override proc-based and pluggable. [#364]

  http://github.com/wycats/merb-co...

• 

  Yehuda Katz (wycats) September 18th, 2008 @ 09:29 PM

  • → Tag changed from “” to “dispatch merb-core patch”

  (from [6155eae38811b4a8676530786fdedf48ba46cdea]) Merge branch 'master' of git@github.com:wycats/merb-core

  • 'master' of git@github.com:wycats/merb-core: (50 commits) add Merb::Rack::Profiler middleware. to use add the following to your config/rack.rb file: update license Missing require require 'stringio' Add support for non-standard template reloading. Plugin authors need to override load_template_io to handle special reloading logic. Add support for IO templates instead of just paths. See PLUGIN_API_CHANGELOG for details. add swiftiplied mongrel rack adapter. Swapped the order of helper and controller in the default_framework Why should capture be private? Merb::Config[:framework] now needs absolute paths to work (breaks BC) Merb::Config[:framework] paths now pickup files using glob */.rb unless specified otherwise Don't automatically use Facebook signature Fixes failing spec. make the default rake task 'specs' so running rake in merb-core just runs the specs Rename @status to @_status Avoid adding nil values in place of missing keys for Hash#only Fix issue with null segments Fixed session cookie expires value [#366 state:resolved] Made HTTP method override proc-based and pluggable. [#364] siiiiiigh. ... http://github.com/wycats/merb-co...