Merge pull request rails#41735 from ags/private-no-store-cache-control

Allow 'private, no-store' Cache-Control header

Author: pixeltrix

actionpack/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Add support for 'private, no-store' Cache-Control headers.
+
+    Previously, 'no-store' was exclusive; no other directives could be specified.
+
+    *Alex Smith*
+
 *   Expand payload of `unpermitted_parameters.action_controller` instrumentation to allow subscribers to
     know which controller action received unpermitted parameters.
 

actionpack/lib/action_dispatch/http/cache.rb

@@ -195,31 +195,30 @@ def merge_and_normalize_cache_control!(cache_control)
 
           control.merge! cache_control
 
+          options = []
+
           if control[:no_store]
-            self._cache_control = NO_STORE
+            options << PRIVATE if control[:private]
+            options << NO_STORE
           elsif control[:no_cache]
-            options = []
             options << PUBLIC if control[:public]
             options << NO_CACHE
             options.concat(control[:extras]) if control[:extras]
-
-            self._cache_control = options.join(", ")
           else
             extras = control[:extras]
             max_age = control[:max_age]
             stale_while_revalidate = control[:stale_while_revalidate]
             stale_if_error = control[:stale_if_error]
 
-            options = []
             options << "max-age=#{max_age.to_i}" if max_age
             options << (control[:public] ? PUBLIC : PRIVATE)
             options << MUST_REVALIDATE if control[:must_revalidate]
             options << "stale-while-revalidate=#{stale_while_revalidate.to_i}" if stale_while_revalidate
             options << "stale-if-error=#{stale_if_error.to_i}" if stale_if_error
             options.concat(extras) if extras
-
-            self._cache_control = options.join(", ")
           end
+
+          self._cache_control = options.join(", ")
         end
       end
     end

actionpack/test/dispatch/live_response_test.rb

@@ -62,12 +62,24 @@ def test_cache_control_is_set_manually
         assert_equal "public", @response.headers["Cache-Control"]
       end
 
+      def test_cache_control_no_store_default_standalone
+        @response.set_header("Cache-Control", "no-store")
+        @response.stream.write "omg"
+        assert_equal "no-store", @response.headers["Cache-Control"]
+      end
+
       def test_cache_control_no_store_is_respected
-        @response.set_header("Cache-Control", "private, no-store")
+        @response.set_header("Cache-Control", "public, no-store")
         @response.stream.write "omg"
         assert_equal "no-store", @response.headers["Cache-Control"]
       end
 
+      def test_cache_control_no_store_private
+        @response.set_header("Cache-Control", "private, no-store")
+        @response.stream.write "omg"
+        assert_equal "private, no-store", @response.headers["Cache-Control"]
+      end
+
       def test_content_length_is_removed
         @response.headers["Content-Length"] = "1234"
         @response.stream.write "omg"

actionpack/test/dispatch/response_test.rb

@@ -306,6 +306,17 @@ def test_only_set_charset_still_defaults_to_text_html
     assert_equal("no-store", resp.headers["Cache-Control"])
   end
 
+  test "respect private, no-store cache-control" do
+    resp = ActionDispatch::Response.new.tap { |response|
+      response.cache_control[:private] = true
+      response.cache_control[:no_store] = true
+      response.body = "Hello"
+    }
+    resp.to_a
+
+    assert_equal("private, no-store", resp.headers["Cache-Control"])
+  end
+
   test "read content type with default charset utf-8" do
     resp = ActionDispatch::Response.new(200, "Content-Type" => "text/xml")
     assert_equal("utf-8", resp.charset)