Skip to content

Commit

Permalink
fix: DownloadResponse cache headers (codeigniter4#9237)
Browse files Browse the repository at this point in the history
* fix: `DownloadResponse` cache headers

* update phpstan-baseline
  • Loading branch information
michalsn authored Nov 17, 2024
1 parent bb5f925 commit 27ae8bb
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 31 deletions.
6 changes: 0 additions & 6 deletions phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -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\\(\\)$#',
Expand Down
2 changes: 2 additions & 0 deletions system/Exceptions/DownloadException.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public static function forNotFoundDownloadSource()
}

/**
* @deprecated Since v4.5.6
*
* @return static
*/
public static function forCannotSetCache()
Expand Down
12 changes: 0 additions & 12 deletions system/HTTP/DownloadResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -242,16 +242,6 @@ public function noCache(): self
return $this;
}

/**
* Disables cache configuration.
*
* @throws DownloadException
*/
public function setCache(array $options = [])
{
throw DownloadException::forCannotSetCache();
}

/**
* {@inheritDoc}
*
Expand Down Expand Up @@ -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();
}

/**
Expand Down
50 changes: 43 additions & 7 deletions tests/system/HTTP/DownloadResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'));
}
Expand Down
10 changes: 4 additions & 6 deletions user_guide_src/source/changelogs/v4.5.6.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 27ae8bb

Please sign in to comment.