[Proposal] Support post[title]=Hello params parsing

[Josh Peek]

I'd like to propose that Rack::Requset#params support the nested hash
convention.

In Rails, a query string "post[title]=Hello" will be parsed as
{ "post" => { "title" => "Hello" } }. Rack::Request will parse it as
{ "post[title]" => "Hello" }.

AFAIK, Merb follows the same convention as well, and I'm really
curious what other frameworks are doing the same. If this the defacto
params parsing style I think Rack should support it too.

[Yehuda Katz]

+1

-- Yehuda

--
Yehuda Katz
Developer | Engine Yard
(ph) 718.877.1325

[Michael Klishin]

On 28.12.2008, at 3:24, Josh Peek wrote:

> AFAIK, Merb follows the same convention as well

It does, and +1 for this idea from me.

MK

[Mark Bates]

+1 here as well. Mack does the same thing too. I was thinking today that this should be put into Rack as well I say let's pick the best implementation of it and put it into Rack. 

Should we also merge in date/time select parameter parsing too?

Also what do people think about string/symbol retrieval of params from the request parameters?

--------

Mark Bates

[James Britt]

Josh Peek wrote:
> I'd like to propose that Rack::Requset#params support the nested hash
> convention.
>
> In Rails, a query string "post[title]=Hello" will be parsed as
> { "post" => { "title" => "Hello" } }. Rack::Request will parse it as
> { "post[title]" => "Hello" }.

Why shouldn't this be application-level behavior, or at least a Rack
plugin, rather than default, built-in behavior?

While this happens to be common behavior in most Ruby Web frameworks (I
believe Ramaze follows this as well) I'm leery of libs that make certain
assumptions about data.

--
James Britt

www.happycamperstudios.com - Wicked Cool Coding
www.jamesbritt.com - Playing with Better Toys
www.ruby-doc.org - Ruby Help & Documentation
www.rubystuff.com - The Ruby Store for Ruby Stuff

[Scytrin dai Kinthra]

I'm not a huge fan of adding more and more features to Rack::Request.
The setting up of hashes in the params makes sense but adds the
complication of a param value being a String, an Array, or a Hash. I'd
vote yea due to the convenience it provides. With an 'eh' on parsing
issues.

Parameters retrievable via symbols and strings is a right out no to
me. The issue of identity vs content. Also, how would you deal with
environments running with $SAFE > 0? Or are you talking about
stringifying all symbol key lookups, which creates additional overhead
as well as defeating the general point of symbol keyed hashes.

date/time parsing?

--
stadik.net

[Michael Fellinger]

On Sun, Dec 28, 2008 at 11:53 AM, James Britt <james...@gmail.com> wrote:
>
> Josh Peek wrote:
>>
>> I'd like to propose that Rack::Requset#params support the nested hash
>> convention.
>>
>> In Rails, a query string "post[title]=Hello" will be parsed as
>> { "post" => { "title" => "Hello" } }. Rack::Request will parse it as
>> { "post[title]" => "Hello" }.
>
>
> Why shouldn't this be application-level behavior, or at least a Rack plugin,
> rather than default, built-in behavior?
>
> While this happens to be common behavior in most Ruby Web frameworks (I
> believe Ramaze follows this as well) I'm leery of libs that make certain
> assumptions about data.

Indeed, there are issues with parsing that might not reach a consent.
(Ramaze/Innate) are actually reparsing Request#params with less than
stellar results in performance and a few divergences from the normal
(say PHP) behaviour.
I think it would be good to take a look at the existing
implementations, line them up and see which one is the fittest to make
a suitable middleware anybody can use.

The candidate I send into battle:
http://paste.linuxhelp.tv/pastes/view/15309

Due to its dependence on Rack::Request#params it might be less
performant than other solutions.

^ manveru

[Yehuda Katz]

The problem with a middleware is that it cannot be lazy-loaded without hacks. I think this belongs in Rack::Request. We can call it robust_params or something  to distinguish it from the simpler version.

-- Yehuda

[Michael Fellinger]

On Sun, Dec 28, 2008 at 12:47 PM, Yehuda Katz <wyc...@gmail.com> wrote:
> The problem with a middleware is that it cannot be lazy-loaded without
> hacks. I think this belongs in Rack::Request. We can call it robust_params
> or something to distinguish it from the simpler version.

