Change nested query parsing behavior of "foo[]" from {"foo" => [nil]} to {"foo" => []}

Hello lovely folks,

first and foremost thanks for your excellent work on this project practically the entire ruby webdev eco system relies on 💚 🎉 🚀

Problem

I've come here today to propose a change in the parsing behavior of the query string param[].

There is a spec asserting the current behavior so I assume it's intentional

Rack::Utils.parse_nested_query("foo[]").
must_equal "foo" => [nil]

I believe/hope the more correct test would be:

    Rack::Utils.parse_nested_query("foo[]").
      must_equal "foo" => []

I base this on three things:

  • we otherwise have no way to indicate an empty array and an array with nil doesn't seem particularly useful (or intuitive) vs. an empty array
  • the introduction was 12 years ago ( 6e330bd ) and might be a side effect of the implementation due to the splitting behavior exhibited here:
    unless qs.nil? || qs.empty?
    (qs || '').split(d ? (COMMON_SEP[d] || /[#{d}] */n) : DEFAULT_SEP).each do |p|
    k, v = p.split('=', 2).map! { |s| unescape(s) }
    normalize_params(params, k, v, param_depth_limit)
    end
  • the popular HTTP client library faraday (which her and many projects rely upon) changed it's way of serializing empty arrays to the query string above #800 correct handling parameter value empty array lostisland/faraday#801

Especially due to the last point this creates a sort of dissonance in the Ruby eco system where I send an empty array from the one side and it arrives as [nil] on the server side. Of course, the fix might also be on the faraday side.

I have not yet checked the HTTP specs or anything about this to see what's "officially correct"

To have complete traceability, I came here via ruby-grape/grape#2079 :)

Implementation

I'm not sure if it's the complete implementation but it seems to be easily fixable in

elsif after == "[]"
params[k] ||= []
raise ParameterTypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array)
params[k] << v

all it needs is unless v.nil?:

      elsif after == "[]"
        params[k] ||= []
        raise ParameterTypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array)
        params[k] << v unless v.nil?

All tests (with exception of the one mentioned above) pass with this change.

(I have the code, I found the old spec while implementing this/my spec was elsewhere)

Happy to make a PR should it be wanted!

Thanks for all your work! 🎉 🚀