Request Parameters
Daniel N
I've been thinking about request parameters a bit lately and there are currently some issues that I've identified with the way that they're implemented, parsing schemes aside.
In Rails + Merb + ?? there are custom request objects. That is, they don't use Rack::Request.new to setup their requets objects. Most portable middleware I've seen however does.
1. Each request object parses the parameters resulting in a doubling of effort
2. A middleware author must know which type of request object is being used in order to manipulate the params hash.
For middleware like http://github.com/mutle/rack-uploads which rely on manipulating the request params hash, this could present issues when writing the middleware to be portable. Checks for request object type would potentially fracture effort, and add unnecessary complexity to middlware developers who need to manipulate the params hash.
What I'd like to propose is that we setup the params hash of Rack::Request to be in env['rack.params'] which could then be shared across request object inplementation. Rails, Merb, <insert framework here> can then make use of custom requirements, and prevent double parsing while allowing middleware to inpsect / inject keys and values.
The params hash should be hash like, and support accessing all keys via a string of the key name. (ie HashWithIndifferentAccess or Mash would work too)
Any thoughts on this?
Cheers
Daniel
Luke Matthew Sutton
I can foresee many cases where it would be useful to have middleware
inject or modify values in the params -- for example, injecting locale
as a param -- but as you point out, the differing implementations
across frameworks makes it difficult to build something generic.
On Jul 14, 10:34 am, Daniel N <has....@gmail.com> wrote:
> Hi All,
>
> I've been thinking about request parameters a bit lately and there are
> currently some issues that I've identified with the way that they're
> implemented, parsing schemes aside.
>
> In Rails + Merb + ?? there are custom request objects. That is, they don't
> use Rack::Request.new to setup their requets objects. Most portable
> middleware I've seen however does.
>
> 1. Each request object parses the parameters resulting in a doubling of
> effort
> 2. A middleware author must know which type of request object is being used
> in order to manipulate the params hash.
>
Joshua Peek
> In Rails + Merb + ?? there are custom request objects. That is, they don't
> use Rack::Request.new to setup their requets objects. Most portable
> middleware I've seen however does.
Yes Rails does. Calling request.params in Rails offloads Rack::Request
to parse the params and Rack::Request sticks its cache in
rack.request.form_hash.
> 1. Each request object parses the parameters resulting in a doubling of
> effort
> 2. A middleware author must know which type of request object is being used
> in order to manipulate the params hash.
These are not problems if your custom request object inherits from
Rack::Request.
> What I'd like to propose is that we setup the params hash of Rack::Request
> to be in env['rack.params'] which could then be shared across request object
> inplementation. Rails, Merb, <insert framework here> can then make use of
> custom requirements, and prevent double parsing while allowing middleware to
> inpsect / inject keys and values.
The convention atm is to stuff your params in the rack request cache
"rack.request.form_hash". I agree, we should convert this from a
generic cache to a "spec" similar to rack.session. "rack.params"
sounds good too.
References:
http://github.com/rack/rack-contrib/blob/cfee6eefe8291d0aa3c70a12b3b2cc350ec09beb/lib/rack/contrib/nested_params.rb
http://github.com/rack/rack-contrib/blob/cfee6eefe8291d0aa3c70a12b3b2cc350ec09beb/lib/rack/contrib/post_body_content_type_parser.rb
http://github.com/rack/rack-contrib/blob/cfee6eefe8291d0aa3c70a12b3b2cc350ec09beb/lib/rack/contrib/post_body_content_type_parser.rb
http://github.com/rails/rails/blob/01d92021e69f54def1ec8103b2b99f907dd88ec4/actionpack/lib/action_dispatch/middleware/params_parser.rb
> The params hash should be hash like, and support accessing all keys via a
> string of the key name. (ie HashWithIndifferentAccess or Mash would work
> too)
I don't think we should require any special type of data structure.
Even better, just require that whatever object is in
env['rack.params'] respond to [] and []=.
--
Joshua Peek
Yehuda Katz
Daniel N
Hi Josh,
Thanks for correcting my understanding. I've actually been bitten by this with rails when trying to set the action parameter in rails_warden after an authentication failure.
http://github.com/hassox/rails_warden/blob/90d5ce583a238ff3b4364a4a4835cb17a86d9a7e/lib/rails_warden.rb#L10
When I used the standard request object I could not manipulate the params hash and have rails see it, and after looking at what you have it makes sense why. Rails transfers the params hash over to
env["action_dispatch.request.request_parameters"]Which means that on the way back up the stack the params hash is inaccessible to normal Rack::Request objects.
The reason I suggest that we have a standard interface to them is so that items are all available as strings. This would prevent the guessing of should I use a symbol or a string to get hold of this? type questions. By always allowing for access to keys via string there is potentially more portablitiy I think.
Cheers
Daniel
Daniel N
+1 on your proposal. Question: why not use the same structure we used for session? I think the only added things were key? and delete (not sure though).-- Yehuda
My thinking is that in middleware as a least surprise idea, we should _always_ be able to access keys via the string of the name.
Daniel
Christian Neukirchen
> +1 on your proposal. Question: why not use the same structure we used for
> session? I think the only added things were key? and delete (not sure
> though).
> -- Yehuda
Sounds good to me.
--
Christian Neukirchen <chneuk...@gmail.com> http://chneukirchen.org
Joshua Peek
Neukirchen<chneuk...@gmail.com> wrote:
> Sounds good to me.
Great.
Daniel, do you want to write up the patch? We'll need to update Lint
to enforce the SPEC then update any other middleware appropriately.
I'll take it from there.
--
Joshua Peek
Daniel N
Sure thing.. I'll get a patch sorted shortly. Is the best way to attach it here or do it as a pull request on github?
Daniel
Daniel N
I just need to check with you guys on something. At the moment the two hashes, get and post parameters, are stored in the env seperately. Is this something that should keep happening? I don't think it would be robust if we store the params into rack.params if they're also going to be stored seperately unless there's a dual setup where rack.params is merged with any new params.
For example, http://github.com/rack/rack-contrib/blob/cfee6eefe8291d0aa3c70a12b3b2cc350ec09beb/lib/rack/contrib/post_body_content_type_parser.rb
If there were a merged params hash, the post_body_content_type_parser would need to maintain the rack.request.form_hash key and also the rack.params hash.
So is it safe to boil them both up together and just have rack.params or rack.request.params? Or should we maintain the get and post params seperately, and combine them on first access in rack.request.params. Then for cases like the post_body_content_type_parser, it could update the rack.request.form_hash and then merge those results into the params hash.
Cheers
Daniel
Joshua Peek
> So is it safe to boil them both up together and just have rack.params or
> rack.request.params? Or should we maintain the get and post params
> seperately, and combine them on first access in rack.request.params. Then
> for cases like the post_body_content_type_parser, it could update the
> rack.request.form_hash and then merge those results into the params hash.
I'd say merge them. But I'm curious if there is any opposition.
--
Joshua Peek
Luke Matthew Sutton
from a particular source. Most apps don't care, they just want the
params, so I think merge em.
On Jul 16, 1:56 am, Joshua Peek <j...@joshpeek.com> wrote:
Daniel N
Cheers
Daniel
deepak kannan
But, can we get the raw body anywhere and a public api to parse it. My point being, it should be easy to extend.
Deepak Kannan
Joshua Peek
there and we'll get it in.
http://rack.lighthouseapp.com/projects/22435-rack/tickets/62-request-parameters-spec