diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 5d4ae16c09573c3825fad19033ff010b0507d60b..94b3490b80493c2d7d1a3e0e78ccdf201477a7d7 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -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 diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index b3823bb496cfa6d7fc79be00f37c572db623fd71..205ff44b1c6b1f09d1c37d19ca238e48a8bc56fb 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -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 diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 0ae668d42a2109d8f73edbcb7f85ba9e7a7dbf5f..1d0a67d0d246e8e6fc339713f018693e93e02686 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -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. diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 1a8f40037ff247a57acaf37404377c1157edd740..00d09282ca703a8b2ded6d63579d38a45dd53419 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -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 \ No newline at end of file