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 29d73d7

Browse filesBrowse 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
Copy full SHA for 29d73d7

File tree

5 files changed

+38
-9
lines changed
Filter options

5 files changed

+38
-9
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Response.php
+6-4Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Tests/ResponseTest.php
+2-3Lines changed: 2 additions & 3 deletions
Original file 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
+5-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php
+24Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/composer.json
+1-1Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.