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 d178a9b

Browse filesBrowse files
fabpotfabpot
committed
bug #24243 HttpCache does not consider ESI resources in HEAD requests (mpdude)
This PR was squashed before being merged into the 2.7 branch (closes #24243). Discussion ---------- HttpCache does not consider ESI resources in HEAD requests | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Due to this shortcut: https://github.com/symfony/symfony/blob/3b42d8859ea98fdd17012e21f774f55d33cccc3f/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php#L634-L642 ... the `HttpCache` never looks at the response body for `HEAD` requests. This makes it completely miss ESI-related tweaks like computing the correct TTL, removing validation headers or updating the `Content-Length`. From RFC2616 (https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.4): > The HEAD method is identical to GET except that the server MUST NOT return a message-body in the response. The metainformation contained in the HTTP headers in response to a HEAD request SHOULD be identical to the information sent in response to a GET request. Although it says "SHOULD", I think it can be misleading at best when HEAD requests do, for example, return different (greater) `s-maxage` values than a corresponding GET request. Commits ------- 4dd0e53 HttpCache does not consider ESI resources in HEAD requests
2 parents 9ed6dd4 + 4dd0e53 commit d178a9b
Copy full SHA for d178a9b

File tree

2 files changed

+124
-13
lines changed
Filter options

2 files changed

+124
-13
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
+5-9Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -633,14 +633,6 @@ protected function store(Request $request, Response $response)
633633
*/
634634
private function restoreResponseBody(Request $request, Response $response)
635635
{
636-
if ($request->isMethod('HEAD') || 304 === $response->getStatusCode()) {
637-
$response->setContent(null);
638-
$response->headers->remove('X-Body-Eval');
639-
$response->headers->remove('X-Body-File');
640-
641-
return;
642-
}
643-
644636
if ($response->headers->has('X-Body-Eval')) {
645637
ob_start();
646638

@@ -656,7 +648,11 @@ private function restoreResponseBody(Request $request, Response $response)
656648
$response->headers->set('Content-Length', strlen($response->getContent()));
657649
}
658650
} elseif ($response->headers->has('X-Body-File')) {
659-
$response->setContent(file_get_contents($response->headers->get('X-Body-File')));
651+
// Response does not include possibly dynamic content (ESI, SSI), so we need
652+
// not handle the content for HEAD requests
653+
if (!$request->isMethod('HEAD')) {
654+
$response->setContent(file_get_contents($response->headers->get('X-Body-File')));
655+
}
660656
} else {
661657
return;
662658
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php
+119-4Lines changed: 119 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,7 @@ public function testEsiCacheSendsTheLowestTtl()
11331133
array(
11341134
'status' => 200,
11351135
'body' => 'Hello World!',
1136-
'headers' => array('Cache-Control' => 's-maxage=300'),
1136+
'headers' => array('Cache-Control' => 's-maxage=200'),
11371137
),
11381138
array(
11391139
'status' => 200,
@@ -1147,8 +1147,33 @@ public function testEsiCacheSendsTheLowestTtl()
11471147
$this->request('GET', '/', array(), array(), true);
11481148
$this->assertEquals('Hello World! My name is Bobby.', $this->response->getContent());
11491149

1150-
// check for 100 or 99 as the test can be executed after a second change
1151-
$this->assertTrue(in_array($this->response->getTtl(), array(99, 100)));
1150+
$this->assertEquals(100, $this->response->getTtl());
1151+
}
1152+
1153+
public function testEsiCacheSendsTheLowestTtlForHeadRequests()
1154+
{
1155+
$responses = array(
1156+
array(
1157+
'status' => 200,
1158+
'body' => 'I am a long-lived master response, but I embed a short-lived resource: <esi:include src="/foo" />',
1159+
'headers' => array(
1160+
'Cache-Control' => 's-maxage=300',
1161+
'Surrogate-Control' => 'content="ESI/1.0"',
1162+
),
1163+
),
1164+
array(
1165+
'status' => 200,
1166+
'body' => 'I am a short-lived resource',
1167+
'headers' => array('Cache-Control' => 's-maxage=100'),
1168+
),
1169+
);
1170+
1171+
$this->setNextResponses($responses);
1172+
1173+
$this->request('HEAD', '/', array(), array(), true);
1174+
1175+
$this->assertEmpty($this->response->getContent());
1176+
$this->assertEquals(100, $this->response->getTtl());
11521177
}
11531178

11541179
public function testEsiCacheForceValidation()
@@ -1184,6 +1209,37 @@ public function testEsiCacheForceValidation()
11841209
$this->assertTrue($this->response->headers->hasCacheControlDirective('no-cache'));
11851210
}
11861211

1212+
public function testEsiCacheForceValidationForHeadRequests()
1213+
{
1214+
$responses = array(
1215+
array(
1216+
'status' => 200,
1217+
'body' => 'I am the master response and use expiration caching, but I embed another resource: <esi:include src="/foo" />',
1218+
'headers' => array(
1219+
'Cache-Control' => 's-maxage=300',
1220+
'Surrogate-Control' => 'content="ESI/1.0"',
1221+
),
1222+
),
1223+
array(
1224+
'status' => 200,
1225+
'body' => 'I am the embedded resource and use validation caching',
1226+
'headers' => array('ETag' => 'foobar'),
1227+
),
1228+
);
1229+
1230+
$this->setNextResponses($responses);
1231+
1232+
$this->request('HEAD', '/', array(), array(), true);
1233+
1234+
// The response has been assembled from expiration and validation based resources
1235+
// This can neither be cached nor revalidated, so it should be private/no cache
1236+
$this->assertEmpty($this->response->getContent());
1237+
$this->assertNull($this->response->getTtl());
1238+
$this->assertTrue($this->response->mustRevalidate());
1239+
$this->assertTrue($this->response->headers->hasCacheControlDirective('private'));
1240+
$this->assertTrue($this->response->headers->hasCacheControlDirective('no-cache'));
1241+
}
1242+
11871243
public function testEsiRecalculateContentLengthHeader()
11881244
{
11891245
$responses = array(
@@ -1192,7 +1248,6 @@ public function testEsiRecalculateContentLengthHeader()
11921248
'body' => '<esi:include src="/foo" />',
11931249
'headers' => array(
11941250
'Content-Length' => 26,
1195-
'Cache-Control' => 's-maxage=300',
11961251
'Surrogate-Control' => 'content="ESI/1.0"',
11971252
),
11981253
),
@@ -1210,6 +1265,37 @@ public function testEsiRecalculateContentLengthHeader()
12101265
$this->assertEquals(12, $this->response->headers->get('Content-Length'));
12111266
}
12121267

1268+
public function testEsiRecalculateContentLengthHeaderForHeadRequest()
1269+
{
1270+
$responses = array(
1271+
array(
1272+
'status' => 200,
1273+
'body' => '<esi:include src="/foo" />',
1274+
'headers' => array(
1275+
'Content-Length' => 26,
1276+
'Surrogate-Control' => 'content="ESI/1.0"',
1277+
),
1278+
),
1279+
array(
1280+
'status' => 200,
1281+
'body' => 'Hello World!',
1282+
'headers' => array(),
1283+
),
1284+
);
1285+
1286+
$this->setNextResponses($responses);
1287+
1288+
$this->request('HEAD', '/', array(), array(), true);
1289+
1290+
// https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.13
1291+
// "The Content-Length entity-header field indicates the size of the entity-body,
1292+
// in decimal number of OCTETs, sent to the recipient or, in the case of the HEAD
1293+
// method, the size of the entity-body that would have been sent had the request
1294+
// been a GET."
1295+
$this->assertEmpty($this->response->getContent());
1296+
$this->assertEquals(12, $this->response->headers->get('Content-Length'));
1297+
}
1298+
12131299
public function testClientIpIsAlwaysLocalhostForForwardedRequests()
12141300
{
12151301
$this->setNextResponse();
@@ -1301,6 +1387,35 @@ public function testEsiCacheRemoveValidationHeadersIfEmbeddedResponses()
13011387
$this->assertNull($this->response->getLastModified());
13021388
}
13031389

1390+
public function testEsiCacheRemoveValidationHeadersIfEmbeddedResponsesAndHeadRequest()
1391+
{
1392+
$time = \DateTime::createFromFormat('U', time());
1393+
1394+
$responses = array(
1395+
array(
1396+
'status' => 200,
1397+
'body' => '<esi:include src="/hey" />',
1398+
'headers' => array(
1399+
'Surrogate-Control' => 'content="ESI/1.0"',
1400+
'ETag' => 'hey',
1401+
'Last-Modified' => $time->format(DATE_RFC2822),
1402+
),
1403+
),
1404+
array(
1405+
'status' => 200,
1406+
'body' => 'Hey!',
1407+
'headers' => array(),
1408+
),
1409+
);
1410+
1411+
$this->setNextResponses($responses);
1412+
1413+
$this->request('HEAD', '/', array(), array(), true);
1414+
$this->assertEmpty($this->response->getContent());
1415+
$this->assertNull($this->response->getETag());
1416+
$this->assertNull($this->response->getLastModified());
1417+
}
1418+
13041419
public function testDoesNotCacheOptionsRequest()
13051420
{
13061421
$this->setNextResponse(200, array('Cache-Control' => 'public, s-maxage=60'), 'get');

0 commit comments

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