Permalink
Browse files

Return 400 Bad Request for URL paths with invalid encoding.

Passing path parameters with invalid encoding is likely to trigger errors
further on like `ArgumentError (invalid byte sequence in UTF-8)`. This will
result in a 500 error whereas the better error to return is a 400 error which
allows exception notification libraries to filter it out if they wish.

Closes #4450
  • Loading branch information...
1 parent 66eb3f0 commit 3fc561a1f71edf1c2bae695cafa03909d24a5ca3 @pixeltrix pixeltrix committed May 20, 2012
@@ -1,5 +1,7 @@
## Rails 4.0.0 (unreleased) ##
+* URL path parameters with invalid encoding now raise ActionController::BadRequest. *Andrew White*
+
* Malformed query and request parameter hashes now raise ActionController::BadRequest. *Andrew White*
* Add `divider` option to `grouped_options_for_select` to generate a separator
@@ -2,6 +2,7 @@
require 'active_support/core_ext/uri'
require 'active_support/core_ext/array/extract_options'
require 'rack/utils'
+require 'action_controller/metal/exceptions'
module ActionDispatch
module Routing
@@ -16,6 +17,14 @@ def initialize(status, block)
def call(env)
req = Request.new(env)
+ # If any of the path parameters has a invalid encoding then
+ # raise since it's likely to trigger errors further on.
+ req.symbolized_path_parameters.each do |key, value|
+ unless value.valid_encoding?
+ raise ActionController::BadRequest, "Invalid parameter: #{key} => #{value}"
+ end
+ end
+
uri = URI.parse(path(req.symbolized_path_parameters, req))
uri.scheme ||= req.scheme
uri.host ||= req.host
@@ -26,6 +26,15 @@ def initialize(options={})
def call(env)
params = env[PARAMETERS_KEY]
+
+ # If any of the path parameters has a invalid encoding then
+ # raise since it's likely to trigger errors further on.
+ params.each do |key, value|
+ unless value.valid_encoding?
+ raise ActionController::BadRequest, "Invalid parameter: #{key} => #{value}"
+ end
+ end
+
prepare_params!(params)
# Just raise undefined constant errors if a controller was specified as default.
@@ -2697,3 +2697,34 @@ def app; Routes end
assert_response :success
end
end
+
+class TestInvalidUrls < ActionDispatch::IntegrationTest
+ class FooController < ActionController::Base
+ def show
+ render :text => "foo#show"
+ end
+ end
+
+ test "invalid UTF-8 encoding returns a 400 Bad Request" do
+ with_routing do |set|
+ set.draw do
+ get "/bar/:id", :to => redirect("/foo/show/%{id}")
+ get "/foo/show(/:id)", :to => "test_invalid_urls/foo#show"
+ get "/foo(/:action(/:id))", :to => "test_invalid_urls/foo"
+ get "/:controller(/:action(/:id))"
+ end
+
+ get "/%E2%EF%BF%BD%A6"
+ assert_response :bad_request
+
+ get "/foo/%E2%EF%BF%BD%A6"
+ assert_response :bad_request
+
+ get "/foo/show/%E2%EF%BF%BD%A6"
+ assert_response :bad_request
+
+ get "/bar/%E2%EF%BF%BD%A6"
+ assert_response :bad_request
+ end
+ end
+end

0 comments on commit 3fc561a

Please sign in to comment.