bug #36833 [HttpKernel] Fix that the Store would not save responses with the X-Content-Digest header present (mpdude)

This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Fix that the `Store` would not save responses with the X-Content-Digest header present

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

Responses fetched from upstream sources might have a `X-Content-Digest` header, for example if the Symfony Cache is used upstream. This currently prevents the `Store` from saving such responses. In general, the value of this header should not be trusted.

As I consider this header an implementation detail of the `Store`, the fix tries to be local to that class; we should not rely on the `HttpCache` or other classes to remove untrustworthy headers for us.

This fixes the issue that when using the `HttpCache` in combination with the Symfony HttpClient, responses that have also been cached upstream in an instance of `HttpCache` are not cached locally. It adds the overhead of re-computing the content digest every time the `HttpCache` successfully re-validated a response.

Commits
-------

d8964fb8b7 [HttpKernel] Fix that the `Store` would not save responses with the X-Content-Digest header present

Author: nicolas-grekas

HttpCache/Store.php

@@ -177,19 +177,15 @@ public function write(Request $request, Response $response)
         $key = $this->getCacheKey($request);
         $storedEnv = $this->persistRequest($request);
 
-        // write the response body to the entity store if this is the original response
-        if (!$response->headers->has('X-Content-Digest')) {
-            $digest = $this->generateContentDigest($response);
+        $digest = $this->generateContentDigest($response);
+        $response->headers->set('X-Content-Digest', $digest);
 
-            if (!$this->save($digest, $response->getContent())) {
-                throw new \RuntimeException('Unable to store the entity.');
-            }
-
-            $response->headers->set('X-Content-Digest', $digest);
+        if (!$this->save($digest, $response->getContent(), false)) {
+            throw new \RuntimeException('Unable to store the entity.');
+        }
 
-            if (!$response->headers->has('Transfer-Encoding')) {
-                $response->headers->set('Content-Length', \strlen($response->getContent()));
-            }
+        if (!$response->headers->has('Transfer-Encoding')) {
+            $response->headers->set('Content-Length', \strlen($response->getContent()));
         }
 
         // read existing cache entries, remove non-varying, and add this one to the list
@@ -362,15 +358,20 @@ private function load($key)
     /**
      * Save data for the given key.
      *
-     * @param string $key  The store key
-     * @param string $data The data to store
+     * @param string $key       The store key
+     * @param string $data      The data to store
+     * @param bool   $overwrite Whether existing data should be overwritten
      *
      * @return bool
      */
-    private function save($key, $data)
+    private function save($key, $data, $overwrite = true)
     {
         $path = $this->getPath($key);
 
+        if (!$overwrite && file_exists($path)) {
+            return true;
+        }
+
         if (isset($this->locks[$key])) {
             $fp = $this->locks[$key];
             @ftruncate($fp, 0);

Tests/HttpCache/StoreTest.php

@@ -97,6 +97,27 @@ public function testSetsTheXContentDigestResponseHeaderBeforeStoring()
         $this->assertEquals('en9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', $res['x-content-digest'][0]);
     }
 
+    public function testDoesNotTrustXContentDigestFromUpstream()
+    {
+        $response = new Response('test', 200, ['X-Content-Digest' => 'untrusted-from-elsewhere']);
+
+        $cacheKey = $this->store->write($this->request, $response);
+        $entries = $this->getStoreMetadata($cacheKey);
+        list(, $res) = $entries[0];
+
+        $this->assertEquals('en9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', $res['x-content-digest'][0]);
+        $this->assertEquals('en9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', $response->headers->get('X-Content-Digest'));
+    }
+
+    public function testWritesResponseEvenIfXContentDigestIsPresent()
+    {
+        // Prime the store
+        $this->store->write($this->request, new Response('test', 200, ['X-Content-Digest' => 'untrusted-from-elsewhere']));
+
+        $response = $this->store->lookup($this->request);
+        $this->assertNotNull($response);
+    }
+
     public function testFindsAStoredEntryWithLookup()
     {
         $this->storeSimpleEntry();