Bug: Multiple Expires and Cache-Control Headers in File Download Response

[ToastStudios]

PHP Version

8.1

CodeIgniter4 Version

4.5.5

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

macOS, Linux

Which server did you use?

apache

Database

mysql Ver 8.3.0 for macos14.2 on arm64 (Homebrew)

What happened?

While implementing a file download feature in a CodeIgniter 4 application, I encountered an issue where multiple Expires and Cache-Control headers were being sent in the HTTP response. This problem occurred despite attempts to explicitly set these headers using the framework's response methods.

Steps to Reproduce

1. Create a controller method to serve a file download (e.g., CRL file).
2. Use the response object to set custom headers, such as Content-Type, Last-Modified, and Expires.
3. Attempt to remove existing headers using $response->removeHeader().
4. Observe that multiple Expires and Cache-Control headers are still present in the HTTP response.

Expected Output

Only one instance of each header should be sent in the response, specifically the ones set in the controller method.

Anything else?

As a workaround, the issue was resolved by using native PHP functions (header() and readfile()) to manage headers directly, bypassing CodeIgniter’s response handling. This provided complete control over the headers sent, eliminating the duplication issue.

[michalsn]

Can you provide a sample code so we can reproduce the error?

[ToastStudios]

Yes of course, but during the preparation of the example code I could narrow the issue down to a database operation which happened before. In any case I had to prepare the download response to modify any headers, I assume the headers get overridden by ->download().

namespace App\Controllers;

class DownloadTest extends BaseController
{
    public function getIndex($name)
    {
        $filename = WRITEPATH.'example/'.$name;
        if (file_exists($filename)){
            // Log the file access
            model('MyLogsModel')->insert([
                'user_id'        => user_id() ?? 0,
                'message'        => 'Revocation list downloaded: '.$name.'.',
            ]);
            // Prepare the download
            $response = $this->response->download($filename, null);
            $response->removeHeader('Expires'); // no effect
            return $response
                ->setHeader('Content-Type', 'application/pkix-crl')             // RFC 5280, section 4.2.1.13
                ->setHeader('Last-Modified', 'Fri, 18 Oct 2024 13:17:37 GMT')   // RFC 5019, section 6.2
                ->setHeader('Expires', 'Sun, 17 Nov 2024 14:17:37 GMT');        // RFC 5019, section 6.2
        } else {
            return $this->response->setStatusCode(404);
        }
    }
}

Respose:

HTTP/1.1 200 OK
Date: Thu, 24 Oct 2024 07:16:35 GMT
Server: Apache/2.4.59 (Unix) PHP/8.1.28
X-Powered-By: PHP/8.1.28
Expires: Thu, 19 Nov 1981 08:52:00 GMT                          <- first Expires header
Cache-Control: no-store, no-cache, must-revalidate              <- first Cache-Control header
Pragma: no-cache
Last-Modified: Fri, 18 Oct 2024 13:17:37 GMT                    <- this is OK
Expires: Sun, 17 Nov 2024 14:17:37 GMT                          <- second Expires header
Content-Disposition: attachment; filename="my-sign-ca-v1.crl"; filename*=UTF-8''my-sign-ca-v1.crl
Expires-Disposition: 0
Content-Transfer-Encoding: binary
Content-Length: 852
Cache-Control: private, no-transform, no-store, must-revalidate <- second Cache-Control header
Access-Control-Allow-Origin: *
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: application/pkix-crl                              <- this is OK

If I now omit model('MyLogsModel')->insert(...), the response is correct:

HTTP/1.1 200 OK
Date: Thu, 24 Oct 2024 07:25:46 GMT
Server: Apache/2.4.59 (Unix) PHP/8.1.28
X-Powered-By: PHP/8.1.28
Last-Modified: Fri, 18 Oct 2024 13:17:37 GMT                    <- this is OK
Expires: Sun, 17 Nov 2024 14:17:37 GMT                          <- this is OK
Content-Disposition: attachment; filename="my-sign-ca-v1.crl"; filename*=UTF-8''my-sign-ca-v1.crl
Expires-Disposition: 0
Content-Transfer-Encoding: binary
Content-Length: 852
Cache-Control: private, no-transform, no-store, must-revalidate <- single Cache-Control header
Access-Control-Allow-Origin: *
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: application/pkix-crl                              <- this is OK

