Normalize/process Cach-Control headers consistently

In the existing logic, the `Cache-Control` header may or may not get
normalized by additional logic depending on whether `response.cache_conrol`
has been modified. This leads to inconsistent behavior, since sometimes
`Cache-Control` can contain whatever a user sets and sometimes it gets
normalized, based on the logic inside of `set_conditional_cache_control!`. It
seems like this normalization process should happen regardless to ensure
consistent behavior.

2 files changed, 24 insertions, 3 deletions

diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb
index 2cc609406a..eaff10cb54 100644
--- a/actionpack/lib/action_dispatch/http/cache.rb
+++ b/actionpack/lib/action_dispatch/http/cache.rb
@@ -167,9 +167,7 @@ module ActionDispatch
         end
 
         def handle_conditional_get!
-          if etag? || last_modified? || !@cache_control.empty?
-            set_conditional_cache_control!(@cache_control)
-          end
+          set_conditional_cache_control!(@cache_control)
         end
 
         DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate".freeze
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 3619afc513..f1ec383070 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -162,6 +162,17 @@ class TestController < ActionController::Base
     render action: "hello_world"
   end
 
+  def conditional_hello_with_expires_and_confliciting_cache_control_headers
+    response.headers["Cache-Control"] = "no-cache, must-revalidate"
+    expires_now
+    render action: "hello_world"
+  end
+
+  def conditional_hello_without_expires_and_confliciting_cache_control_headers
+    response.headers["Cache-Control"] = "no-cache, must-revalidate"
+    render action: "hello_world"
+  end
+
   def conditional_hello_with_bangs
     render action: "hello_world"
   end
@@ -368,6 +379,18 @@ class ExpiresInRenderTest < ActionController::TestCase
     assert_match(/no-transform/, @response.headers["Cache-Control"])
   end
 
+  def test_expires_now_with_conflicting_cache_control_headers
+    get :conditional_hello_with_expires_and_confliciting_cache_control_headers
+    assert_match(/no-cache/, @response.headers["Cache-Control"])
+    refute_match(/must-revalidate/, @response.headers["Cache-Control"])
+  end
+
+  def test_no_expires_now_with_conflicting_cache_control_headers
+    get :conditional_hello_without_expires_and_confliciting_cache_control_headers
+    assert_match(/no-cache/, @response.headers["Cache-Control"])
+    refute_match(/must-revalidate/, @response.headers["Cache-Control"])
+  end
+
   def test_date_header_when_expires_in
     time = Time.mktime(2011, 10, 30)
     Time.stub :now, time do