uri-query_params icon indicating copy to clipboard operation
uri-query_params copied to clipboard

Arrays and other thoughts

Open mvastola opened this issue 9 years ago • 7 comments

First off, this is an AWESOME gem, because it uniquely provides functionality the built-in URI library is sorely lacking. Thank you for coding it.

The difficulty with this library I think is actually the fact that things are a lot more complicated than they seem at first glance. As a result, some thoughts on features this gem should seek to incorporate/aspire to..

  • in some use cases (especially surrounding adding cryptographic signatures to API requests) the exact order of the parameters matters. Often it's alphabetical, but rules can vary. The best solution here is probably to ensure the exposed object returned by #query_params is an OrderedHash rather than an ordinary one (if not an entirely custom object).
  • Rails in particular has a tendency to use arrays in form element names. So if I have an array of text fields and submit them, I run into this problem:
2.1.2 :009 > u = URI.parse("http://google.com/?list[items][]=1&list[items][]=2")
 => #<URI::HTTP:0x0000000aa61698 URL:http://google.com/?list[items][]=1&list[items][]=2> 
2.1.2 :010 > u.query
 => "list[items][]=1&list[items][]=2" 
2.1.2 :011 > u.query_params
 => {"list[items][]"=>"2"}
  • Obviously the first issue here is that u.query_params should be returning a Hash with one key whose value is an array with the elements 1 and 2, because that's what params[:list][:items] will return to a Rails controller.
  • A second issue is one with your implementation that's a bit more insidious. By the simple act of calling #query_params it seems, without ever setting any values within the object. I have manipulated my URI object. These lines in the IRB console immediately follow those above:
2.1.2 :012 > u.query
 => "list[items][]=2" 
2.1.2 :013 > u
 => #<URI::HTTP:0x0000000aa61698 URL:http://google.com/?list[items][]=2>

This makes this gem completely unusable for me right now because so much as requireing it risks corrupting URLs that are parsed by the URI class in certain cases.

I'm very interested to see where you go with this though and might even be interested to work with you on fixing these bugs. Please let me know what you think in regards to my comments and if you develop some sort of plan for resolving the issues I have brought up.

Thanks, Mike

mvastola avatar Nov 08 '15 01:11 mvastola

This is like several separate issues in one.

  • Re ordering, Ruby >= 1.9 preserves the insertion order of key/values in Hashes. If you need a certain order, simply set the params in that order.
  • Re Rails. I don't know if supporting nested params and Rails specific encoding is within the scope of this gem? I worry if I support Rails style param encoding, I'll end up having to auto-detect and support every other encoding (are they others?).
  • Hm, I did not plan for duplicate params. That should definitely be a feature request.

postmodern avatar Nov 18 '15 22:11 postmodern

Hi @postmodern, sorry for the delay in replying. The reason I submitted these issues together is -- although they seem like idea vomit -- they actually do need to be considered together (at least somewhat, as you'll see in my reply below):

  • I forgot about that on Hash ordering. I stand corrected. Maybe just note that compatibility is limited to 1.9+ then?
  • I personally think it's very important to support what you call Rails-specific encoding (which is to say, multiple occurrences of the same parameter -- I don't think there are other 'encodings'), for two reasons:
    1. This is a Ruby library and -- although I don't know the exact usage statistics offhand -- Rails is the preeminent MVC of the Ruby world. To have this library subtly break any Rails install it is used on would be a cruel punishment to any coder/debugger who was caught using your gem. Even if you generated a warning that checked if defined?(Rails), you're still vastly limiting the usability of the library.
    2. Returning to my example in the OP, it's not simply a matter of "not supporting" a certain style of URLs. As of now, it's a much more serious matter of silently and permanently corrupting URLs and mangling data inside an object, when nothing but a data-retrieval function is called. Even if you were to fix (2), it begs the question how this gem would handle these URLs. Reject them outright? That's not a clean/portable solution.
  • I consider duplicate params the exact same thing as support of "Ruby specific encoding". How do you see them as different? The problem though, is once you make the jump to supporting these, you can't use Hashes (ordered or otherwise), because those rely on unique keys. So basically the entire library needs to be refactored on top of a different primitive object container (a 2-D array would work fine if you want to stay simple). Order would still have to be preserved as before though.

mvastola avatar Nov 29 '15 19:11 mvastola

When I said I wouldn't support Rails-style query params, I meant I wasn't going to automatically add/remove the [] identifier or support deeply nested params (ex: Arrays of Arrays). Repeated params can be automatically coerced into an Array.

postmodern avatar Nov 29 '15 21:11 postmodern

Oh, I see what you mean. Well I think that would be a minor feature enhancement once repeated param support were implemented (I also agree it should be an option most likely), but I don't think support for that is crucial.

What do you mean by

Repeated params can be automatically coerced into an Array.

Do you mean that this is possible now or that that is how you would implement repeated param support? (If the former, I'm not sure how to access that functionality, as it wasn't triggered in the OP.)

Regardless, the issue if this is done is one of param order preservation. Presumably you're talking about having a hash with the params as keys and an array as the value if the param is repeated. If you have a querystring like: ?a=1&b=2&a=3&b=0 it will then be parsed into @params = { 'a' => [ '1', '3' ], 'b' => [ '2', '0' ] } There is no way, from @params to recover the original param ordering.

mvastola avatar Nov 29 '15 22:11 mvastola

@mvastola that's basically what I was thinking. Preserving the original ordering of repeated params would have to be a separate feature.

postmodern avatar Dec 01 '15 21:12 postmodern

but does preserving the exact ordering really matter? Or would preserving the precedence be enough ({ 'a' => [ '1', '3' ], 'b' => [ '2', '0' ]} -> a=1&a=3&b=2&b=0).

postmodern avatar Dec 01 '15 21:12 postmodern

Unfortunately, I don't think so. A lot of APIs do authentication of requests by performing a cryptographic hash on a the request URI that involves the key. This hash is then appended to the querystring as a new param or sent as an X header.

The other side (which also must know the secret key) then works backword, isolating the request URI and following the same calculaton steps before checking for a match.

See Twilio's authentation for an example. (Though they don't use multiple params that I know of.)

mvastola avatar Dec 02 '15 02:12 mvastola