From 27ae8bb0ae04cf8adae274c6a6cf6e5503765bd2 Mon Sep 17 00:00:00 2001 From: Michal Sniatala Date: Sun, 17 Nov 2024 10:50:41 +0100 Subject: [PATCH] fix: `DownloadResponse` cache headers (#9237) * fix: `DownloadResponse` cache headers * update phpstan-baseline --- phpstan-baseline.php | 6 --- system/Exceptions/DownloadException.php | 2 + system/HTTP/DownloadResponse.php | 12 ----- tests/system/HTTP/DownloadResponseTest.php | 50 ++++++++++++++++++--- user_guide_src/source/changelogs/v4.5.6.rst | 10 ++--- 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index a8ac948def52..67662f829954 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -5869,12 +5869,6 @@ 'count' => 2, 'path' => __DIR__ . '/system/HTTP/DownloadResponse.php', ]; -$ignoreErrors[] = [ - // identifier: missingType.iterableValue - 'message' => '#^Method CodeIgniter\\\\HTTP\\\\DownloadResponse\\:\\:setCache\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/system/HTTP/DownloadResponse.php', -]; $ignoreErrors[] = [ // identifier: method.childReturnType 'message' => '#^Return type \\(CodeIgniter\\\\HTTP\\\\DownloadResponse\\) of method CodeIgniter\\\\HTTP\\\\DownloadResponse\\:\\:sendBody\\(\\) should be covariant with return type \\(\\$this\\(CodeIgniter\\\\HTTP\\\\Response\\)\\) of method CodeIgniter\\\\HTTP\\\\Response\\:\\:sendBody\\(\\)$#', diff --git a/system/Exceptions/DownloadException.php b/system/Exceptions/DownloadException.php index df78127d6d64..8ed2ba4849ab 100644 --- a/system/Exceptions/DownloadException.php +++ b/system/Exceptions/DownloadException.php @@ -47,6 +47,8 @@ public static function forNotFoundDownloadSource() } /** + * @deprecated Since v4.5.6 + * * @return static */ public static function forCannotSetCache() diff --git a/system/HTTP/DownloadResponse.php b/system/HTTP/DownloadResponse.php index 13a1e2b2feaa..a0da2e8ce4b9 100644 --- a/system/HTTP/DownloadResponse.php +++ b/system/HTTP/DownloadResponse.php @@ -242,16 +242,6 @@ public function noCache(): self return $this; } - /** - * Disables cache configuration. - * - * @throws DownloadException - */ - public function setCache(array $options = []) - { - throw DownloadException::forCannotSetCache(); - } - /** * {@inheritDoc} * @@ -290,10 +280,8 @@ public function buildHeaders() $this->setHeader('Content-Disposition', $this->getContentDisposition()); } - $this->setHeader('Expires-Disposition', '0'); $this->setHeader('Content-Transfer-Encoding', 'binary'); $this->setHeader('Content-Length', (string) $this->getContentLength()); - $this->noCache(); } /** diff --git a/tests/system/HTTP/DownloadResponseTest.php b/tests/system/HTTP/DownloadResponseTest.php index b62996f3d825..5897a1f58750 100644 --- a/tests/system/HTTP/DownloadResponseTest.php +++ b/tests/system/HTTP/DownloadResponseTest.php @@ -148,12 +148,25 @@ public function testNoCache(): void $this->assertSame('private, no-transform, no-store, must-revalidate', $response->getHeaderLine('Cache-control')); } - public function testCantSetCache(): void + public function testSetCache(): void { $response = new DownloadResponse('unit-test.txt', true); - $this->expectException(DownloadException::class); - $response->setCache(); + $date = date('r'); + + $options = [ + 'etag' => '12345678', + 'last-modified' => $date, + 'max-age' => 300, + 'must-revalidate', + ]; + + $response->setCache($options); + $response->buildHeaders(); + + $this->assertSame('12345678', $response->getHeaderLine('ETag')); + $this->assertSame($date, $response->getHeaderLine('Last-Modified')); + $this->assertSame('max-age=300, must-revalidate', $response->getHeaderLine('Cache-Control')); } public function testWhenFilepathIsSetBinaryCanNotBeSet(): void @@ -200,30 +213,53 @@ public function testCanGetContentLength(): void $this->assertSame($size, $response->getContentLength()); } - public function testIsSetDownloadableHeadlersFromBinary(): void + public function testIsSetDownloadableHeadersFromBinary(): void { $response = new DownloadResponse('unit test.txt', false); $response->setBinary('test'); $response->buildHeaders(); + $this->assertSame('private, no-transform, no-store, must-revalidate', $response->getHeaderLine('Cache-Control')); $this->assertSame('application/octet-stream', $response->getHeaderLine('Content-Type')); $this->assertSame('attachment; filename="unit test.txt"; filename*=UTF-8\'\'unit%20test.txt', $response->getHeaderLine('Content-Disposition')); - $this->assertSame('0', $response->getHeaderLine('Expires-Disposition')); $this->assertSame('binary', $response->getHeaderLine('Content-Transfer-Encoding')); $this->assertSame('4', $response->getHeaderLine('Content-Length')); } - public function testIsSetDownloadableHeadlersFromFile(): void + public function testIsSetDownloadableHeadersFromFile(): void + { + $response = new DownloadResponse('unit-test.php', false); + + $response->setFilePath(__FILE__); + $response->buildHeaders(); + + $this->assertSame('private, no-transform, no-store, must-revalidate', $response->getHeaderLine('Cache-Control')); + $this->assertSame('application/octet-stream', $response->getHeaderLine('Content-Type')); + $this->assertSame('attachment; filename="unit-test.php"; filename*=UTF-8\'\'unit-test.php', $response->getHeaderLine('Content-Disposition')); + $this->assertSame('binary', $response->getHeaderLine('Content-Transfer-Encoding')); + $this->assertSame(filesize(__FILE__), (int) $response->getHeaderLine('Content-Length')); + } + + public function testCustomHeaders(): void { $response = new DownloadResponse('unit-test.php', false); $response->setFilePath(__FILE__); + + $response->setHeader('Last-Modified', 'Fri, 18 Oct 2024 13:17:37 GMT'); + $response->setHeader('Expires', 'Sun, 17 Nov 2024 14:17:37 GMT'); + $response->setHeader('Pragma', 'public'); + $response->removeHeader('Cache-Control'); + $response->setHeader('Cache-Control', 'public'); $response->buildHeaders(); + $this->assertSame('Fri, 18 Oct 2024 13:17:37 GMT', $response->getHeaderLine('Last-Modified')); + $this->assertSame('Sun, 17 Nov 2024 14:17:37 GMT', $response->getHeaderLine('Expires')); + $this->assertSame('public', $response->getHeaderLine('Pragma')); + $this->assertSame('public', $response->getHeaderLine('Cache-Control')); $this->assertSame('application/octet-stream', $response->getHeaderLine('Content-Type')); $this->assertSame('attachment; filename="unit-test.php"; filename*=UTF-8\'\'unit-test.php', $response->getHeaderLine('Content-Disposition')); - $this->assertSame('0', $response->getHeaderLine('Expires-Disposition')); $this->assertSame('binary', $response->getHeaderLine('Content-Transfer-Encoding')); $this->assertSame(filesize(__FILE__), (int) $response->getHeaderLine('Content-Length')); } diff --git a/user_guide_src/source/changelogs/v4.5.6.rst b/user_guide_src/source/changelogs/v4.5.6.rst index a835382a5724..75d4d92212b6 100644 --- a/user_guide_src/source/changelogs/v4.5.6.rst +++ b/user_guide_src/source/changelogs/v4.5.6.rst @@ -29,16 +29,14 @@ Deprecations ********** Bugs Fixed ********** -- **RequestTrait:** Fixed a bug where the ``fetchGlobal()`` method did not allow handling data by numeric key when stored as a list. +- **RequestTrait:** Fixed a bug where the ``fetchGlobal()`` method did not allow handling data by numeric key when stored as a list. - **Session Library:** The session initialization debug message now uses the correct log type "debug" instead of "info". - -- **Validation:** Fixed the `getValidated()` method that did not return valid data when validation rules used multiple asterisks. - +- **Validation:** Fixed the ``getValidated()`` method that did not return valid data when validation rules used multiple asterisks. - **Database:** Fixed the case insensitivity option in the ``like()`` method when dealing with accented characters. - - **Parser:** Fixed bug that caused equal key names to be replaced by the key name defined first. - +- **DownloadResponse:** Fixed a bug that prevented setting custom cache headers. We can now also use the ``setCache()`` method. +- **DownloadResponse:** Fixed a bug involving sending a custom "Expires-Disposition" header. - **Routing:** Fixed a TypeError in `str_replace()` when `Routing::$translateURIDashes` is set to `true` and a route is defined using a closure. - **Validation:** Fixed a bug where complex language strings were not properly handled.