From dd956e4c6ca1cc31b3db2bd76b9d85047e8671dc Mon Sep 17 00:00:00 2001 From: James Read Date: Thu, 7 Mar 2024 22:01:55 +0800 Subject: [PATCH 1/6] Testing for empty body on cache hit Adding tests ETag and Last-Modified to ensure the response body shouldn't send data when their is a cache hit. --- tests/CacheTest.php | 58 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/tests/CacheTest.php b/tests/CacheTest.php index 64fab81..ef31e24 100644 --- a/tests/CacheTest.php +++ b/tests/CacheTest.php @@ -16,11 +16,18 @@ use Slim\Psr7\Factory\ResponseFactory; use Slim\Psr7\Factory\ServerRequestFactory; +use Slim\Psr7\Factory\StreamFactory; use function gmdate; use function time; class CacheTest extends TestCase { + private function createCache(string $type = 'privte', int $maxAge = 86400, bool $mustRevalidate = false): Cache + { + return new Cache(new StreamFactory(), $type, $maxAge, $mustRevalidate); + } + + public function requestFactory(): ServerRequestInterface { $serverRequestFactory = new ServerRequestFactory(); @@ -64,7 +71,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface public function testCacheControlHeader() { - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory(); $res = $cache->process($req, $this->createRequestHandler()); @@ -76,7 +83,7 @@ public function testCacheControlHeader() public function testCacheControlHeaderWithMustRevalidate() { - $cache = new Cache('private', 86400, true); + $cache = $this->createCache('private', 86400, true); $req = $this->requestFactory(); $res = $cache->process($req, $this->createRequestHandler()); @@ -88,7 +95,7 @@ public function testCacheControlHeaderWithMustRevalidate() public function testCacheControlHeaderWithZeroMaxAge() { - $cache = new Cache('private', 0, false); + $cache = $this->createCache('private', 0, false); $req = $this->requestFactory(); $res = $cache->process($req, $this->createRequestHandler()); @@ -100,7 +107,7 @@ public function testCacheControlHeaderWithZeroMaxAge() public function testCacheControlHeaderDoesNotOverrideExistingHeader() { - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory(); $res = $this->createResponse()->withHeader('Cache-Control', 'no-cache,no-store'); @@ -116,7 +123,7 @@ public function testLastModifiedWithCacheHit() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 86400); - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); @@ -131,7 +138,7 @@ public function testLastModifiedWithCacheHitAndNewerDate() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 172800); // <-- Newer date - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); @@ -145,7 +152,7 @@ public function testLastModifiedWithCacheHitAndOlderDate() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now); // <-- Older date - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); @@ -159,7 +166,7 @@ public function testLastModifiedWithCacheMiss() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now - 86400); - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); @@ -172,7 +179,7 @@ public function testETagWithCacheHit() { $etag = 'abc'; $ifNoneMatch = 'abc'; - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch); $res = $this->createResponse()->withHeader('Etag', $etag); @@ -185,7 +192,7 @@ public function testETagWithCacheMiss() { $etag = 'abc'; $ifNoneMatch = 'xyz'; - $cache = new Cache('public', 86400); + $cache = $this->createCache('public', 86400); $req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch); $res = $this->createResponse()->withHeader('Etag', $etag); @@ -193,4 +200,35 @@ public function testETagWithCacheMiss() $this->assertEquals(200, $res->getStatusCode()); } + + public function testETagReturnsNoBodyOnCacheHit(): void + { + $etag = 'abc'; + $cache = $this->createCache(); + $req = $this->requestFactory()->withHeader('If-None-Match', $etag); + + $res = $this->createResponse()->withHeader('Etag', $etag); + $res->getBody()->write('payload data'); + $res = $cache->process($req, $this->createRequestHandler($res)); + + self::assertSame(304, $res->getStatusCode()); + self::assertSame('', (string) $res->getBody()); + } + + public function testLastModifiedReturnsNoBodyOnCacheHit(): void + { + $now = time() + 86400; + $lastModified = gmdate('D, d M Y H:i:s T', $now); + $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now); + $cache = $this->createCache(); + + $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); + $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); + $res->getBody()->write('payload data'); + + $res = $cache->process($req, $this->createRequestHandler($res)); + + self::assertEquals(304, $res->getStatusCode()); + self::assertSame('', (string) $res->getBody()); + } } From 674fd7d7f3c1704647abc51a57907ff7427f4bdf Mon Sep 17 00:00:00 2001 From: James Read Date: Thu, 7 Mar 2024 22:10:01 +0800 Subject: [PATCH 2/6] Replace response body on cache hit Replacing the response body on cache hits to reduce data transmission --- src/Cache.php | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Cache.php b/src/Cache.php index 769d8ee..bb7b63e 100644 --- a/src/Cache.php +++ b/src/Cache.php @@ -11,9 +11,11 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Slim\Psr7\Factory\StreamFactory; use function in_array; use function is_array; use function is_numeric; @@ -45,6 +47,11 @@ class Cache implements MiddlewareInterface */ protected $mustRevalidate; + /** + * @var StreamFactoryInterface + */ + protected $streamFactory; + /** * Create new HTTP cache * @@ -52,11 +59,16 @@ class Cache implements MiddlewareInterface * @param int $maxAge The maximum age of client-side cache * @param bool $mustRevalidate must-revalidate */ - public function __construct(string $type = 'private', int $maxAge = 86400, bool $mustRevalidate = false) - { + public function __construct( + StreamFactoryInterface $streamFactory, + string $type = 'private', + int $maxAge = 86400, + bool $mustRevalidate = false + ) { $this->type = $type; $this->maxAge = $maxAge; $this->mustRevalidate = $mustRevalidate; + $this->streamFactory = $streamFactory; } /** @@ -101,7 +113,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface if ($ifNoneMatch) { $etagList = preg_split('@\s*,\s*@', $ifNoneMatch); if (is_array($etagList) && (in_array($etag, $etagList) || in_array('*', $etagList))) { - return $response->withStatus(304); + return $response->withStatus(304) + ->withBody($this->streamFactory->createStream('')); } } } @@ -118,7 +131,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface $ifModifiedSince = $request->getHeaderLine('If-Modified-Since'); if ($ifModifiedSince && $lastModified <= strtotime($ifModifiedSince)) { - return $response->withStatus(304); + return $response->withStatus(304) + ->withBody($this->streamFactory->createStream('')); } } From 569323459a663aefe4363af245ef95f1d5845bbb Mon Sep 17 00:00:00 2001 From: James Read Date: Thu, 8 Aug 2024 13:19:11 +0100 Subject: [PATCH 3/6] Removing unused import Removig StreamFactory as it's not used. --- src/Cache.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Cache.php b/src/Cache.php index bb7b63e..61d424c 100644 --- a/src/Cache.php +++ b/src/Cache.php @@ -15,7 +15,6 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; -use Slim\Psr7\Factory\StreamFactory; use function in_array; use function is_array; use function is_numeric; From 2821e9f91602114de3f1ab0cd4fc5d9023c569ed Mon Sep 17 00:00:00 2001 From: Rob Allen Date: Sun, 2 Nov 2025 16:04:17 +0000 Subject: [PATCH 4/6] Make StreamFactory optional This means that we don't break BC. --- src/Cache.php | 27 +++++++++++++++++---------- tests/CacheTest.php | 38 +++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/Cache.php b/src/Cache.php index 61d424c..a1e67a1 100644 --- a/src/Cache.php +++ b/src/Cache.php @@ -47,22 +47,23 @@ class Cache implements MiddlewareInterface protected $mustRevalidate; /** - * @var StreamFactoryInterface + * @var StreamFactoryInterface|null */ protected $streamFactory; /** * Create new HTTP cache * - * @param string $type The cache type: "public" or "private" - * @param int $maxAge The maximum age of client-side cache - * @param bool $mustRevalidate must-revalidate + * @param string $type The cache type: "public" or "private" + * @param int $maxAge The maximum age of client-side cache + * @param bool $mustRevalidate must-revalidate + * @param StreamFactoryInterface|null $streamFactory Will clear body of a 304 if provided */ public function __construct( - StreamFactoryInterface $streamFactory, string $type = 'private', int $maxAge = 86400, - bool $mustRevalidate = false + bool $mustRevalidate = false, + ?StreamFactoryInterface $streamFactory = null ) { $this->type = $type; $this->maxAge = $maxAge; @@ -112,8 +113,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface if ($ifNoneMatch) { $etagList = preg_split('@\s*,\s*@', $ifNoneMatch); if (is_array($etagList) && (in_array($etag, $etagList) || in_array('*', $etagList))) { - return $response->withStatus(304) - ->withBody($this->streamFactory->createStream('')); + $response = $response->withStatus(304); + if ($this->streamFactory) { + $response = $response->withBody($this->streamFactory->createStream('')); + } + return $response; } } } @@ -130,8 +134,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface $ifModifiedSince = $request->getHeaderLine('If-Modified-Since'); if ($ifModifiedSince && $lastModified <= strtotime($ifModifiedSince)) { - return $response->withStatus(304) - ->withBody($this->streamFactory->createStream('')); + $response = $response->withStatus(304); + if ($this->streamFactory) { + $response = $response->withBody($this->streamFactory->createStream('')); + } + return $response; } } diff --git a/tests/CacheTest.php b/tests/CacheTest.php index 8805c9b..635d662 100644 --- a/tests/CacheTest.php +++ b/tests/CacheTest.php @@ -22,12 +22,12 @@ class CacheTest extends TestCase { - private function createCache(string $type = 'privte', int $maxAge = 86400, bool $mustRevalidate = false): Cache + private function createCache(string $type, int $maxAge, bool $mustRevalidate, bool $withStreamFactory): Cache { - return new Cache(new StreamFactory(), $type, $maxAge, $mustRevalidate); + $streamFactory = $withStreamFactory ? new StreamFactory() : null; + return new Cache($type, $maxAge, $mustRevalidate, $streamFactory); } - public function requestFactory(): ServerRequestInterface { $serverRequestFactory = new ServerRequestFactory(); @@ -71,7 +71,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface public function testCacheControlHeader() { - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory(); $res = $cache->process($req, $this->createRequestHandler(null)); @@ -83,7 +83,7 @@ public function testCacheControlHeader() public function testCacheControlHeaderWithMustRevalidate() { - $cache = $this->createCache('private', 86400, true); + $cache = $this->createCache('private', 86400, true, false); $req = $this->requestFactory(); $res = $cache->process($req, $this->createRequestHandler(null)); @@ -95,7 +95,7 @@ public function testCacheControlHeaderWithMustRevalidate() public function testCacheControlHeaderWithZeroMaxAge() { - $cache = $this->createCache('private', 0, false); + $cache = $this->createCache('private', 0, false, false); $req = $this->requestFactory(); $res = $cache->process($req, $this->createRequestHandler(null)); @@ -107,7 +107,7 @@ public function testCacheControlHeaderWithZeroMaxAge() public function testCacheControlHeaderDoesNotOverrideExistingHeader() { - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory(); $res = $this->createResponse()->withHeader('Cache-Control', 'no-cache,no-store'); @@ -123,14 +123,16 @@ public function testLastModifiedWithCacheHit() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 86400); - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); + $res->getBody()->write('payload data'); $res = $cache->process($req, $this->createRequestHandler($res)); $this->assertEquals(304, $res->getStatusCode()); + self::assertSame('payload data', (string) $res->getBody()); } public function testLastModifiedWithCacheHitAndNewerDate() @@ -138,7 +140,7 @@ public function testLastModifiedWithCacheHitAndNewerDate() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 172800); // <-- Newer date - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); @@ -152,7 +154,7 @@ public function testLastModifiedWithCacheHitAndOlderDate() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now); // <-- Older date - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); @@ -166,7 +168,7 @@ public function testLastModifiedWithCacheMiss() $now = time(); $lastModified = gmdate('D, d M Y H:i:s T', $now + 86400); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now - 86400); - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); @@ -179,20 +181,22 @@ public function testETagWithCacheHit() { $etag = 'abc'; $ifNoneMatch = 'abc'; - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch); $res = $this->createResponse()->withHeader('Etag', $etag); + $res->getBody()->write('payload data'); $res = $cache->process($req, $this->createRequestHandler($res)); $this->assertEquals(304, $res->getStatusCode()); + self::assertSame('payload data', (string) $res->getBody()); } public function testETagWithCacheMiss() { $etag = 'abc'; $ifNoneMatch = 'xyz'; - $cache = $this->createCache('public', 86400); + $cache = $this->createCache('public', 86400, false, false); $req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch); $res = $this->createResponse()->withHeader('Etag', $etag); @@ -201,10 +205,10 @@ public function testETagWithCacheMiss() $this->assertEquals(200, $res->getStatusCode()); } - public function testETagReturnsNoBodyOnCacheHit(): void + public function testETagReturnsNoBodyOnCacheHitWhenAStreamFactoryIsProvided(): void { $etag = 'abc'; - $cache = $this->createCache(); + $cache = $this->createCache('private', 86400, false, true); $req = $this->requestFactory()->withHeader('If-None-Match', $etag); $res = $this->createResponse()->withHeader('Etag', $etag); @@ -215,12 +219,12 @@ public function testETagReturnsNoBodyOnCacheHit(): void self::assertSame('', (string) $res->getBody()); } - public function testLastModifiedReturnsNoBodyOnCacheHit(): void + public function testLastModifiedReturnsNoBodyOnCacheHitWhenAStreamFactoryIsProvided(): void { $now = time() + 86400; $lastModified = gmdate('D, d M Y H:i:s T', $now); $ifModifiedSince = gmdate('D, d M Y H:i:s T', $now); - $cache = $this->createCache(); + $cache = $this->createCache('private', 86400, false, true); $req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince); $res = $this->createResponse()->withHeader('Last-Modified', $lastModified); From 063404890072433cdaa7f45b00b051388f9650d4 Mon Sep 17 00:00:00 2001 From: Rob Allen Date: Sun, 2 Nov 2025 19:20:06 +0000 Subject: [PATCH 5/6] Move private method to bottom of class --- tests/CacheTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/CacheTest.php b/tests/CacheTest.php index 635d662..2d73f87 100644 --- a/tests/CacheTest.php +++ b/tests/CacheTest.php @@ -22,12 +22,6 @@ class CacheTest extends TestCase { - private function createCache(string $type, int $maxAge, bool $mustRevalidate, bool $withStreamFactory): Cache - { - $streamFactory = $withStreamFactory ? new StreamFactory() : null; - return new Cache($type, $maxAge, $mustRevalidate, $streamFactory); - } - public function requestFactory(): ServerRequestInterface { $serverRequestFactory = new ServerRequestFactory(); @@ -235,4 +229,10 @@ public function testLastModifiedReturnsNoBodyOnCacheHitWhenAStreamFactoryIsProvi self::assertEquals(304, $res->getStatusCode()); self::assertSame('', (string) $res->getBody()); } + + private function createCache(string $type, int $maxAge, bool $mustRevalidate, bool $withStreamFactory): Cache + { + $streamFactory = $withStreamFactory ? new StreamFactory() : null; + return new Cache($type, $maxAge, $mustRevalidate, $streamFactory); + } } From 5dd9dddd02493300355d0df7aa8efc775b676d61 Mon Sep 17 00:00:00 2001 From: Rob Allen Date: Sun, 2 Nov 2025 19:25:36 +0000 Subject: [PATCH 6/6] Fix formatting of use statements in CacheTest --- tests/CacheTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CacheTest.php b/tests/CacheTest.php index 2d73f87..df8a39b 100644 --- a/tests/CacheTest.php +++ b/tests/CacheTest.php @@ -15,8 +15,8 @@ use Slim\HttpCache\Cache; use Slim\Psr7\Factory\ResponseFactory; use Slim\Psr7\Factory\ServerRequestFactory; - use Slim\Psr7\Factory\StreamFactory; + use function gmdate; use function time;