Add Rack::Request#form_pairs (rack#2351)

This method returns form data as key-value pairs, preserving duplicate
keys that would otherwise be lost when converting to a hash. For example,
"a=1&a=2" returns [["a", "1"], ["a", "2"]] instead of {"a" => "2"}.

Author: matthewd

CHANGELOG.md

@@ -6,14 +6,15 @@ All notable changes to this project will be documented in this file. For info on
 
 ### SPEC Changes
 
-- Request environment keys must now be strings. [#2310](https://github.com/rack/rack/issues/2310), [@jeremyevans])
+- Request environment keys must now be strings. ([#2310](https://github.com/rack/rack/issues/2310), [@jeremyevans])
 - Add `nil` as a valid return from a Response `body.to_path` ([#2318](https://github.com/rack/rack/pull/2318), [@MSP-Greg])
 
 ### Added
 
 - Introduce `Rack::VERSION` constant. ([#2199](https://github.com/rack/rack/pull/2199), [@ioquatix])
 - ISO-2022-JP encoded parts within MIME Multipart sections of an HTTP request body will now be converted to UTF-8. ([#2245](https://github.com/rack/rack/pull/2245), [@nappa](https://github.com/nappa))
-- Add `Rack::Request#query_parser=` to allow setting the query parser to use. [#2349](https://github.com/rack/rack/pull/2349), [@jeremyevans])
+- Add `Rack::Request#query_parser=` to allow setting the query parser to use. ([#2349](https://github.com/rack/rack/pull/2349), [@jeremyevans])
+- Add `Rack::Request#form_pairs` to access form data as raw key-value pairs, preserving duplicate keys. ([#2351](https://github.com/rack/rack/pull/2351), [@matthewd])
 
 ### Changed
 
@@ -1205,3 +1206,4 @@ Items below this line are from the previously maintained HISTORY.md and NEWS.md
 [@davidstosik]: https://github.com/davidstosik "David Stosik"
 [@earlopain]: https://github.com/earlopain "Earlopain"
 [@wynksaiddestroy]: https://github.com/wynksaiddestroy "Fabian Winkler"
+[@matthewd]: https://github.com/matthewd "Matthew Draper"

lib/rack/query_parser.rb

@@ -69,14 +69,9 @@ def initialize(params_class, param_depth_limit, bytesize_limit: BYTESIZE_LIMIT,
     # to parse cookies by changing the characters used in the second parameter
     # (which defaults to '&').
     def parse_query(qs, separator = nil, &unescaper)
-      unescaper ||= method(:unescape)
-
       params = make_params
 
-      check_query_string(qs, separator).split(separator ? (COMMON_SEP[separator] || /[#{separator}] */n) : DEFAULT_SEP).each do |p|
-        next if p.empty?
-        k, v = p.split('=', 2).map!(&unescaper)
-
+      each_query_pair(qs, separator, unescaper) do |k, v|
         if cur = params[k]
           if cur.class == Array
             params[k] << v
@@ -91,6 +86,19 @@ def parse_query(qs, separator = nil, &unescaper)
       return params.to_h
     end
 
+    # Parses a query string by breaking it up at the '&', returning all key-value
+    # pairs as an array of [key, value] arrays. Unlike parse_query, this preserves
+    # all duplicate keys rather than collapsing them.
+    def parse_query_pairs(qs, separator = nil)
+      pairs = []
+
+      each_query_pair(qs, separator) do |k, v|
+        pairs << [k, v]
+      end
+
+      pairs
+    end
+
     # parse_nested_query expands a query string into structural types. Supported
     # types are Arrays, Hashes and basic value types. It is possible to supply
     # query strings with parameters of conflicting types, in this case a
@@ -99,17 +107,11 @@ def parse_query(qs, separator = nil, &unescaper)
     def parse_nested_query(qs, separator = nil)
       params = make_params
 
-      unless qs.nil? || qs.empty?
-        check_query_string(qs, separator).split(separator ? (COMMON_SEP[separator] || /[#{separator}] */n) : DEFAULT_SEP).each do |p|
-          k, v = p.split('=', 2).map! { |s| unescape(s) }
-
-          _normalize_params(params, k, v, 0)
-        end
+      each_query_pair(qs, separator) do |k, v|
+        _normalize_params(params, k, v, 0)
       end
 
       return params.to_h
-    rescue ArgumentError => e
-      raise InvalidParameterError, e.message, e.backtrace
     end
 
     # normalize_params recursively expands parameters into structural types. If
@@ -215,20 +217,35 @@ def params_hash_has_key?(hash, key)
       true
     end
 
-    def check_query_string(qs, sep)
-      if qs
-        if qs.bytesize > @bytesize_limit
-          raise QueryLimitError, "total query size (#{qs.bytesize}) exceeds limit (#{@bytesize_limit})"
-        end
+    def each_query_pair(qs, separator, unescaper = nil)
+      return if !qs || qs.empty?
 
-        if (param_count = qs.count(sep.is_a?(String) ? sep : '&')) >= @params_limit
-          raise QueryLimitError, "total number of query parameters (#{param_count+1}) exceeds limit (#{@params_limit})"
-        end
+      if qs.bytesize > @bytesize_limit
+        raise QueryLimitError, "total query size (#{qs.bytesize}) exceeds limit (#{@bytesize_limit})"
+      end
+
+      pairs = qs.split(separator ? (COMMON_SEP[separator] || /[#{separator}] */n) : DEFAULT_SEP, @params_limit + 1)
+
+      if pairs.size > @params_limit
+        param_count = pairs.size + pairs.last.count(separator || "&")
+        raise QueryLimitError, "total number of query parameters (#{param_count}) exceeds limit (#{@params_limit})"
+      end
 
-        qs
+      if unescaper
+        pairs.each do |p|
+          next if p.empty?
+          k, v = p.split('=', 2).map!(&unescaper)
+          yield k, v
+        end
       else
-        ''
+        pairs.each do |p|
+          next if p.empty?
+          k, v = p.split('=', 2).map! { |s| unescape(s) }
+          yield k, v
+        end
       end
+    rescue ArgumentError => e
+      raise InvalidParameterError, e.message, e.backtrace
     end
 
     def unescape(string, encoding = Encoding::UTF_8)

lib/rack/request.rb

@@ -492,13 +492,13 @@ def GET
         get_header(RACK_REQUEST_QUERY_HASH) || set_header(RACK_REQUEST_QUERY_HASH, parse_query(query_string, '&'))
       end
 
-      # Returns the data received in the request body.
+      # Returns the form data pairs received in the request body.
       #
       # This method support both application/x-www-form-urlencoded and
       # multipart/form-data.
-      def POST
-        if form_hash = get_header(RACK_REQUEST_FORM_HASH)
-          return form_hash
+      def form_pairs
+        if pairs = get_header(RACK_REQUEST_FORM_PAIRS)
+          return pairs
         elsif error = get_header(RACK_REQUEST_FORM_ERROR)
           raise error.class, error.message, cause: error.cause
         end
@@ -508,11 +508,10 @@ def POST
 
           # Otherwise, figure out how to parse the input:
           if rack_input.nil?
-            set_header(RACK_REQUEST_FORM_HASH, {})
+            set_header(RACK_REQUEST_FORM_PAIRS, [])
           elsif form_data? || parseable_data?
             if pairs = Rack::Multipart.parse_multipart(env, Rack::Multipart::ParamList)
               set_header RACK_REQUEST_FORM_PAIRS, pairs
-              set_header RACK_REQUEST_FORM_HASH, expand_param_pairs(pairs)
             else
               form_vars = get_header(RACK_INPUT).read
 
@@ -521,17 +520,33 @@ def POST
               form_vars.slice!(-1) if form_vars.end_with?("\0")
 
               set_header RACK_REQUEST_FORM_VARS, form_vars
-              set_header RACK_REQUEST_FORM_HASH, parse_query(form_vars, '&')
+              pairs = query_parser.parse_query_pairs(form_vars, '&')
+              set_header(RACK_REQUEST_FORM_PAIRS, pairs)
             end
           else
-            set_header(RACK_REQUEST_FORM_HASH, {})
+            set_header(RACK_REQUEST_FORM_PAIRS, [])
           end
         rescue => error
           set_header(RACK_REQUEST_FORM_ERROR, error)
           raise
         end
       end
 
+      # Returns the data received in the request body.
+      #
+      # This method support both application/x-www-form-urlencoded and
+      # multipart/form-data.
+      def POST
+        if form_hash = get_header(RACK_REQUEST_FORM_HASH)
+          return form_hash
+        elsif error = get_header(RACK_REQUEST_FORM_ERROR)
+          raise error.class, error.message, cause: error.cause
+        end
+
+        pairs = form_pairs
+        set_header RACK_REQUEST_FORM_HASH, expand_param_pairs(pairs)
+      end
+
       # The union of GET and POST data.
       #
       # Note that modifications will not be persisted in the env. Use update_param or delete_param if you want to destructively modify params.

test/spec_query_parser.rb

@@ -12,25 +12,33 @@
     query_parser.parse_nested_query("a=a").must_equal({"a" => "a"})
     query_parser.parse_nested_query("a=").must_equal({"a" => ""})
     query_parser.parse_nested_query("a").must_equal({"a" => nil})
+    query_parser.parse_query_pairs("a=a").must_equal([["a", "a"]])
+    query_parser.parse_query_pairs("a=").must_equal([["a", ""]])
+    query_parser.parse_query_pairs("a").must_equal([["a", nil]])
   end
 
   it "accepts bytesize_limit to specify maximum size of query string to parse" do
     query_parser = Rack::QueryParser.make_default(32, bytesize_limit: 3)
     query_parser.parse_query("a=a").must_equal({"a" => "a"})
     query_parser.parse_nested_query("a=a").must_equal({"a" => "a"})
     query_parser.parse_nested_query("a=a", '&').must_equal({"a" => "a"})
+    query_parser.parse_query_pairs("a=a").must_equal([["a", "a"]])
     proc { query_parser.parse_query("a=aa") }.must_raise Rack::QueryParser::QueryLimitError
     proc { query_parser.parse_nested_query("a=aa") }.must_raise Rack::QueryParser::QueryLimitError
     proc { query_parser.parse_nested_query("a=aa", '&') }.must_raise Rack::QueryParser::QueryLimitError
+    proc { query_parser.parse_query_pairs("a=aa") }.must_raise Rack::QueryParser::QueryLimitError
   end
 
   it "accepts params_limit to specify maximum number of query parameters to parse" do
     query_parser = Rack::QueryParser.make_default(32, params_limit: 2)
     query_parser.parse_query("a=a&b=b").must_equal({"a" => "a", "b" => "b"})
     query_parser.parse_nested_query("a=a&b=b").must_equal({"a" => "a", "b" => "b"})
     query_parser.parse_nested_query("a=a&b=b", '&').must_equal({"a" => "a", "b" => "b"})
+    query_parser.parse_query_pairs("a=a&b=b").must_equal([["a", "a"], ["b", "b"]])
+    query_parser.parse_query_pairs("a=1&a=2").must_equal([["a", "1"], ["a", "2"]])
     proc { query_parser.parse_query("a=a&b=b&c=c") }.must_raise Rack::QueryParser::QueryLimitError
     proc { query_parser.parse_nested_query("a=a&b=b&c=c", '&') }.must_raise Rack::QueryParser::QueryLimitError
     proc { query_parser.parse_query("b[]=a&b[]=b&b[]=c") }.must_raise Rack::QueryParser::QueryLimitError
+    proc { query_parser.parse_query_pairs("a=a&b=b&c=c") }.must_raise Rack::QueryParser::QueryLimitError
   end
 end

test/spec_request.rb

@@ -795,6 +795,135 @@ def initialize(*)
     req.POST.must_equal "foo" => "bar", "quux" => "bla"
   end
 
+  it "return form_pairs for url-encoded POST data" do
+    req = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST', :input => "foo=bar&quux=bla")
+    req.form_pairs.must_equal [["foo", "bar"], ["quux", "bla"]]
+  end
+
+  it "preserve duplicate keys in form_pairs" do
+    req = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST', :input => "foo=1&foo=2&bar=3")
+    req.form_pairs.must_equal [["foo", "1"], ["foo", "2"], ["bar", "3"]]
+  end
+
+  it "handle empty values in form_pairs" do
+    req = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST', :input => "foo=&bar=baz&empty")
+    req.form_pairs.must_equal [["foo", ""], ["bar", "baz"], ["empty", nil]]
+  end
+
+  it "return empty array for form_pairs with no POST data" do
+    req = make_request \
+      Rack::MockRequest.env_for("/", 'REQUEST_METHOD' => 'POST', :input => "")
+    req.form_pairs.must_equal []
+  end
+
+  it "return empty array for form_pairs with non-form content type" do
+    req = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST',
+        "CONTENT_TYPE" => 'text/plain',
+        :input => "foo=bar")
+    req.form_pairs.must_equal []
+  end
+
+  it "raise same error for form_pairs as POST with invalid encoding" do
+    req = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST', :input => "a%=1")
+    lambda { req.form_pairs }.must_raise(Rack::Utils::InvalidParameterError).
+      message.must_equal "invalid %-encoding (a%)"
+  end
+
+  it "return form_pairs for multipart form data" do
+    input = <<EOF
+--AaB03x\r
+content-disposition: form-data; name="reply"\r
+\r
+yes\r
+--AaB03x\r
+content-disposition: form-data; name="name"\r
+\r
+John\r
+--AaB03x--\r
+EOF
+    req = make_request Rack::MockRequest.env_for("/",
+                      "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x",
+                      "CONTENT_LENGTH" => input.size,
+                      :input => input)
+
+    pairs = req.form_pairs
+    pairs.must_equal [["reply", "yes"], ["name", "John"]]
+  end
+
+  it "preserve duplicate keys in multipart form_pairs" do
+    input = <<EOF
+--AaB03x\r
+content-disposition: form-data; name="item"\r
+\r
+first\r
+--AaB03x\r
+content-disposition: form-data; name="item"\r
+\r
+second\r
+--AaB03x\r
+content-disposition: form-data; name="other"\r
+\r
+value\r
+--AaB03x--\r
+EOF
+    req = make_request Rack::MockRequest.env_for("/",
+                      "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x",
+                      "CONTENT_LENGTH" => input.size,
+                      :input => input)
+
+    pairs = req.form_pairs
+    pairs.must_equal [["item", "first"], ["item", "second"], ["other", "value"]]
+  end
+
+  it "include file uploads in multipart form_pairs" do
+    input = <<EOF
+--AaB03x\r
+content-disposition: form-data; name="reply"\r
+\r
+yes\r
+--AaB03x\r
+content-disposition: form-data; name="fileupload"; filename="test.txt"\r
+content-type: text/plain\r
+\r
+file content\r
+--AaB03x--\r
+EOF
+    req = make_request Rack::MockRequest.env_for("/",
+                      "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x",
+                      "CONTENT_LENGTH" => input.size,
+                      :input => input)
+
+    pairs = req.form_pairs
+    pairs.length.must_equal 2
+    pairs[0].must_equal ["reply", "yes"]
+    pairs[1][0].must_equal "fileupload"
+    pairs[1][1].must_be_kind_of Hash
+    pairs[1][1][:filename].must_equal "test.txt"
+    pairs[1][1][:type].must_equal "text/plain"
+  end
+
+  it "return empty array for empty multipart form_pairs" do
+    input = <<EOF
+--AaB03x--\r
+EOF
+    req = make_request Rack::MockRequest.env_for("/",
+                      "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x",
+                      "CONTENT_LENGTH" => input.size,
+                      :input => input)
+
+    req.form_pairs.must_equal []
+  end
+
   it "extract referrer correctly" do
     req = make_request \
       Rack::MockRequest.env_for("/", "HTTP_REFERER" => "/some/path")