From 64f9802e90369bcf8bb906a8c7b01212e02b0e39 Mon Sep 17 00:00:00 2001
From: Chirag Singhal <chirag.singhal@sumerusolutions.com>
Date: Sun, 28 Feb 2016 15:56:12 +0530
Subject: [PATCH] Return 307 status instead of 301 when rerouting POST requests
 to SSL

When `config.force_ssl` is set to `true`, any POST/PUT/DELETE requests coming in to non-secure url are being redirected with a 301 status.
However, when that happens, the request is converted to a GET request and ends up hitting a different action on the controller.

Since we can not do non-GET redirects, we can instead redirect with a 307 status code instead to indicate to the caller that a fresh request should be tried preserving the original request method.

`rack-ssl` gem which was used to achieve this before we had this middleware directly baked into Rails also used to do the same, ref: https://github.com/josh/rack-ssl/blob/master/lib/rack/ssl.rb#L54

This would be specially important for any apps switching from older version of Rails or apps which expose an API through Rails.
---
 actionpack/CHANGELOG.md                       | 20 +++++++++++++++++++
 .../lib/action_dispatch/middleware/ssl.rb     | 10 +++++++++-
 actionpack/test/dispatch/ssl_test.rb          | 14 +++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 9dab1cc76a..d1a2b9b827 100644
--- a/actionpack/CHANGELOG.md
+++ b/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
diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb
index 0b81d0ad43..992daab3aa 100644
--- a/actionpack/lib/action_dispatch/middleware/ssl.rb
+++ b/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
diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb
index ccddb90bb5..71b274bf1e 100644
--- a/actionpack/test/dispatch/ssl_test.rb
+++ b/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