Merge pull request #899 from matthewd/1-6-semicolon

[1-6-stable] Fix GET semicolons without breaking API compatibility

HISTORY.md

@@ -1,3 +1,7 @@
+Fri Jun 19 07:14:50 2015  Matthew Draper <matthew@trebex.net>
+
+	* Work around a Rails incompatibility in our private API
+
 Fri Jun 12 11:37:41 2015  Aaron Patterson <tenderlove@ruby-lang.org>
 
 	* Prevent extremely deep parameters from being parsed. CVE-2015-3225

lib/rack/request.rb

@@ -188,7 +188,7 @@ def GET
       if @env["rack.request.query_string"] == query_string
         @env["rack.request.query_hash"]
       else
-        p = parse_query(query_string)
+        p = parse_query({ :query => query_string, :separator => '&;' })
         @env["rack.request.query_string"] = query_string
         @env["rack.request.query_hash"]   = p
       end
@@ -212,7 +212,7 @@ def POST
           form_vars.slice!(-1) if form_vars[-1] == ?\0
 
           @env["rack.request.form_vars"] = form_vars
-          @env["rack.request.form_hash"] = parse_query(form_vars)
+          @env["rack.request.form_hash"] = parse_query({ :query => form_vars, :separator => '&' })
 
           @env["rack.input"].rewind
         end
@@ -366,7 +366,9 @@ def reject_trusted_ip_addresses(ip_addresses)
       end
 
       def parse_query(qs)
-        Utils.parse_nested_query(qs, '&')
+        d = '&'
+        qs, d = qs[:query], qs[:separator] if Hash === qs
+        Utils.parse_nested_query(qs, d)
       end
 
       def parse_multipart(env)

test/spec_request.rb

@@ -134,12 +134,23 @@
     req.params.should.equal "foo" => "bar", "quux" => "bla"
   end
 
-  should "not truncate query strings containing semi-colons #543" do
-    req = Rack::Request.new(Rack::MockRequest.env_for("/?foo=bar&quux=b;la"))
-    req.query_string.should.equal "foo=bar&quux=b;la"
-    req.GET.should.equal "foo" => "bar", "quux" => "b;la"
+  should "not truncate query strings containing semi-colons #543 only in POST" do
+    mr = Rack::MockRequest.env_for("/",
+      "REQUEST_METHOD" => 'POST',
+      :input => "foo=bar&quux=b;la")
+    req = Rack::Request.new mr
+    req.query_string.should.equal ""
+    req.GET.should.be.empty
+    req.POST.should.equal "foo" => "bar", "quux" => "b;la"
+    req.params.should.equal req.GET.merge(req.POST)
+  end
+
+  should "use semi-colons as separators for query strings in GET" do
+    req = Rack::Request.new(Rack::MockRequest.env_for("/?foo=bar&quux=b;la;wun=duh"))
+    req.query_string.should.equal "foo=bar&quux=b;la;wun=duh"
+    req.GET.should.equal "foo" => "bar", "quux" => "b", "la" => nil, "wun" => "duh"
     req.POST.should.be.empty
-    req.params.should.equal "foo" => "bar", "quux" => "b;la"
+    req.params.should.equal "foo" => "bar", "quux" => "b", "la" => nil, "wun" => "duh"
   end
 
   should "limit the keys from the GET query string" do