From 0b4e8c8d84fe4f66e85cf7cfeebe604e0f2849db Mon Sep 17 00:00:00 2001 From: James Tucker Date: Mon, 18 Jun 2012 16:49:03 -0700 Subject: Ensure that cache-control headers are merged There are several aspects to this commit, that don't well fit into broken down commits, so they are detailed here: * When a user uses response.headers['Cache-Control'] = some_value, then the documented convention in ConditionalGet is not adhered to, in this case, response.cache_control is ignored due to `return if self[CACHE_CONTROL].present?` * When a middleware sets cache-control headers that would clobber, they're converted to symbols directly, without underscores. This would lead to bugs. * Items that would live in :extras if set through expires_in, are placed directly in the @cache_control hash, and not respected in many cases (somewhat adhering to the aforementioned documentation). * Although quite useless, any directive named 'extras' would be ignored. The general convention applied is that expires_* take precedence, but no longer overwrite everything and expires_* are ALWAYS applied, even if the header is set. I am still unhappy about the contents of this commit, and the code in general. Ideally it should be refactored to no longer use :extras. I'd likely recommend expanding @cache_control into a class, and giving it the power to handle the merge in a more efficient fashion. Such a commit would be a larger change that could have additional semantic changes for other libraries unless they utilize expires_in in very standard ways. --- actionpack/test/controller/render_test.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'actionpack/test/controller/render_test.rb') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 3d58c02338..e9cfda45a9 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -116,6 +116,12 @@ class TestController < ActionController::Base render :action => 'hello_world' end + def conditional_hello_with_cache_control_headers + response.headers['Cache-Control'] = 'no-transform' + expires_now + render :action => 'hello_world' + end + def conditional_hello_with_bangs render :action => 'hello_world' end @@ -1512,6 +1518,12 @@ class ExpiresInRenderTest < ActionController::TestCase assert_equal "no-cache", @response.headers["Cache-Control"] end + def test_expires_now + get :conditional_hello_with_cache_control_headers + assert_match /no-cache/, @response.headers["Cache-Control"] + assert_match /no-transform/, @response.headers["Cache-Control"] + end + def test_date_header_when_expires_in time = Time.mktime(2011,10,30) Time.stubs(:now).returns(time) -- cgit v1.2.3