In rack core or contrib?

[Jon Crosby]

One of the traits that makes Rack so powerful is its minimalist simplicity. While having parameter parsing based on a hash convention might be valuable to some frameworks, it is extraneous for others. My vote would be to keep this in middleware for those who need it. If the parameter parsing technique is implemented in a module and then hooked into to a minimal call method in a piece of rack-contrib middleware, then authors of other middleware can choose to either note its upstream requirement in their documentation or, more ideally, apply the mixin in their own middleware.

Further, there seems to be some concern about when things are loaded and processed upstream and how to vary behavior based on this downstream. In the parameter parsing scenario, MiddlewareB may question if ParamParsingMiddlewareA has been executed upstream, for example. HTTP solved this problem with the Via header and it may be helpful in thinking about similar solutions for Rack:

http://tools.ietf.org/html/rfc2616#section-14.45

The above linked Via header shouldn't be used to declare middleware inside of Rack because it is intended to document things happening over the wire whereas Rack is of course self-contained. However, the specific question of "what is upstream" came up in CloudKit when the OpenID Filter needed to know whether to also send OAuth Discovery challenge headers along with its login page. A stack-scoped "cloudkit.via" entry was added to the environment and behaves in a similar manner to Via headers in HTTP.

In the case of Rack, each piece of middleware could drop a "pseudo via" entry into the environment, perhaps based on a canonical version of its gem name or main class name.

-Jon

http://joncrosby.me

[Matt Todd]

I've created several tickets regarding the discussions in this thread since I think separating the requests will help further discussion.

http://rack.lighthouseapp.com/projects/22435-rack/tickets/3-add-support-for-nested-params-parsing-in-rackrequest

http://rack.lighthouseapp.com/projects/22435-rack/tickets/4-stringsymbol-lookup-for-params-hash

http://rack.lighthouseapp.com/projects/22435-rack/tickets/5-datetime-params-parsing

I think the specific issue of interest in this thread, though, is adding support for nested params parsing.

I'm in favor of it, though I don't have the evidence from any specific RFC mentioning this support for nesting (please provide if there exists).

I like to think of Rack as a culmination of experience and expertise for web development practices and implementations in the Ruby community... a bit lofty for the reality of it, but it's still a goal I have in my approach to Rack's implementation. Because of this, I think that Rack should be able to handle anything web standards provide for... including nested params which occurs frequently.

