Not possible to use array-syntax for query string params

[hobodave]

Expected behavior (with CURL):

$ curl -X GET "http://requestb.in/1d2oe0t1?foo\[\]=bar&foo\[\]=baz"
ok

Received by server: http://requestb.in/1d2oe0t1?inspect#1mtdlx

The server received foo[]: 'bar' and foo[]:: 'baz' (as query string params).

Incorrect behavior (Manticore):

Manticore.get('http://requestb.in/1d2oe0t1', params: { 'foo[]' => 'bar', 'foo[]' => 'baz' }).body
# "ok"

Received by server: http://requestb.in/1d2oe0t1?inspect#ts3ow1

As you can see, this was received only as a body, and the foo[] => 'bar' param was overwritten by the 'baz' param.

Also, is there a way to bypass the use of HttpGetWithEntity and use a regular RFC-2616 compliant GET request? Perhaps the GetWithEntity should be explicitly configurable?

Thanks!

update

Perhaps the hash_to_entity (assuming we're stuck with using a GET with an entity) should support a regular Ruby Array as the value of a query param and did the conversion on its own. We tried the following, but it exhibited the same behavior as the example above:

# attempted implementation
    def hash_to_entity(hash)
      # This is a really stupid way to get the "lowest common denominator" encoding for the options hash
      # Is there a better way?
      encoding = minimum_encoding_for hash.to_a.flatten.join
      pairs = []
      hash.each do |key, val|
        if val.is_a? Array
          val.each do |_val|
            pairs << BasicNameValuePair.new("#{key}[]", _val)
          end
        else
          pairs << BasicNameValuePair.new(key, val)
        end
      end
      UrlEncodedFormEntity.new(pairs, encoding)
    end

# usage
Manticore.get(url, params: { 'foo' => ['bar', 'baz'] }).body

[cheald]

HttpGetWithEntity should be just a GET, except that it'll accept an entity body. If you don't pass a body/entity param, it'll just behave as an RFC-compliant GET. I would prefer RFC compliance, but there were several early examples of practical use cases with GETs-with-bodies being necessary (Elasticsearch primarily), and a survey of other Ruby HTTP clients showed that they tended to pass entity bodies, so I elected for practical compliance over RFC adherence. Is it actually breaking something for you, or what's sparking the concern?

I'll investigate what's up with the hash collisions; the array syntax should definitely be properly supported.

[hobodave]

Thanks for the quick response @cheald.

The behavior I see of Manticore.get is that it converts any params to a form body. In fact, poking around Manticore::Client I don't see any facility to parse params into a GET query string. Should there be one?

[cheald]

Oh, I see - you're using params rather than query (which is admittedly confusing!); params is really intended to accept a hash as a post body (ie, a form -> Rails -> Manticore request)

Using this spec to test with:

it "can send an array of parameters" do
  response = client.get(local_server, query: {foo: ["baz", "bar"]})
  p JSON.load(response.body)
end

You get this wire output:

2015/08/18 10:27:24:833 MST [DEBUG] headers - http-outgoing-0 >> GET /?foo=baz&foo=bar HTTP/1.1
2015/08/18 10:27:24:835 MST [DEBUG] headers - http-outgoing-0 >> Connection: Keep-Alive
2015/08/18 10:27:24:835 MST [DEBUG] headers - http-outgoing-0 >> Content-Length: 0
2015/08/18 10:27:24:835 MST [DEBUG] headers - http-outgoing-0 >> Host: localhost:55441
2015/08/18 10:27:24:835 MST [DEBUG] headers - http-outgoing-0 >> User-Agent: Manticore 0.4.3
2015/08/18 10:27:24:835 MST [DEBUG] headers - http-outgoing-0 >> Accept-Encoding: gzip,deflate
2015/08/18 10:27:25:115 MST [DEBUG] headers - http-outgoing-0 << HTTP/1.1 200 OK
2015/08/18 10:27:25:115 MST [DEBUG] headers - http-outgoing-0 << Content-Type: text/plain
2015/08/18 10:27:25:115 MST [DEBUG] headers - http-outgoing-0 << Content-Encoding: gzip
2015/08/18 10:27:25:142 MST [DEBUG] DefaultManagedHttpClientConnection - http-outgoing-0: Shutdown connection
2015/08/18 10:27:25:143 MST [DEBUG] MainClientExec - Connection discarded
2015/08/18 10:27:25:143 MST [DEBUG] DefaultManagedHttpClientConnection - http-outgoing-0: Close connection
2015/08/18 10:27:25:143 MST [DEBUG] PoolingHttpClientConnectionManager - Connection released: [id: 0][route: {}->http://localhost:55441][total kept alive: 0; route allocated: 0 of 50; total allocated: 0 of 50]
{"method"=>"GET", "uri"=>{"path"=>"/", "query"=>"foo=baz&foo=bar"}, "version"=>"1.1", "headers"=>{"Connection"=>"Keep-Alive", "Content-Length"=>"0", "Host"=>"localhost:55441", "User-Agent"=>"Manticore 0.4.3", "Accept-Encoding"=>"gzip,deflate"}, "body"=>""}

I think the correct fix is to probably just treat params as equivalent to query for the purpose of GET requests (so that they aren't accidentally passed to as a request body)

[hobodave]

OK @cheald

I just found the ui_from_url_and_options method, and see that I should be passing these as :query options, not :params. :(

[hobodave]

This is what I just found too. Thanks a lot @cheald. I think maybe just an example in the README with query and params would go a long way towards helping prevent other wayward developers from going down this path. I also notice that it isn't in the RDoc block at the top of the Manticore::Client; params is but query is not. Thanks again!

[cheald]

Manticore uses the addressable gem to handle query params, and it looks like the foo: [:bar, :baz] => foo[]=bar&foo[]=baz syntax was explicitly deprecated, as discussed at sporkmonger/addressable#77 - I'll have to do some research and figure out if that behavior is something that Manticore should transparently support, or if we should leave it up to the requester to explicitly build it in. The current query syntax is good enough to avoid collisions for most URI parsers, though, I think?

I'll get the doc updated at a minimum, and see if there are other things I can do to make it more clear. Thanks!

[cheald]

4353034 should smooth over these issues nicely. :params now implicitly becomes :query for verbs which don't typically accept request bodies if :query is not explicitly passed, and more documentation has been added which should help clarify the preferred usage in the future.

Feel free to re-open this ticket if you find anything else not-quite-right with it!

[hobodave]

Thanks for the quick turnaround @cheald.

However, I this is still not generating the foo[] syntax in the Query String, and thus duplicate keys are being ignored by Rails/Rack applications.

e.g.

Manticore.get(url, params: { 'foo' => ['bar', 'baz'] }).body

Is generating this URL:

http://exmple.com?foo=bar&foo=baz

When received by Rails then the params hash simply has { foo: 'baz' }. Here is the change we made that seems to make this work:

    def struct_to_name_value_pairs(value, namespace = nil)
      case value
      when nil
        []
      when Hash
        value.flat_map {|key, val| struct_to_name_value_pairs val, namespace ? "#{namespace}[#{key}]" : key }
      when Array
        value.flat_map {|val| struct_to_name_value_pairs val, "#{namespace}[]" } # This is the changed line
      else
        BasicNameValuePair.new(namespace, value.to_s)
      end
    end

[cheald]

Yeah, that's the issue that I was referring to WRT Addressable; it was in at one point, and then rolled back for greater compatibility. The Ruby CGI module, for example, interprets each of these as:

> CGI.parse "foo=bar&foo=baz"
 => {"foo"=>["bar", "baz"]}

> CGI.parse "foo[]=bar&foo[]=baz"
 => {"foo[]"=>["bar", "baz"]}

I think that for the time being, I'd suggest that if you want to pass an array of values with []-style multi-value deliniation, I'd recommend:

client.get url, query: {"foo[]" => [:a, :b]}

[hobodave]

@cheald Yep, we just finished hypothesizing and testing that ourselves. That seems reasonable to me. Thanks again!