Skip to content

Loading…

Create an array when multiple instances of the same query parameter are used #519

Closed
wants to merge 1 commit into from

3 participants

@kreynolds

This PR is sure to cause controversy, but it seems like broken behavior to disallow the ability for specifying multiple variables to create an array instead of having the last one win. I understand the use of the bracket hash notation and maybe this could be modified to only operate on root variables instead of nested ones, but that seems just as broken/magical to me.

@rkh
Official Rack repositories member

btw, servlets always create an array

@rkh
Official Rack repositories member
rkh commented

This is a breaking change. Any opinions?

@raggi
Official Rack repositories member

Cannot happen before 2.0.

@raggi raggi closed this
@zaphod42 zaphod42 referenced this pull request in puppetlabs/puppet
Closed

(#22652) Parse query strings in rack w/o rack #1938

@haus haus pushed a commit to haus/puppet that referenced this pull request
@zaphod42 zaphod42 (#22652) Parse query strings in rack w/o rack
Rack does not actually support parsing multiple valued query strings
into an array of values (see rack/rack#519).
This is not going to be supported before rack 2.0, but we need this
behavior in order to not have to deal with YAML query parameters.

This changes the rack based query parameter parsing to use CGI.parse
instead of the built in rack semantics (which also has a large number of
other semantics, which we weren't using). As a note, it does not appear
that the behavior of rack's query parameter parsing is documented
anywhere.
8f2967e
@ffrank ffrank pushed a commit to ffrank/puppet that referenced this pull request
@zaphod42 zaphod42 (#22652) Parse query strings in rack w/o rack
Rack does not actually support parsing multiple valued query strings
into an array of values (see rack/rack#519).
This is not going to be supported before rack 2.0, but we need this
behavior in order to not have to deal with YAML query parameters.

This changes the rack based query parameter parsing to use CGI.parse
instead of the built in rack semantics (which also has a large number of
other semantics, which we weren't using). As a note, it does not appear
that the behavior of rack's query parameter parsing is documented
anywhere.
9bd7abc
@abargnesi abargnesi added a commit to OpenBEL/openbel-api that referenced this pull request
@abargnesi abargnesi multi-parameter 'value' (side-steps rack)
rack does not parse same-key parameters to an array so the
env["QUERY_STRING"] value (from rack) is parsed directly

rack/rack#519

400 returned if value is not present (value+ required)
c069f43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 11 additions and 3 deletions.
  1. +9 −1 lib/rack/utils.rb
  2. +2 −2 test/spec_utils.rb
View
10 lib/rack/utils.rb
@@ -108,7 +108,15 @@ def normalize_params(params, name, v = nil)
return if k.empty?
if after == ""
- params[k] = v
+ if cur = params[k]
+ if cur.class == Array
+ params[k] << v
+ else
+ params[k] = [cur, v]
+ end
+ else
+ params[k] = v
+ end
elsif after == "[]"
params[k] ||= []
raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array)
View
4 test/spec_utils.rb
@@ -134,7 +134,7 @@ def kcodeu
should.equal "foo" => "\"bar\""
Rack::Utils.parse_nested_query("foo=bar&foo=quux").
- should.equal "foo" => "quux"
+ should.equal "foo" => ["bar", "quux"]
Rack::Utils.parse_nested_query("foo&foo=").
should.equal "foo" => ""
Rack::Utils.parse_nested_query("foo=1&bar=2").
@@ -170,7 +170,7 @@ def kcodeu
Rack::Utils.parse_nested_query("x[y][z][]=1").
should.equal "x" => {"y" => {"z" => ["1"]}}
Rack::Utils.parse_nested_query("x[y][z]=1&x[y][z]=2").
- should.equal "x" => {"y" => {"z" => "2"}}
+ should.equal "x" => {"y" => {"z" => ["1", "2"]}}
Rack::Utils.parse_nested_query("x[y][z][]=1&x[y][z][]=2").
should.equal "x" => {"y" => {"z" => ["1", "2"]}}
Something went wrong with that request. Please try again.