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

rails#39461 changed the `no-store`
directive for the `Cache-Control` header to be exclusive, i.e. when
setting `Cache-Control` to `private, no-store`, this is simplified to
just `no-store`. `private` should typically be superfluous there, but
it's not always.

For instance, Fastly "does not currently respect no-store or no-cache
directives" and says that "if you need to prevent caching by both Fastly
and web browsers, we recommend combining the private directive with
max-age=0 or no-store".

https://docs.fastly.com/en/guides/configuring-caching#do-not-cache

Since it's not possible to override this directive reduction behaviour,
the changes in rails#39461 prevent Fastly users from upgrading Rails.

This changes the behaviour to allow setting a 'private, no-store' header
when private is specified - similar to how 'public' can be specified
when 'no-cache' is, but not as a default.

Fixes rails#40798

Author: ags

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)