Skip to content

Commit d178a9b

Browse 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

File tree

2 files changed

+124
-13
lines changed

2 files changed

+124
-13
lines changed

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

+5-9
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

+119-4
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)