Merge pull request #23941 from chiragsinghal/patch-1

Return 307 status instead of 301 when rerouting POST requests to SSL

actionpack/CHANGELOG.md

@@ -1,3 +1,23 @@
+*   SSL: Changes redirect behavior for all non-GET and non-HEAD requests
+    (like POST/PUT/PATCH etc) to `http://` resources to redirect to `https://`
+    with a [307 status code](http://tools.ietf.org/html/rfc7231#section-6.4.7) instead of [301 status code](http://tools.ietf.org/html/rfc7231#section-6.4.2).
+
+    307 status code instructs the HTTP clients to preserve the original
+    request method while redirecting. It has been part of HTTP RFC since
+    1999 and is implemented/recognized by most (if not all) user agents.
+
+        # Before
+        POST http://example.com/articles (i.e. ArticlesContoller#create)
+        redirects to
+        GET https://example.com/articles (i.e. ArticlesContoller#index)
+
+        # After
+        POST http://example.com/articles (i.e. ArticlesContoller#create)
+        redirects to
+        POST https://example.com/articles (i.e. ArticlesContoller#create)
+
+   *Chirag Singhal*
+
 *   Add `:as` option to `ActionController:TestCase#process` and related methods.
 
     Specifying `as: mime_type` allows the `CONTENT_TYPE` header to be specified

actionpack/lib/action_dispatch/middleware/ssl.rb

@@ -133,12 +133,20 @@ module ActionDispatch
       end
 
       def redirect_to_https(request)
-        [ @redirect.fetch(:status, 301),
+        [ @redirect.fetch(:status, redirection_status(request)),
           { "Content-Type" => "text/html",
             "Location" => https_location_for(request) },
           @redirect.fetch(:body, []) ]
       end
 
+      def redirection_status(request)
+        if request.get? || request.head?
+          301 # Issue a permanent redirect via a GET request.
+        else
+          307 # Issue a fresh request redirect to preserve the HTTP method.
+        end
+      end
+
       def https_location_for(request)
         host = @redirect[:host] || request.host
         port = @redirect[:port] || request.port

actionpack/test/dispatch/ssl_test.rb

@@ -38,6 +38,16 @@ class RedirectSSLTest < SSLTest
     assert_equal redirect[:body].join, @response.body
   end
 
+  def assert_post_redirected(redirect: {}, from: "http://a/b?c=d",
+    to: from.sub("http", "https"))
+
+    self.app = build_app ssl_options: { redirect: redirect }
+
+    post from
+    assert_response redirect[:status] || 307
+    assert_redirected_to to
+  end
+
   test "exclude can avoid redirect" do
     excluding = { exclude: -> request { request.path =~ /healthcheck/ } }
 
@@ -57,6 +67,10 @@ class RedirectSSLTest < SSLTest
     assert_redirected
   end
 
+  test "http POST is redirected to https with status 307" do
+    assert_post_redirected
+  end
+
   test "redirect with non-301 status" do
     assert_redirected redirect: { status: 307 }
   end