Duplicate headers also appear when inserting without model:

$db = \Config\Database::connect();
$db->table('my_logs')->insert([
    'user_id'        => user_id() ?? 0,
    'message'        => 'Revocation list downloaded: '.$name.'.',
]);

[michalsn]

Thank you for the update.

I have not been able to reproduce the problem, and I have to admit that the conditions that must be met for it to occur are strange.

Please update CodeIgniter to the latest version (4.5.5) and try again with the built-in server:

php spark serve

I would guess that Apache adds some headers automatically, but since it doesn't always happen, I don't know what is causing it. By any chance, do you use any custom Filters?

Generally, downloads have automatic headers set to prevent caching. To change this, you would need to extend the DownloadResponse class and modify the buildHeaders() method.

The only problem I see here is setting the Expires-Disposition header - this is the first time I've seen it. I guess it is a bug, but it does not affect the described problem.

[ToastStudios]

I did an upgrade to 4.5.5 but the issue persists, it even behaves exactly the same when using php spark serve.

I have to disagree with the statement that the headers come from the web server. The following code does not involve multiple Headers, no matter whether via spark or apache. (I know, unconventional and goes against typical framework practices)

public function getIndex($name)
{
    $filename = WRITEPATH . 'example/' . $name;
    if (file_exists($filename)) {
        // Log the file access
        model('MyLogsModel')->insert([
            'user_id'        => user_id() ?? 0,
            'message'        => 'Revocation list downloaded: '.$name.'.',
        ]);
        // Avoid using framework's response object and directly set headers with PHP
        header('Content-Type: application/pkix-crl');           // RFC 5280 section 4.2.1.13
        header('Last-Modified: Fri, 18 Oct 2024 13:17:37 GMT'); // RFC 5019 section 6.2
        header('Expires: Sun, 17 Nov 2024 14:17:37 GMT');       // RFC 5019 section 6.2
        header('Content-Length: ' . filesize($filename));       // Set content length header
        header('Cache-Control: public');                        // Allow caching
        header('Pragma: public');                               // Ensure cache compatibility
        // Output the file content
        readfile($filename);  // Outputs the CRL directly to the client
        // End script execution to avoid any further output
        exit();
    } else {
        // Return a 404 response if the file does not exist
        header("HTTP/1.0 404 Not Found");
        echo 'The requested CRL was not found.';
        exit();
    }
}

Response

HTTP/1.1 200 OK
Host: localhost:8080
Date: Thu, 24 Oct 2024 11:59:43 GMT
Connection: close
X-Powered-By: PHP/8.1.28
Set-Cookie: ci_session=qm29kjtoqo387836jmrhhu4mj4undc4o; expires=Thu, 24-Oct-2024 13:59:43 GMT; Max-Age=7200; path=/; HttpOnly; SameSite=Lax
Content-Type: application/pkix-crl
Last-Modified: Fri, 18 Oct 2024 13:17:37 GMT
Expires: Sun, 17 Nov 2024 14:17:37 GMT
Content-Length: 852
Cache-Control: public
Pragma: public

Regarding the filters, I use shield sessions, but an exception is made for this controller because it does not require authentication. The Before Filters and After Filters for this controller shown by php spark routes are empty.

[michalsn]

Thank you for mentioning Shield. That helped a lot because I realized you were using a session.

Well - using a session is the real "problem". These headers are set at the PHP session level and out of CodeIgniter's control. When there is a session - all data is considered private and cache is disabled.

However, the question is, shouldn't we allow session.cache_limiter setting to be specified using ini_set(). Now we are using the default setting, which is nocache. If we allow this setting to be changed, it would cause some security concerns that users don't need to worry about now.

On the other hand, maybe we should just modify the way we send headers and force a replace when we deal with only one header: https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/HTTP/ResponseTrait.php#L406

I'm leaning towards the second option, but I'm not convinced - I'd like to hear other people's opinions.

[ToastStudios]

I think I understand the concerns, but if I deliberately set headers with ->setHeader('Expires', $expires), I expect that they replace any existing headers of the same type. That is also what I read from the documentation for Setting Headers. If I want to add headers without modifying existing ones, I have appendHeader() and prependHeader() available.

nocache is perfectly acceptable as default in my opinion if the user does not specify anything.