From ea40dd3328aa1b0bef26a4f8dc970ed8e45f231e Mon Sep 17 00:00:00 2001 From: Alex Robbin Date: Thu, 24 Dec 2020 22:29:44 -0500 Subject: [PATCH] quietly handle unknown HTTP methods in Action Dispatch SSL middleware Because `ActionDispatch::SSL` is included higher up in the middleware stack than `ActionDispatch::ShowExceptions`, it should ideally not be raising any exceptions. In this case, `ActionDispatch::Request#{get,head}?` are called, which check if the HTTP method is valid. If it isn't, `ActionController::UnknownHttpMethod` is raised. Instead of calling the Rack-provided predicate methods, we leverage `raw_request_method`. --- actionpack/lib/action_dispatch/middleware/ssl.rb | 4 +++- actionpack/test/dispatch/ssl_test.rb | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index cd3883f0e2..9cff2a1dc6 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -52,6 +52,8 @@ module ActionDispatch # Default to 2 years as recommended on hstspreload.org. HSTS_EXPIRES_IN = 63072000 + PERMANENT_REDIRECT_REQUEST_METHODS = %w[GET HEAD].freeze + def self.default_hsts_options { expires: HSTS_EXPIRES_IN, subdomains: true, preload: false } end @@ -131,7 +133,7 @@ module ActionDispatch end def redirection_status(request) - if request.get? || request.head? + if PERMANENT_REDIRECT_REQUEST_METHODS.include?(request.raw_request_method) 301 # Issue a permanent redirect via a GET request. elsif @ssl_default_redirect_status @ssl_default_redirect_status diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index 0115c51298..da26cf1283 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -68,6 +68,15 @@ class RedirectSSLTest < SSLTest assert_redirected redirect: { status: 308 } end + test "redirect with unknown request method" do + self.app = build_app + + process :not_an_http_method, "http://a/b?c=d" + + assert_response 307 + assert_redirected_to "https://a/b?c=d" + end + test "redirect with ssl_default_redirect_status" do self.app = build_app(ssl_options: { ssl_default_redirect_status: 308 })