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
This commit is contained in:
commit
af0df4ce14
@ -177,19 +177,15 @@ class Store implements StoreInterface
|
|||||||
$key = $this->getCacheKey($request);
|
$key = $this->getCacheKey($request);
|
||||||
$storedEnv = $this->persistRequest($request);
|
$storedEnv = $this->persistRequest($request);
|
||||||
|
|
||||||
// write the response body to the entity store if this is the original response
|
$digest = $this->generateContentDigest($response);
|
||||||
if (!$response->headers->has('X-Content-Digest')) {
|
$response->headers->set('X-Content-Digest', $digest);
|
||||||
$digest = $this->generateContentDigest($response);
|
|
||||||
|
|
||||||
if (!$this->save($digest, $response->getContent())) {
|
if (!$this->save($digest, $response->getContent(), false)) {
|
||||||
throw new \RuntimeException('Unable to store the entity.');
|
throw new \RuntimeException('Unable to store the entity.');
|
||||||
}
|
}
|
||||||
|
|
||||||
$response->headers->set('X-Content-Digest', $digest);
|
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
|
// read existing cache entries, remove non-varying, and add this one to the list
|
||||||
@ -362,15 +358,20 @@ class Store implements StoreInterface
|
|||||||
/**
|
/**
|
||||||
* Save data for the given key.
|
* Save data for the given key.
|
||||||
*
|
*
|
||||||
* @param string $key The store key
|
* @param string $key The store key
|
||||||
* @param string $data The data to store
|
* @param string $data The data to store
|
||||||
|
* @param bool $overwrite Whether existing data should be overwritten
|
||||||
*
|
*
|
||||||
* @return bool
|
* @return bool
|
||||||
*/
|
*/
|
||||||
private function save($key, $data)
|
private function save($key, $data, $overwrite = true)
|
||||||
{
|
{
|
||||||
$path = $this->getPath($key);
|
$path = $this->getPath($key);
|
||||||
|
|
||||||
|
if (!$overwrite && file_exists($path)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
if (isset($this->locks[$key])) {
|
if (isset($this->locks[$key])) {
|
||||||
$fp = $this->locks[$key];
|
$fp = $this->locks[$key];
|
||||||
@ftruncate($fp, 0);
|
@ftruncate($fp, 0);
|
||||||
|
@ -97,6 +97,27 @@ class StoreTest extends TestCase
|
|||||||
$this->assertEquals('en9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', $res['x-content-digest'][0]);
|
$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()
|
public function testFindsAStoredEntryWithLookup()
|
||||||
{
|
{
|
||||||
$this->storeSimpleEntry();
|
$this->storeSimpleEntry();
|
||||||
|
Reference in New Issue
Block a user