aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorEdouard CHIN <edouard.chin@shopify.com>2019-03-25 21:51:22 +0100
committerEdouard CHIN <edouard.chin@shopify.com>2019-03-26 02:03:47 +0100
commit378b4fedb1d4b55e642e82d0a7b273803118ca30 (patch)
tree7d6b9d4297d5e7ad1bfe728504142efc266140db /actionpack
parent15ca8ad0c1e94d11d0deb02535bc286e077d43ce (diff)
downloadrails-378b4fedb1d4b55e642e82d0a7b273803118ca30.tar.gz
rails-378b4fedb1d4b55e642e82d0a7b273803118ca30.tar.bz2
rails-378b4fedb1d4b55e642e82d0a7b273803118ca30.zip
Add the `Mime::Type::InvalidMimeType` error in the default rescue_response:
- https://github.com/rails/rails/pull/35604 introduced a vulnerability fix to raise an error in case the `HTTP_ACCEPT` headers contains malformated mime type. This will cause applications to throw a 500 if a User Agent sends an invalid header. This PR adds the `InvalidMimeType` in the default `rescue_responses` from the ExceptionWrapper and will return a 406. I looked up the HTTP/1.1 RFC and it doesn't stand what should be returned when the UA sends malformated mime type. Decided to get 406 as it seemed to be the status the better suited for this.
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/lib/action_dispatch/middleware/debug_exceptions.rb6
-rw-r--r--actionpack/lib/action_dispatch/middleware/exception_wrapper.rb1
-rw-r--r--actionpack/lib/action_dispatch/middleware/public_exceptions.rb8
-rw-r--r--actionpack/test/dispatch/debug_exceptions_test.rb6
-rw-r--r--actionpack/test/dispatch/show_exceptions_test.rb6
5 files changed, 24 insertions, 3 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
index 61773d97a2..bb49bc4dda 100644
--- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -60,7 +60,11 @@ module ActionDispatch
log_error(request, wrapper)
if request.get_header("action_dispatch.show_detailed_exceptions")
- content_type = request.formats.first
+ begin
+ content_type = request.formats.first
+ rescue Mime::Type::InvalidMimeType
+ render_for_api_request(Mime[:text], wrapper)
+ end
if api_request?(content_type)
render_for_api_request(content_type, wrapper)
diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
index 1fb3e9db00..0cc56f5013 100644
--- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
+++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
@@ -12,6 +12,7 @@ module ActionDispatch
"ActionController::UnknownHttpMethod" => :method_not_allowed,
"ActionController::NotImplemented" => :not_implemented,
"ActionController::UnknownFormat" => :not_acceptable,
+ "Mime::Type::InvalidMimeType" => :not_acceptable,
"ActionController::MissingExactTemplate" => :not_acceptable,
"ActionController::InvalidAuthenticityToken" => :unprocessable_entity,
"ActionController::InvalidCrossOriginRequest" => :unprocessable_entity,
diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb
index 3feb3a19f3..a88ad40f21 100644
--- a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb
@@ -21,8 +21,12 @@ module ActionDispatch
def call(env)
request = ActionDispatch::Request.new(env)
status = request.path_info[1..-1].to_i
- content_type = request.formats.first
- body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) }
+ begin
+ content_type = request.formats.first
+ rescue Mime::Type::InvalidMimeType
+ content_type = Mime[:text]
+ end
+ body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) }
render(status, content_type, body)
end
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index c85476fa38..8b1b3c0277 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -58,6 +58,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
raise ActionController::NotImplemented
when "/unprocessable_entity"
raise ActionController::InvalidAuthenticityToken
+ when "/invalid_mimetype"
+ raise Mime::Type::InvalidMimeType
when "/not_found_original_exception"
begin
raise AbstractController::ActionNotFound.new
@@ -178,6 +180,10 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
get "/parameter_missing", headers: { "action_dispatch.show_exceptions" => true }
assert_response 400
assert_match(/ActionController::ParameterMissing/, body)
+
+ get "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => true }
+ assert_response 406
+ assert_match(/Mime::Type::InvalidMimeType/, body)
end
test "rescue with text error for xhr request" do
diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb
index f802abc653..6fafa4e426 100644
--- a/actionpack/test/dispatch/show_exceptions_test.rb
+++ b/actionpack/test/dispatch/show_exceptions_test.rb
@@ -9,6 +9,8 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
case req.path
when "/not_found"
raise AbstractController::ActionNotFound
+ when "/invalid_mimetype"
+ raise Mime::Type::InvalidMimeType
when "/bad_params", "/bad_params.json"
begin
raise StandardError.new
@@ -62,6 +64,10 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
get "/unknown_http_method", env: { "action_dispatch.show_exceptions" => true }
assert_response 405
assert_equal "", body
+
+ get "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => true }
+ assert_response 406
+ assert_equal "", body
end
test "localize rescue error page" do