Skip to content

Commit

Permalink
Make query string parsing conform to URL spec
Browse files Browse the repository at this point in the history
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:

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

Fixes rack#1696
  • Loading branch information
jeremyevans committed Jul 21, 2020
1 parent 297bf99 commit 26afb6a
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. For info on
### Changed

- BREAKING CHANGE: Require `status` to be an Integer. ([#1662](https://github.com/rack/rack/pull/1662), [@olleolleolle](https://github.com/olleolleolle))
- BREAKING CHANGE: Query parsing now treats parameters without `=` as having the empty string value instead of nil value, to conform to the URL spec. ([#1696](https://github.com/rack/rack/issues/1696), [@jeremyevans](https://github.com/jeremyevans))
- Relax validations around `Rack::Request#host` and `Rack::Request#hostname`. ([#1606](https://github.com/rack/rack/issues/1606), [@pvande](https://github.com/pvande))
- Removed antiquated handlers: FCGI, LSWS, SCGI, Thin. ([#1658](https://github.com/rack/rack/pull/1658), [@ioquatix](https://github.com/ioquatix))
- Removed options from `Rack::Builder.parse_file` and `Rack::Builder.load_file`. ([#1663](https://github.com/rack/rack/pull/1663), [@ioquatix](https://github.com/ioquatix))
Expand Down
1 change: 1 addition & 0 deletions lib/rack/query_parser.rb
Expand Up @@ -86,6 +86,7 @@ def normalize_params(params, name, v, depth)
name =~ %r(\A[\[\]]*([^\[\]]+)\]*)
k = $1 || ''
after = $' || ''
v ||= String.new

if k.empty?
if !v.nil? && name == "[]"
Expand Down
4 changes: 2 additions & 2 deletions test/spec_request.rb
Expand Up @@ -311,9 +311,9 @@ def initialize(*)
it "use semi-colons as separators for query strings in GET" do
req = make_request(Rack::MockRequest.env_for("/?foo=bar&quux=b;la;wun=duh"))
req.query_string.must_equal "foo=bar&quux=b;la;wun=duh"
req.GET.must_equal "foo" => "bar", "quux" => "b", "la" => nil, "wun" => "duh"
req.GET.must_equal "foo" => "bar", "quux" => "b", "la" => "", "wun" => "duh"
req.POST.must_be :empty?
req.params.must_equal "foo" => "bar", "quux" => "b", "la" => nil, "wun" => "duh"
req.params.must_equal "foo" => "bar", "quux" => "b", "la" => "", "wun" => "duh"
end

it "limit the keys from the GET query string" do
Expand Down
16 changes: 9 additions & 7 deletions test/spec_utils.rb
Expand Up @@ -135,7 +135,7 @@ def assert_nested_query(exp, act)

it "parse nested query strings correctly" do
Rack::Utils.parse_nested_query("foo").
must_equal "foo" => nil
must_equal "foo" => ""
Rack::Utils.parse_nested_query("foo=").
must_equal "foo" => ""
Rack::Utils.parse_nested_query("foo=bar").
Expand All @@ -152,7 +152,7 @@ def assert_nested_query(exp, act)
Rack::Utils.parse_nested_query("&foo=1&&bar=2").
must_equal "foo" => "1", "bar" => "2"
Rack::Utils.parse_nested_query("foo&bar=").
must_equal "foo" => nil, "bar" => ""
must_equal "foo" => "", "bar" => ""
Rack::Utils.parse_nested_query("foo=bar&baz=").
must_equal "foo" => "bar", "baz" => ""
Rack::Utils.parse_nested_query("my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F").
Expand All @@ -162,19 +162,19 @@ def assert_nested_query(exp, act)
must_equal "pid=1234" => "1023", "a" => "b"

Rack::Utils.parse_nested_query("foo[]").
must_equal "foo" => [nil]
must_equal "foo" => [""]
Rack::Utils.parse_nested_query("foo[]=").
must_equal "foo" => [""]
Rack::Utils.parse_nested_query("foo[]=bar").
must_equal "foo" => ["bar"]
Rack::Utils.parse_nested_query("foo[]=bar&foo").
must_equal "foo" => nil
must_equal "foo" => ""
Rack::Utils.parse_nested_query("foo[]=bar&foo[").
must_equal "foo" => ["bar"], "foo[" => nil
must_equal "foo" => ["bar"], "foo[" => ""
Rack::Utils.parse_nested_query("foo[]=bar&foo[=baz").
must_equal "foo" => ["bar"], "foo[" => "baz"
Rack::Utils.parse_nested_query("foo[]=bar&foo[]").
must_equal "foo" => ["bar", nil]
must_equal "foo" => ["bar", ""]
Rack::Utils.parse_nested_query("foo[]=bar&foo[]=").
must_equal "foo" => ["bar", ""]

Expand All @@ -185,6 +185,8 @@ def assert_nested_query(exp, act)
Rack::Utils.parse_nested_query("foo[]=bar&baz[]=1&baz[]=2&baz[]=3").
must_equal "foo" => ["bar"], "baz" => ["1", "2", "3"]

Rack::Utils.parse_nested_query("x[y][z]").
must_equal "x" => { "y" => { "z" => "" } }
Rack::Utils.parse_nested_query("x[y][z]=1").
must_equal "x" => { "y" => { "z" => "1" } }
Rack::Utils.parse_nested_query("x[y][z][]=1").
Expand Down Expand Up @@ -338,7 +340,7 @@ def initialize(*)
end

it 'performs the inverse function of #parse_nested_query' do
[{ "foo" => nil, "bar" => "" },
[{ "bar" => "" },
{ "foo" => "bar", "baz" => "" },
{ "foo" => ["1", "2"] },
{ "foo" => "bar", "baz" => ["1", "2", "3"] },
Expand Down

0 comments on commit 26afb6a

Please sign in to comment.