Merge pull request #5515 from rafaelfranca/remove-exclude

Remove exclude option from ActionDispatch::SSL and fix secure cookies

2 files changed, 29 insertions, 14 deletions

diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb
index c758110367..9098f4e170 100644
--- a/actionpack/lib/action_dispatch/middleware/ssl.rb
+++ b/actionpack/lib/action_dispatch/middleware/ssl.rb
@@ -13,14 +13,11 @@ module ActionDispatch
       @hsts = {} if @hsts == true
       @hsts = self.class.default_hsts_options.merge(@hsts) if @hsts
 
-      @exclude = options[:exclude]
       @host    = options[:host]
       @port    = options[:port]
     end
 
     def call(env)
-      return @app.call(env) if exclude?(env)
-
       request = Request.new(env)
 
       if request.ssl?
@@ -34,10 +31,6 @@ module ActionDispatch
     end
 
     private
-      def exclude?(env)
-        @exclude && @exclude.call(env)
-      end
-
       def redirect_to_https(request)
         url        = URI(request.url)
         url.scheme = "https"
@@ -65,7 +58,7 @@ module ActionDispatch
           cookies = cookies.split("\n")
 
           headers['Set-Cookie'] = cookies.map { |cookie|
-            if cookie !~ /; secure(;|$)/
+            if cookie !~ /;\s+secure(;|$)/
               "#{cookie}; secure"
             else
               cookie
diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb
index b1463f31cf..6f075a9074 100644
--- a/actionpack/test/dispatch/ssl_test.rb
+++ b/actionpack/test/dispatch/ssl_test.rb
@@ -31,12 +31,6 @@ class SSLTest < ActionDispatch::IntegrationTest
       response.headers['Location']
   end
 
-  def test_exclude_from_redirect
-    self.app = ActionDispatch::SSL.new(default_app, :exclude => lambda { |env| true })
-    get "http://example.org/"
-    assert_response :success
-  end
-
   def test_hsts_header_by_default
     get "https://example.org/"
     assert_equal "max-age=31536000",
@@ -90,6 +84,34 @@ class SSLTest < ActionDispatch::IntegrationTest
       response.headers['Set-Cookie'].split("\n")
   end
 
+  def test_flag_cookies_as_secure_with_more_spaces_before
+    self.app = ActionDispatch::SSL.new(lambda { |env|
+      headers = {
+        'Content-Type' => "text/html",
+        'Set-Cookie' => "problem=def; path=/; HttpOnly;  secure"
+      }
+      [200, headers, ["OK"]]
+    })
+
+    get "https://example.org/"
+    assert_equal ["problem=def; path=/; HttpOnly;  secure"],
+      response.headers['Set-Cookie'].split("\n")
+  end
+
+  def test_flag_cookies_as_secure_with_more_spaces_after
+    self.app = ActionDispatch::SSL.new(lambda { |env|
+      headers = {
+        'Content-Type' => "text/html",
+        'Set-Cookie' => "problem=def; path=/; secure;  HttpOnly"
+      }
+      [200, headers, ["OK"]]
+    })
+
+    get "https://example.org/"
+    assert_equal ["problem=def; path=/; secure;  HttpOnly"],
+      response.headers['Set-Cookie'].split("\n")
+  end
+
   def test_no_cookies
     self.app = ActionDispatch::SSL.new(lambda { |env|
       [200, {'Content-Type' => "text/html"}, ["OK"]]