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 symfony/symfony#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
-------

2639c4353a [Response] `getMaxAge()` returns non-negative integer

Author: fabpot

HttpCache/HttpCache.php

@@ -718,7 +718,11 @@ private function mayServeStaleWhileRevalidate(Response $entry): bool
             $timeout = $this->options['stale_while_revalidate'];
         }
 
-        return abs($entry->getTtl() ?? 0) < $timeout;
+        $age = $entry->getAge();
+        $maxAge = $entry->getMaxAge() ?? 0;
+        $ttl = $maxAge - $age;
+
+        return abs($ttl) < $timeout;
     }
 
     /**

Tests/EventListener/SessionListenerTest.php

@@ -590,6 +590,30 @@ public function testResponseHeadersMaxAgeAndExpiresDefaultValuesIfSessionStarted
         $this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
     }
 
+    public function testPrivateResponseMaxAgeIsRespectedIfSessionStarted()
+    {
+        $kernel = $this->createMock(HttpKernelInterface::class);
+
+        $session = $this->createMock(Session::class);
+        $session->expects($this->once())->method('getUsageIndex')->willReturn(1);
+        $request = new Request([], [], [], [], [], ['SERVER_PROTOCOL' => 'HTTP/1.0']);
+        $request->setSession($session);
+
+        $response = new Response();
+        $response->headers->set('Cache-Control', 'no-cache');
+        $response->prepare($request);
+
+        $listener = new SessionListener(new Container());
+        $listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $response));
+
+        $this->assertSame(0, $response->getMaxAge());
+        $this->assertFalse($response->headers->hasCacheControlDirective('public'));
+        $this->assertTrue($response->headers->hasCacheControlDirective('private'));
+        $this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
+        $this->assertLessThanOrEqual(new \DateTimeImmutable('now', new \DateTimeZone('UTC')), new \DateTimeImmutable($response->headers->get('Expires')));
+        $this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
+    }
+
     public function testSurrogateMainRequestIsPublic()
     {
         $session = $this->createMock(Session::class);

composer.json

@@ -20,7 +20,7 @@
         "symfony/deprecation-contracts": "^2.1|^3",
         "symfony/error-handler": "^4.4|^5.0|^6.0",
         "symfony/event-dispatcher": "^5.0|^6.0",
-        "symfony/http-foundation": "^5.3.7|^6.0",
+        "symfony/http-foundation": "^5.4.21|^6.2.7",
         "symfony/polyfill-ctype": "^1.8",
         "symfony/polyfill-php73": "^1.9",
         "symfony/polyfill-php80": "^1.16",