diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 8 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/request.rb | 5 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/request/utils.rb | 15 | ||||
| -rw-r--r-- | actionpack/test/dispatch/request_test.rb | 16 |
4 files changed, 37 insertions, 7 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a3b4f6e989..4f95a9bab9 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Catch invalid UTF-8 querystring values and respond with BadRequest + + Check querystring params for invalid UTF-8 characters, and raise an + ActionController::BadRequest error if present. Previously these strings + would typically trigger errors further down the stack. + + *Grey Baker* + * Parse RSS/ATOM responses as XML, not HTML. *Alexander Kaupanin* diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index c6ab4dbc9a..35e3ac304f 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -338,7 +338,10 @@ module ActionDispatch # Override Rack's GET method to support indifferent access def GET fetch_header("action_dispatch.request.query_parameters") do |k| - set_header k, Request::Utils.normalize_encode_params(super || {}) + rack_query_params = super || {} + # Check for non UTF-8 parameter values, which would cause errors later + Request::Utils.check_param_encoding(rack_query_params) + set_header k, Request::Utils.normalize_encode_params(rack_query_params) end rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e raise ActionController::BadRequest.new("Invalid query parameters: #{e.message}", e) diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index a8151a8224..bb3df3c311 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -13,6 +13,21 @@ module ActionDispatch end end + def self.check_param_encoding(params) + case params + when Array + params.each { |element| check_param_encoding(element) } + when Hash + params.each_value { |value| check_param_encoding(value) } + when String + unless params.valid_encoding? + # Raise Rack::Utils::InvalidParameterError for consistency with Rack. + # ActionDispatch::Request#GET will re-raise as a BadRequest error. + raise Rack::Utils::InvalidParameterError, "Non UTF-8 value: #{params}" + end + end + end + class ParamEncoder # :nodoc: # Convert nested Hash to HashWithIndifferentAccess. # diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index dfedc8ae25..e9896a71f4 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -977,13 +977,17 @@ class RequestParameters < BaseRequestTest test "parameters not accessible after rack parse error of invalid UTF8 character" do request = stub_request("QUERY_STRING" => "foo%81E=1") + assert_raises(ActionController::BadRequest) { request.parameters } + end - 2.times do - assert_raises(ActionController::BadRequest) do - # rack will raise a Rack::Utils::InvalidParameterError when parsing this query string - request.parameters - end - end + test "parameters containing an invalid UTF8 character" do + request = stub_request("QUERY_STRING" => "foo=%81E") + assert_raises(ActionController::BadRequest) { request.parameters } + end + + test "parameters containing a deeply nested invalid UTF8 character" do + request = stub_request("QUERY_STRING" => "foo[bar]=%81E") + assert_raises(ActionController::BadRequest) { request.parameters } end test "parameters not accessible after rack parse error 1" do |