From 8a7e91b084c9d502ebe569c6c64616f1ebdba7fd Mon Sep 17 00:00:00 2001 From: Patrick Toomey Date: Tue, 22 Aug 2017 12:45:22 -0600 Subject: 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. --- actionpack/lib/action_dispatch/http/cache.rb | 4 +--- actionpack/test/controller/render_test.rb | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) (limited to 'actionpack') 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 -- cgit v1.2.3