Skip to content

Commit 9d7a4a8

Browse files
Merge branch '3.4' into 4.4
* 3.4: switch the context when validating nested forms Fix typo [HttpKernel] Fix regression where Store does not return response body correctly rework form validator tests Update AbstractController.php
2 parents f972768 + 350cc53 commit 9d7a4a8

File tree

2 files changed

+59
-13
lines changed

2 files changed

+59
-13
lines changed

HttpCache/Store.php

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ public function lookup(Request $request)
150150
}
151151

152152
$headers = $match[1];
153-
if (file_exists($body = $this->getPath($headers['x-content-digest'][0]))) {
154-
return $this->restoreResponse($headers, $body);
153+
if (file_exists($path = $this->getPath($headers['x-content-digest'][0]))) {
154+
return $this->restoreResponse($headers, $path);
155155
}
156156

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

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

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

185-
if (!$response->headers->has('Transfer-Encoding')) {
186-
$response->headers->set('Content-Length', \strlen($response->getContent()));
193+
if (!$this->save($digest, $response->getContent(), false)) {
194+
throw new \RuntimeException('Unable to store the entity.');
195+
}
196+
197+
if (!$response->headers->has('Transfer-Encoding')) {
198+
$response->headers->set('Content-Length', \strlen($response->getContent()));
199+
}
187200
}
188201

189202
// read existing cache entries, remove non-varying, and add this one to the list
@@ -448,15 +461,15 @@ private function persistResponse(Response $response): array
448461
/**
449462
* Restores a Response from the HTTP headers and body.
450463
*/
451-
private function restoreResponse(array $headers, string $body = null): Response
464+
private function restoreResponse(array $headers, string $path = null): Response
452465
{
453466
$status = $headers['X-Status'][0];
454467
unset($headers['X-Status']);
455468

456-
if (null !== $body) {
457-
$headers['X-Body-File'] = [$body];
469+
if (null !== $path) {
470+
$headers['X-Body-File'] = [$path];
458471
}
459472

460-
return new Response($body, $status, $headers);
473+
return new Response($path, $status, $headers);
461474
}
462475
}

Tests/HttpCache/StoreTest.php

Lines 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 honor 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)