Making this behavior a part of the middleware may prove difficult to implement in Rack::Request since it must first discern if there is the middleware available and secondly it must then parse it. There are lots of problems, like whether to be specific or support all kinds of hooks for other middleware to change how request params are parsed (which I'm against).

Let's keep it simple by parsing nested params. If anything, we can see what the performance difference is... keeping in mind that it only occurs once now that we memoize the Rack::Request object.

Matt

--
Matt Todd
Highgroove Studios
www.highgroove.com
cell: 404-314-2612
blog: maraby.org

Scout - Web Monitoring and Reporting Software
www.scoutapp.com

[Scytrin dai Kinthra]

# What was accidentally posted in the wrong thread.
Honestly, the idea of a rack-contrib mixin with a #robust_params makes the most
sense to me. Memoizing the superduper params hash in a different
location in the env for later retrieval similar to the way params
does.

--
stadik.net

[Christian Neukirchen]

Josh Peek <jo...@joshpeek.com> writes:

The feature seems sensible, but I would not want it to be the default
behavior for #params. I'm fine with having a seperate method for it,
or a method in Utils. And please specify exactly how it works! There
are so many details there to take care of.

--
Christian Neukirchen <chneuk...@gmail.com> http://chneukirchen.org

[Christian Neukirchen]

"Scytrin dai Kinthra" <scy...@gmail.com> writes:

> I'm not a huge fan of adding more and more features to Rack::Request.
> The setting up of hashes in the params makes sense but adds the
> complication of a param value being a String, an Array, or a Hash. I'd
> vote yea due to the convenience it provides. With an 'eh' on parsing
> issues.
>
> Parameters retrievable via symbols and strings is a right out no to
> me. The issue of identity vs content.

+1

Also, I wonder how often people need *nested* hashes...

[Joshua Peek]

I knew the issue would be controversial, and I agree it may not be
best to include into Rack since it is a convention not part of any
RFC.

I think we had issues trying to override the default behavior without
having to do alot of work. Maybe this issue can be solved by a simple
refactoring?

[Ezra Zygmuntowicz]

What about sidestepping the issue by making Rack::Request call a
callback to do params parsing with the default callback being the
current implementation? Then any framework that wants its own parsing
can provide its own callback for params parsing?

This keeps it simple, does not break backwardas compat and solves all
of our issues with regards to how we want to do params parsing.

Cheers-

Ezra Zygmuntowicz
e...@engineyard.com

[Yehuda Katz]

Depending on how the callback is specified, it could be slower (for instance, if a proc was specified).

-- Yehuda

[James Tucker]

On 28 Dec 2008, at 15:46, Yehuda Katz wrote:

> Depending on how the callback is specified, it could be slower (for
> instance, if a proc was specified).

The fastest callback form is using #send. Unfortunately that requires
two objects to specify a callback. I've been considering wrapping up a
gem for this, as I'm starting to use callbacks more and more.

Some examples of what kind of API you end up with using either can be
found in:

http://github.com/raggi/thin/tree/async_for_rack/example # all proc
based stuff

and

http://github.com/raggi/object_protocol/ # All send based.

All of the above is somewhat experimental (particularly wrt api).
Another place that may have structures of interest, might be omnibus:

http://github.com/mental/concurrent/tree

Enjoy.

>
> -- Yehuda

[Mark Bates]

Ezra, I think the original idea was that if nearly every framework is
doing the same thing, shouldn't that be default. Callbacks would have
the same problem, every framework writing the same implementation of
the same thing. With that said, I completely understand the other side
of the argument.

Here are my suggestions:

1.) A config parameter that let's the Rack app decide which type of
parameter 'processing' it would like to use.
2.) An 'extended_params' method that returns the extra processed
parameters.
3.) A piece of Rack middleware in Rack-contrib that does the pre-
processing of the parameters, so it's only used if people put it in
line.

#3 is my favorite. It allows people to pick and chose which
implementation of parameter processing they want. If you use this
middleware then the params method returns a nested Hash that Rails,
Merb, and Mack users are already familiar with. If you don't use the
middleware, then you get back a straight, non-nested Hash.

#2 I'm not that big a fan of because, as a framework guy, I would know
to use that method, but people extending the framework wouldn't know
that, so it's possible for them to use the wrong method and cause
bugs. This method would most likely end up being aliased to params by
the frameworks to avoid this confusion.

#1 is alright, but a bit hacky. It would work, but if you want to add
a different type of processing then you would have to alter the
original Rack source, and that's uncool.

What do people think about #3? I think it solves all the problems.

-------------------------------------------------------------------------------------------------
Mark Bates
ma...@mackframework.com
http://www.mackframework.com
http://api.mackframework.com/
http://github.com/markbates/mack

On Dec 28, 2008, at 2:44 PM, Ezra Zygmuntowicz wrote:\

[Jon Crosby]

On Sun, Dec 28, 2008 at 2:43 PM, Mark Bates <ma...@mackframework.com> wrote:

Ezra, I think the original idea was that if nearly every framework is doing the same thing, shouldn't that be default. Callbacks would have the same problem, every framework writing the same implementation of the same thing. With that said, I completely understand the other side of the argument.

Here are my suggestions:

1.) A config parameter that let's the Rack app decide which type of parameter 'processing' it would like to use.
2.) An 'extended_params' method that returns the extra processed parameters.

3.) A piece of Rack middleware in Rack-contrib that does the pre-processing of the parameters, so it's only used if people put it in line.

My previous post probably indicated this, but my vote is for #3 here with the added convenience of having this available as a mixin (and used that way in the middleware) too.

-Jon

[Scytrin dai Kinthra]

As stated previously, my ideal resolution to this proposal would be a
mesh of #2 and #3, a rack-contrib mixin for Rack::Request that would
add an extended_params method rather than preprocessing.
If only the preprocessing was done then there would probably be a
follow up request for a method to be added to Rack::Request for faster
access.

--
stadik.net

[Christian Neukirchen]

Mark Bates <ma...@mackframework.com> writes:

> Here are my suggestions:
>
> 1.) A config parameter that let's the Rack app decide which type of
> parameter 'processing' it would like to use.
> 2.) An 'extended_params' method that returns the extra processed
> parameters.
> 3.) A piece of Rack middleware in Rack-contrib that does the pre-
> processing of the parameters, so it's only used if people put it in
> line.

