Fix Utils.build_nested_query to URL-encode all query string fields (#…

…1989) In the formatted query string, the field should be fully URL-encoded. The previous implementation left the square brackets as un-escaped. It was noticed that if params were provided as a nested hash it produced a different result than a hash of string fields. For example, the params { 'q[s]' => 'name' } would result in the query string: q%5Bs%5D=name but { q: { s: 'name' } } would result in: q[s]=name Both forms now produce the same correct result with fully URL-encoded params.
rack · Dec 5, 2022 · ab1f1c1 · ab1f1c1
1 parent 59c29a4
commit ab1f1c1
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file. For info on
 
 - `MethodOverride` does not look for an override if a request does not include form/parseable data.
 - `Rack::Lint::Wrapper` correctly handles `respond_to?` with `to_ary`, `each`, `call` and `to_path`, forwarding to the body. ([#1981](https://github.com/rack/rack/pull/1981), [@ioquatix])
+- `Utils.build_nested_query` URL-encodes nested field names including the square brackets.
 
 ## [3.0.0] - 2022-09-06
 

diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb
@@ -121,13 +121,13 @@ def build_nested_query(value, prefix = nil)
         }.join("&")
       when Hash
         value.map { |k, v|
-          build_nested_query(v, prefix ? "#{prefix}[#{escape(k)}]" : escape(k))
+          build_nested_query(v, prefix ? "#{prefix}[#{k}]" : k)
         }.delete_if(&:empty?).join('&')
       when nil
-        prefix
+        escape(prefix)
       else
         raise ArgumentError, "value must be a Hash" if prefix.nil?
-        "#{prefix}=#{escape(value)}"
+        "#{escape(prefix)}=#{escape(value)}"
       end
     end
 

diff --git a/test/spec_mock_request.rb b/test/spec_mock_request.rb
@@ -192,17 +192,17 @@
     env = YAML.unsafe_load(res.body)
     env["REQUEST_METHOD"].must_equal "GET"
     env["QUERY_STRING"].must_include "baz=2"
-    env["QUERY_STRING"].must_include "foo[bar]=1"
+    env["QUERY_STRING"].must_include "foo%5Bbar%5D=1"
     env["PATH_INFO"].must_equal "/foo"
     env["mock.postdata"].must_equal ""
   end
 
   it "accept raw input in params for GET requests" do
-    res = Rack::MockRequest.new(app).get("/foo?baz=2", params: "foo[bar]=1")
+    res = Rack::MockRequest.new(app).get("/foo?baz=2", params: "foo%5Bbar%5D=1")
     env = YAML.unsafe_load(res.body)
     env["REQUEST_METHOD"].must_equal "GET"
     env["QUERY_STRING"].must_include "baz=2"
-    env["QUERY_STRING"].must_include "foo[bar]=1"
+    env["QUERY_STRING"].must_include "foo%5Bbar%5D=1"
     env["PATH_INFO"].must_equal "/foo"
     env["mock.postdata"].must_equal ""
   end
@@ -214,17 +214,17 @@
     env["QUERY_STRING"].must_equal ""
     env["PATH_INFO"].must_equal "/foo"
     env["CONTENT_TYPE"].must_equal "application/x-www-form-urlencoded"
-    env["mock.postdata"].must_equal "foo[bar]=1"
+    env["mock.postdata"].must_equal "foo%5Bbar%5D=1"
   end
 
   it "accept raw input in params for POST requests" do
-    res = Rack::MockRequest.new(app).post("/foo", params: "foo[bar]=1")
+    res = Rack::MockRequest.new(app).post("/foo", params: "foo%5Bbar%5D=1")
     env = YAML.unsafe_load(res.body)
     env["REQUEST_METHOD"].must_equal "POST"
     env["QUERY_STRING"].must_equal ""
     env["PATH_INFO"].must_equal "/foo"
     env["CONTENT_TYPE"].must_equal "application/x-www-form-urlencoded"
-    env["mock.postdata"].must_equal "foo[bar]=1"
+    env["mock.postdata"].must_equal "foo%5Bbar%5D=1"
   end
 
   it "accept params and build multipart encoded params for POST requests" do

diff --git a/test/spec_utils.rb b/test/spec_utils.rb
@@ -328,9 +328,9 @@ def initialize(*)
     assert_nested_query("my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F",
                         "my weird field" => "q1!2\"'w$5&7/z8)?")
 
-    Rack::Utils.build_nested_query("foo" => [nil]).must_equal "foo[]"
-    Rack::Utils.build_nested_query("foo" => [""]).must_equal "foo[]="
-    Rack::Utils.build_nested_query("foo" => ["bar"]).must_equal "foo[]=bar"
+    Rack::Utils.build_nested_query("foo" => [nil]).must_equal "foo%5B%5D"
+    Rack::Utils.build_nested_query("foo" => [""]).must_equal "foo%5B%5D="
+    Rack::Utils.build_nested_query("foo" => ["bar"]).must_equal "foo%5B%5D=bar"
     Rack::Utils.build_nested_query('foo' => []).must_equal ''
     Rack::Utils.build_nested_query('foo' => {}).must_equal ''
     Rack::Utils.build_nested_query('foo' => 'bar', 'baz' => []).must_equal 'foo=bar'
@@ -341,35 +341,39 @@ def initialize(*)
     Rack::Utils.build_nested_query('foo' => 'bar', 'baz' => '').
       must_equal 'foo=bar&baz='
     Rack::Utils.build_nested_query('foo' => ['1', '2']).
-      must_equal 'foo[]=1&foo[]=2'
+      must_equal 'foo%5B%5D=1&foo%5B%5D=2'
     Rack::Utils.build_nested_query('foo' => 'bar', 'baz' => ['1', '2', '3']).
-      must_equal 'foo=bar&baz[]=1&baz[]=2&baz[]=3'
+      must_equal 'foo=bar&baz%5B%5D=1&baz%5B%5D=2&baz%5B%5D=3'
     Rack::Utils.build_nested_query('foo' => ['bar'], 'baz' => ['1', '2', '3']).
-      must_equal 'foo[]=bar&baz[]=1&baz[]=2&baz[]=3'
+      must_equal 'foo%5B%5D=bar&baz%5B%5D=1&baz%5B%5D=2&baz%5B%5D=3'
     Rack::Utils.build_nested_query('foo' => ['bar'], 'baz' => ['1', '2', '3']).
-      must_equal 'foo[]=bar&baz[]=1&baz[]=2&baz[]=3'
+      must_equal 'foo%5B%5D=bar&baz%5B%5D=1&baz%5B%5D=2&baz%5B%5D=3'
     Rack::Utils.build_nested_query('x' => { 'y' => { 'z' => '1' } }).
-      must_equal 'x[y][z]=1'
+      must_equal 'x%5By%5D%5Bz%5D=1'
     Rack::Utils.build_nested_query('x' => { 'y' => { 'z' => ['1'] } }).
-      must_equal 'x[y][z][]=1'
+      must_equal 'x%5By%5D%5Bz%5D%5B%5D=1'
     Rack::Utils.build_nested_query('x' => { 'y' => { 'z' => ['1', '2'] } }).
-      must_equal 'x[y][z][]=1&x[y][z][]=2'
+      must_equal 'x%5By%5D%5Bz%5D%5B%5D=1&x%5By%5D%5Bz%5D%5B%5D=2'
     Rack::Utils.build_nested_query('x' => { 'y' => [{ 'z' => '1' }] }).
-      must_equal 'x[y][][z]=1'
+      must_equal 'x%5By%5D%5B%5D%5Bz%5D=1'
     Rack::Utils.build_nested_query('x' => { 'y' => [{ 'z' => ['1'] }] }).
-      must_equal 'x[y][][z][]=1'
+      must_equal 'x%5By%5D%5B%5D%5Bz%5D%5B%5D=1'
     Rack::Utils.build_nested_query('x' => { 'y' => [{ 'z' => '1', 'w' => '2' }] }).
-      must_equal 'x[y][][z]=1&x[y][][w]=2'
+      must_equal 'x%5By%5D%5B%5D%5Bz%5D=1&x%5By%5D%5B%5D%5Bw%5D=2'
     Rack::Utils.build_nested_query('x' => { 'y' => [{ 'v' => { 'w' => '1' } }] }).
-      must_equal 'x[y][][v][w]=1'
+      must_equal 'x%5By%5D%5B%5D%5Bv%5D%5Bw%5D=1'
     Rack::Utils.build_nested_query('x' => { 'y' => [{ 'z' => '1', 'v' => { 'w' => '2' } }] }).
-      must_equal 'x[y][][z]=1&x[y][][v][w]=2'
+      must_equal 'x%5By%5D%5B%5D%5Bz%5D=1&x%5By%5D%5B%5D%5Bv%5D%5Bw%5D=2'
     Rack::Utils.build_nested_query('x' => { 'y' => [{ 'z' => '1' }, { 'z' => '2' }] }).
-      must_equal 'x[y][][z]=1&x[y][][z]=2'
+      must_equal 'x%5By%5D%5B%5D%5Bz%5D=1&x%5By%5D%5B%5D%5Bz%5D=2'
     Rack::Utils.build_nested_query('x' => { 'y' => [{ 'z' => '1', 'w' => 'a' }, { 'z' => '2', 'w' => '3' }] }).
-      must_equal 'x[y][][z]=1&x[y][][w]=a&x[y][][z]=2&x[y][][w]=3'
+      must_equal 'x%5By%5D%5B%5D%5Bz%5D=1&x%5By%5D%5B%5D%5Bw%5D=a&x%5By%5D%5B%5D%5Bz%5D=2&x%5By%5D%5B%5D%5Bw%5D=3'
     Rack::Utils.build_nested_query({ "foo" => ["1", ["2"]] }).
-      must_equal 'foo[]=1&foo[][]=2'
+      must_equal 'foo%5B%5D=1&foo%5B%5D%5B%5D=2'
+
+    # A nested hash is the same as string keys with brackets.
+    Rack::Utils.build_nested_query('foo' => { 'bar' => 'baz' }).
+      must_equal Rack::Utils.build_nested_query('foo[bar]' => 'baz')
 
     lambda { Rack::Utils.build_nested_query("foo=bar") }.
       must_raise(ArgumentError).