Make query string parsing conform to URL spec

[jeremyevans]

The URL spec section 5.1.3.3 specifies that if = is not present
in the byte sequence, it should be treated as if the byte sequence
is the name of the tuple and the value is the empty string.

This affects all parameters without =, not just arrays:

Rack::Utils.parse_nested_query("foo[bar]&baz[]&quux")
{"foo"=>{"bar"=>nil}, "baz"=>[nil], "quux"=>nil} # Before
{"foo"=>{"bar"=>""}, "baz"=>[""], "quux"=>""}    # After

Fixes #1696

[ioquatix]

The implications of this change are:

1. Can no longer specify nil values in any way (maybe a good thing?), and
2. Existing code that depends on nil may be broken in subtle ways.

As an alternative to specify nil, at least at the top level, simply don't specify the key at all. However, there would be no way to replicate this in an array of values, e.g.

> Rack::Utils.build_query({x: [nil, nil, nil]})
=> "x&x&x"

# Current behaviour: (wrong?)
> Rack::Utils.parse_nested_query("x&x&x")
=> {"x"=>nil}

# Proposed behaviour: (maybe better?)
=> ["", "", ""]

# Alternative option:
=> [nil, nil, nil]

[jeremyevans]

You are correct about the implications. In general, you should omit the parameter if it should have a nil value. That's more of an implicit nil, this would remove the explicit nil.

Correct that this will change behavior and can break things that depend on nil values. That's why I labeled it a breaking change in the CHANGELOG.

Looks like the result of Rack::Utils.build_query({x: [nil, nil, nil]}) is bad. It should be x[]=&x[]=&x[]= or x[]&x[]&x[] or the empty string (if we drop elements with nil values). That's not strictly related to parsing, so it should probably be fixed in a separate pull request. nil won't roundtrip through query parsing, but neither do empty arrays, empty hashes, numbers, booleans, etc. Anything that needs more than what query parsing provides should probably use JSON.

A somewhat informal explanation of query parsing types using an algebraic data types:

A QueryParseResult (result of Rack::Utils.parse_nested_query) is a hash with String keys and QueryParseValue values.

A QueryParseValue is either a:

• String
• QueryParseResult
• Array of String|QueryParseResult

[ioquatix]

I basically agree with this but I'm concerned about the blast radius of this change.

[ioquatix]

Do you think you can tidy up this parsing logic a bit? Using $ variables concerns me a little.

[jeremyevans]

I can, but I don't see the reason to do so in this PR. It's better to keep the implementation the same as before. After all, there are other $ variables used elsewhere in the code, including later in this method. If you would like to remove $ variable usage, we should probably do that globally in a separate PR.