Does 3.) imply the parameters always are parsed?

[Mark Bates]

Hmm... Yeah. Good point. I didn't think about that. Back to the
drawing board I guess.

--------
Mark Bates

On Dec 28, 2008, at 6:42 PM, Christian Neukirchen <chneuk...@gmail.com

[Christian Neukirchen]

Mark Bates <ma...@mackframework.com> writes:

> Hmm... Yeah. Good point. I didn't think about that. Back to the
> drawing board I guess.

I wonder if that really hurts... parsing query_string is quick, and
file uploads usually don't happen to arbitrary endpoints.

[Yehuda Katz]

Extended parameters may never actually be used by the endpoint. In
Merb, param parsing doesn't happen unless the user actually tries to
use params()

Also, it is possible for param parsing to be reasonably slow,
especially as more complex parameters are used.

I really want us to be in the habit of trying to avoid processing
things unless they are actually needed.

Sent from my iPhone

[Mark Bates]

Agreed. That's why I said back to the drawing board. :)

--------
Mark Bates

[Matt Todd]

I'm personally against adding specialized methods like extended_params and providing different modes of operation via config values for params parsing. I think it clutters up the API somewhat, and can be constant source of confusion. (Speculation, of course.)

I'm in favor of clean and consistent APIs that perform standardized web development tasks simply.

With this, I think the appropriate response would be to always parse the params only when requested (in Rack::Request, for instance), and should include nested params.

Matt

[Yehuda Katz]

I'm in favor of this, but others were concerned that it would be wasteful if not needed. I actually think that's speculative, since my recollection is that Merb's impl is basically the same speed if nested params are not used. I will confirm tomorrow.

-- Yehuda

[Christian Neukirchen]

"Yehuda Katz" <wyc...@gmail.com> writes:

> I'm in favor of this, but others were concerned that it would be wasteful if
> not needed. I actually think that's speculative, since my recollection is that
> Merb's impl is basically the same speed if nested params are not used. I will
> confirm tomorrow.
>
> -- Yehuda

A good heuristic could be QUERY_STRING =~ /\[/.

Another thing: If we have foo[bar]=quux, should we return a hash like

{"foo" => {"bar" => "quux"},
"foo[bar]" => "quux"}

Then apps not knowing of this behavior can work as usual.

[Scytrin dai Kinthra]

I am in favor of returning the later hash for compatibility, the only
problem being the possibility of conflicting names. How would
foo[bar]=1&foo=true be handled?

--
stadik.net

[Yehuda Katz]

The code is:

    def self.query(query_string, delimiter = '&;', preserve_order = false)

      query = preserve_order ? Dictionary.new : {}

      for pair in (query_string || '').split(/[#{delimiter}] */n)

        key, value = unescape(pair).split('=',2)

        next if key.nil?

        if key.include?('[')

          normalize_params(query, key, value)

        else

          query[key] = value

        end

      end

      preserve_order ? query : query.to_mash

    end

So the only real overhead that happens when you don't use nested hashes is the lookup for "[" in the query string snippet, which should be very very small (and never bubbled up in any profiles we did after making the change).

-- Yehuda

[Joshua Peek]

I do not want to see 2 ways to do this in Rack. It is totally fine
that we decide this feature not be included in core, but having both
implementations is worse.

Please do not include this feature unless we decide it will replace the old one.

--
Joshua Peek

[Yehuda Katz]

What I was showing above was that the overhead from supporting the feature is limited to include?("["), which is *extremely* small.

-- Yehuda

[Magnus Holm]

+1

In the latest Camping we accomplish this by going through each of the
params and clean up with some regex (after Request#params has done its
job). It's however very hackish and not 100% perfect.

The "array[] & hash[key]"-syntax is used by Rails, Merb, Ramaze, Mack,
Camping and PHP (and probably several more) and while it's not defined
in a RFC, it's been made a standard by the framework-makers. I believe
this should live in rack core; maybe in a seperate method.

And let's not forget to make Utils#build_query handle Hash-values too!

(Of course, my real motive for putting this in rack core is because we
need this in Camping, and I don't like adding another depenency, but
hey! It's a really useful feature :-)