Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit af0df4c

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
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 ------- d8964fb [HttpKernel] Fix that the `Store` would not save responses with the X-Content-Digest header present
2 parents 42c7975 + d8964fb commit af0df4c
Copy full SHA for af0df4c

File tree

Expand file treeCollapse file tree

2 files changed

+36
-14
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+36
-14
lines changed

‎src/Symfony/Component/HttpKernel/HttpCache/Store.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/Store.php
+15-14Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -177,19 +177,15 @@ public function write(Request $request, Response $response)
177177
$key = $this->getCacheKey($request);
178178
$storedEnv = $this->persistRequest($request);
179179

180-
// write the response body to the entity store if this is the original response
181-
if (!$response->headers->has('X-Content-Digest')) {
182-
$digest = $this->generateContentDigest($response);
180+
$digest = $this->generateContentDigest($response);
181+
$response->headers->set('X-Content-Digest', $digest);
183182

184-
if (!$this->save($digest, $response->getContent())) {
185-
throw new \RuntimeException('Unable to store the entity.');
186-
}
187-
188-
$response->headers->set('X-Content-Digest', $digest);
183+
if (!$this->save($digest, $response->getContent(), false)) {
184+
throw new \RuntimeException('Unable to store the entity.');
185+
}
189186

190-
if (!$response->headers->has('Transfer-Encoding')) {
191-
$response->headers->set('Content-Length', \strlen($response->getContent()));
192-
}
187+
if (!$response->headers->has('Transfer-Encoding')) {
188+
$response->headers->set('Content-Length', \strlen($response->getContent()));
193189
}
194190

195191
// read existing cache entries, remove non-varying, and add this one to the list
@@ -362,15 +358,20 @@ private function load($key)
362358
/**
363359
* Save data for the given key.
364360
*
365-
* @param string $key The store key
366-
* @param string $data The data to store
361+
* @param string $key The store key
362+
* @param string $data The data to store
363+
* @param bool $overwrite Whether existing data should be overwritten
367364
*
368365
* @return bool
369366
*/
370-
private function save($key, $data)
367+
private function save($key, $data, $overwrite = true)
371368
{
372369
$path = $this->getPath($key);
373370

371+
if (!$overwrite && file_exists($path)) {
372+
return true;
373+
}
374+
374375
if (isset($this->locks[$key])) {
375376
$fp = $this->locks[$key];
376377
@ftruncate($fp, 0);

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php
+21Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,27 @@ public function testSetsTheXContentDigestResponseHeaderBeforeStoring()
9797
$this->assertEquals('en9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', $res['x-content-digest'][0]);
9898
}
9999

100+
public function testDoesNotTrustXContentDigestFromUpstream()
101+
{
102+
$response = new Response('test', 200, ['X-Content-Digest' => 'untrusted-from-elsewhere']);
103+
104+
$cacheKey = $this->store->write($this->request, $response);
105+
$entries = $this->getStoreMetadata($cacheKey);
106+
list(, $res) = $entries[0];
107+
108+
$this->assertEquals('en9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', $res['x-content-digest'][0]);
109+
$this->assertEquals('en9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', $response->headers->get('X-Content-Digest'));
110+
}
111+
112+
public function testWritesResponseEvenIfXContentDigestIsPresent()
113+
{
114+
// Prime the store
115+
$this->store->write($this->request, new Response('test', 200, ['X-Content-Digest' => 'untrusted-from-elsewhere']));
116+
117+
$response = $this->store->lookup($this->request);
118+
$this->assertNotNull($response);
119+
}
120+
100121
public function testFindsAStoredEntryWithLookup()
101122
{
102123
$this->storeSimpleEntry();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.