Skip to content

Commit

Permalink
Return 400 Bad Request for URL paths with invalid encoding.
Browse files Browse the repository at this point in the history
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 rails#4450
  • Loading branch information
pixeltrix committed May 20, 2012
1 parent 66eb3f0 commit 3fc561a
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 0 deletions.
2 changes: 2 additions & 0 deletions 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
Expand Down
9 changes: 9 additions & 0 deletions actionpack/lib/action_dispatch/routing/redirection.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions actionpack/lib/action_dispatch/routing/route_set.rb
Expand Up @@ -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.
Expand Down
31 changes: 31 additions & 0 deletions actionpack/test/dispatch/routing_test.rb
Expand Up @@ -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.