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 176e769

Browse filesBrowse files
mpdudefabpot
authored andcommitted
[HttpKernel] Fix regression where Store does not return response body correctly
1 parent f87c993 commit 176e769
Copy full SHA for 176e769

File tree

2 files changed

+60
-14
lines changed
Filter options

2 files changed

+60
-14
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/Store.php
+27-14Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,8 @@ public function lookup(Request $request)
152152
}
153153

154154
$headers = $match[1];
155-
if (file_exists($body = $this->getPath($headers['x-content-digest'][0]))) {
156-
return $this->restoreResponse($headers, $body);
155+
if (file_exists($path = $this->getPath($headers['x-content-digest'][0]))) {
156+
return $this->restoreResponse($headers, $path);
157157
}
158158

159159
// TODO the metaStore referenced an entity that doesn't exist in
@@ -177,15 +177,28 @@ public function write(Request $request, Response $response)
177177
$key = $this->getCacheKey($request);
178178
$storedEnv = $this->persistRequest($request);
179179

180-
$digest = $this->generateContentDigest($response);
181-
$response->headers->set('X-Content-Digest', $digest);
180+
if ($response->headers->has('X-Body-File')) {
181+
// Assume the response came from disk, but at least perform some safeguard checks
182+
if (!$response->headers->has('X-Content-Digest')) {
183+
throw new \RuntimeException('A restored response must have the X-Content-Digest header.');
184+
}
182185

183-
if (!$this->save($digest, $response->getContent(), false)) {
184-
throw new \RuntimeException('Unable to store the entity.');
185-
}
186+
$digest = $response->headers->get('X-Content-Digest');
187+
if ($this->getPath($digest) !== $response->headers->get('X-Body-File')) {
188+
throw new \RuntimeException('X-Body-File and X-Content-Digest do not match.');
189+
}
190+
// Everything seems ok, omit writing content to disk
191+
} else {
192+
$digest = $this->generateContentDigest($response);
193+
$response->headers->set('X-Content-Digest', $digest);
186194

187-
if (!$response->headers->has('Transfer-Encoding')) {
188-
$response->headers->set('Content-Length', \strlen($response->getContent()));
195+
if (!$this->save($digest, $response->getContent(), false)) {
196+
throw new \RuntimeException('Unable to store the entity.');
197+
}
198+
199+
if (!$response->headers->has('Transfer-Encoding')) {
200+
$response->headers->set('Content-Length', \strlen($response->getContent()));
201+
}
189202
}
190203

191204
// read existing cache entries, remove non-varying, and add this one to the list
@@ -477,19 +490,19 @@ private function persistResponse(Response $response)
477490
* Restores a Response from the HTTP headers and body.
478491
*
479492
* @param array $headers An array of HTTP headers for the Response
480-
* @param string $body The Response body
493+
* @param string $path Path to the Response body
481494
*
482495
* @return Response
483496
*/
484-
private function restoreResponse($headers, $body = null)
497+
private function restoreResponse($headers, $path = null)
485498
{
486499
$status = $headers['X-Status'][0];
487500
unset($headers['X-Status']);
488501

489-
if (null !== $body) {
490-
$headers['X-Body-File'] = [$body];
502+
if (null !== $path) {
503+
$headers['X-Body-File'] = [$path];
491504
}
492505

493-
return new Response($body, $status, $headers);
506+
return new Response($path, $status, $headers);
494507
}
495508
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php
+33Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,39 @@ public function testWritesResponseEvenIfXContentDigestIsPresent()
118118
$this->assertNotNull($response);
119119
}
120120

121+
public function testWritingARestoredResponseDoesNotCorruptCache()
122+
{
123+
/*
124+
* This covers the regression reported in https://github.com/symfony/symfony/issues/37174.
125+
*
126+
* A restored response does *not* load the body, but only keep the file path in a special X-Body-File
127+
* header. For reasons (?), the file path was also used as the restored response body.
128+
* It would be up to others (HttpCache...?) to hohor this header and actually load the response content
129+
* from there.
130+
*
131+
* When a restored response was stored again, the Store itself would ignore the header. In the first
132+
* step, this would compute a new Content Digest based on the file path in the restored response body;
133+
* this is covered by "Checkpoint 1" below. But, since the X-Body-File header was left untouched (Checkpoint 2), downstream
134+
* code (HttpCache...) would not immediately notice.
135+
*
136+
* Only upon performing the lookup for a second time, we'd get a Response where the (wrong) Content Digest
137+
* is also reflected in the X-Body-File header, this time also producing wrong content when the downstream
138+
* evaluates it.
139+
*/
140+
$this->store->write($this->request, $this->response);
141+
$digest = $this->response->headers->get('X-Content-Digest');
142+
$path = $this->getStorePath($digest);
143+
144+
$response = $this->store->lookup($this->request);
145+
$this->store->write($this->request, $response);
146+
$this->assertEquals($digest, $response->headers->get('X-Content-Digest')); // Checkpoint 1
147+
$this->assertEquals($path, $response->headers->get('X-Body-File')); // Checkpoint 2
148+
149+
$response = $this->store->lookup($this->request);
150+
$this->assertEquals($digest, $response->headers->get('X-Content-Digest'));
151+
$this->assertEquals($path, $response->headers->get('X-Body-File'));
152+
}
153+
121154
public function testFindsAStoredEntryWithLookup()
122155
{
123156
$this->storeSimpleEntry();

0 commit comments

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