Skip to content

Commit

Permalink
Default to & as separator instead of either & or ;
Browse files Browse the repository at this point in the history
Allowing ; as separator by default can lead to web cache poisoning.

Fixes #1732
  • Loading branch information
jeremyevans committed Jan 20, 2021
1 parent 2223f2f commit b5cbe43
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 9 deletions.
9 changes: 4 additions & 5 deletions lib/rack/query_parser.rb
Expand Up @@ -4,7 +4,7 @@ module Rack
class QueryParser
(require_relative 'core_ext/regexp'; using ::Rack::RegexpExtensions) if RUBY_VERSION < '2.4'

DEFAULT_SEP = /[&;] */n
DEFAULT_SEP = /[&] */n
COMMON_SEP = { ";" => /[;] */n, ";," => /[;,] */n, "&" => /[&] */n }

# ParameterTypeError is the error that is raised when incoming structural
Expand All @@ -29,10 +29,9 @@ def initialize(params_class, key_space_limit, param_depth_limit)
end

# Stolen from Mongrel, with some small modifications:
# Parses a query string by breaking it up at the '&'
# and ';' characters. You can also use this to parse
# cookies by changing the characters used in the second
# parameter (which defaults to '&;').
# Parses a query string by breaking it up at the '&'. You can also use this
# to parse cookies by changing the characters used in the second parameter
# (which defaults to '&').
def parse_query(qs, d = nil, &unescaper)
unescaper ||= method(:unescape)

Expand Down
2 changes: 1 addition & 1 deletion lib/rack/request.rb
Expand Up @@ -427,7 +427,7 @@ def GET
if get_header(RACK_REQUEST_QUERY_STRING) == query_string
get_header(RACK_REQUEST_QUERY_HASH)
else
query_hash = parse_query(query_string, '&;')
query_hash = parse_query(query_string, '&')
set_header(RACK_REQUEST_QUERY_STRING, query_string)
set_header(RACK_REQUEST_QUERY_HASH, query_hash)
end
Expand Down
6 changes: 3 additions & 3 deletions test/spec_request.rb
Expand Up @@ -308,12 +308,12 @@ def initialize(*)
req.params[:quux].must_equal "bla"
end

it "use semi-colons as separators for query strings in GET" do
it "does not 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

0 comments on commit b5cbe43

Please sign in to comment.