8000 bug #48880 [Response] `getMaxAge()` returns non-negative integer (pkr… · symfony/symfony@29d73d7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 29d73d7

Browse files
fabpotfabpot
committed
bug #48880 [Response] getMaxAge() returns non-negative integer (pkruithof, fabpot)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Response] `getMaxAge()` returns non-negative integer | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Refs #48651 (comment) | License | MIT | Doc PR | The `max-age` directive should be a non-negative integer, see [MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control): > The max-age=N request directive indicates that the client allows a stored response that is generated on the origin server within N seconds — where N may be any non-negative integer (including 0). In case the value is negative, it's encouraged to be treated as 0: > In other words, for any max-age value that isn't an integer or isn't non-negative, the caching behavior that's encouraged is to treat the value as if it were 0. In my case, it lead to a response that was `private,no-cache` but with an `Expires` header set in the future. Not every browser handled this inconsistency the same, which eventually led to authentication issues (see linked comment for a more elaborate explanation). Commits ------- 2639c43 [Response] `getMaxAge()` returns non-negative integer
2 parents 41246d4 + 2639c43 commit 29d73d7

File tree

5 files changed

+38
-9
lines changed

5 files changed

+38
-9
lines changed

‎src/Symfony/Component/HttpFoundation/Response.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -774,8 +774,10 @@ public function getMaxAge(): ?int
774774
return (int) $this->headers->getCacheControlDirective('max-age');
775775
}
776776

777-
if (null !== $this->getExpires()) {
778-
return (int) $this->getExpires()->format('U') - (int) $this->getDate()->format('U');
777+
if (null !== $expires = $this->getExpires()) {
778+
$maxAge = (int) $expires->format('U') - (int) $this->getDate()->format('U');
779+
780+
return max($maxAge, 0);
779781
}
780782

781783
return null;
@@ -819,7 +821,7 @@ public function setSharedMaxAge(int $value): object
819821
*
820822
* It returns null when no freshness information is present in the response.
821823
*
822-
* When the responses TTL is <= 0, the response may not be served from cache without first
824+
* When the response's TTL is 0, the response may not be served from cache without first
823825
* revalidating with the origin.
824826
*
825827
* @final
@@ -828,7 +830,7 @@ public function getTtl(): ?int
828830
{
829831
$maxAge = $this->getMaxAge();
830832

831-
return null !== $maxAge ? $maxAge - $this->getAge() : null;
833+
return null !== $maxAge ? max($maxAge - $this->getAge(), 0) : null;
832834
}
833835

834836
/**

‎src/Symfony/Component/HttpFoundation/Tests/ResponseTest.php

Lines changed: 2 additions & 3 deletions
Original file 8000 line numberDiff line numberDiff line change
@@ -353,9 +353,8 @@ public function testGetMaxAge()
353353
$this->assertEquals(3600, $response->getMaxAge(), '->getMaxAge() falls back to Expires when no max-age or s-maxage directive present');
354354

355355
$response = new Response();
356-
$response->headers->set('Cache-Control', 'must-revalidate');
357356
$response->headers->set('Expires', -1);
358-
$this->assertLessThanOrEqual(time() - 2 * 86400, $response->getExpires()->format('U'));
357+
$this->assertSame(0, $response->getMaxAge());
359358

360359
$response = new Response();
361360
$this->assertNull($response->getMaxAge(), '->getMaxAge() returns null if no freshness information available');
@@ -436,7 +435,7 @@ public function testGetTtl()
436435

437436
$response = new Response();
438437
$response->headers->set('Expires', $this->createDateTimeOneHourAgo()->format(\DATE_RFC2822));
439-
$this->assertLessThan(0, $response->getTtl(), '->getTtl() returns negative values when Expires is in past');
438+
$this->assertSame(0, $response->getTtl(), '->getTtl() returns zero when Expires is in past');
440439

441440
$response = new Response();
442441
$response->headers->set('Expires', $response->getDate()->format(\DATE_RFC2822));

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,11 @@ private function mayServeStaleWhileRevalidate(Response $entry): bool
718718
$timeout = $this->options['stale_while_revalidate'];
719719
}
720720

721-
return abs($entry->getTtl() ?? 0) < $timeout;
721+
$age = $entry->getAge();
722+
$maxAge = $entry->getMaxAge() ?? 0;
723+
$ttl = $maxAge - $age;
724+
725+
return abs($ttl) < $timeout;
722726
}
723727

724728
/**

‎src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,30 @@ public function testResponseHeadersMaxAgeAndExpiresDefaultValuesIfSessionStarted
590590
$this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
591591
}
592592

593+
public function testPrivateResponseMaxAgeIsRespectedIfSessionStarted()
594+
{
595+
$kernel = $this->createMock(HttpKernelInterface::class);
596+
597+
$session = $this->createMock(Session::class);
598+
$session->expects($this->once())->method('getUsageIndex')->willReturn(1);
599+
$request = new Request([], [], [], [], [], ['SERVER_PROTOCOL' => 'HTTP/1.0']);
600+
$request->setSession($session);
601+
602+
$response = new Response();
603+
$response->headers->set('Cache-Control', 'no-cache');
604+
$response->prepare($request);
605+
606+
$listener = new SessionListener(new Container());
607+
$listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $response));
608+
609+
$this->assertSame(0, $response->getMaxAge());
610+
$this->assertFalse($response->headers->hasCacheControlDirective('public'));
611+
$this->assertTrue($response->headers->hasCacheControlDirective('private'));
612+
$this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
613+
$this->assertLessThanOrEqual(new \DateTimeImmutable('now', new \DateTimeZone('UTC')), new \DateTimeImmutable($response->headers->get('Expires')));
614+
$this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
615+
}
616+
593617
public function testSurrogateMainRequestIsPublic()
594618
{
595619
$session = $this->createMock(Session::class);

‎src/Symfony/Component/HttpKernel/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"symfony/deprecation-contracts": "^2.1|^3",
2121
"symfony/error-handler": "^4.4|^5.0|^6.0",
2222
"symfony/event-dispatcher": "^5.0|^6.0",
23-
"symfony/http-foundation": "^5.3.7|^6.0",
23+
"symfony/http-foundation": "^5.4.21|^6.2.7",
2424
"symfony/polyfill-ctype": "^1.8",
2525
"symfony/polyfill-php73": "^1.9",
2626
"symfony/polyfill-php80": "^1.16",

0 commit comments

Comments
 (0)
0