aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/CHANGELOG.md8
-rw-r--r--actionpack/lib/action_dispatch/http/request.rb5
-rw-r--r--actionpack/lib/action_dispatch/request/utils.rb15
-rw-r--r--actionpack/test/dispatch/request_test.rb16
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