codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.38k stars 1.9k forks source link

Bug: secureheaders does not work with cache #6281

Closed ThibautPV closed 2 years ago

ThibautPV commented 2 years ago

PHP Version

8.1

CodeIgniter4 Version

4.2.1

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

No response

What happened?

Http headers of "secureheaders" are not present when cache is active

Steps to Reproduce

  1. Active "secureheaders" in "app/Filters.php"

  2. Add "$this->cachePage(200);" in your Controller

Expected Output

The following lines should appear in the http header :

x-content-type-options: nosniff x-download-options: noopen x-frame-options: SAMEORIGIN x-permitted-cross-domain-policies: none

Anything else?

No response

iRedds commented 2 years ago

Thanks for the report.

4Team: The gatherOutput() method, in which the page caching function is called, is called before the filters (after) have been processed. Therefore, headers set in filters do not get into the cache. The gatherOutput() method also calls the displayPerformanceMetrics() method on which the debug toolbar filter depends. This will not allow the gatherOutput() method to be called after the filters. Although it would be logical.

So the only way out is to extract the caching from the gatherOutput() method.

https://github.com/codeigniter4/CodeIgniter4/blob/62048964f20f6561cb1e05e6fa58978a114d1026/system/CodeIgniter.php#L476-L485

kenjis commented 2 years ago

@iRedds Thanks for your analysis. I sent a PR #6282

MGatner commented 2 years ago

I'm not sure if this is safe to consider a "bug". After filters can do any number of things, including content not safe for caching. For example I have a number of libraries that use after filters to apply content changes specific to a logged-in user. I chose to go with after filters as opposed to View Decorators in part due to the caching differences.

If this change is needed to make advertised features work as expected then it needs to be handled as a breaking change with larger potential impact.

MGatner commented 2 years ago

Here's an example of adding CSS/JS tags based on an authenticated user's selection of a theme: https://github.com/tattersoftware/codeigniter4-themes/blob/develop/src/Filters/ThemesFilter.php

kenjis commented 2 years ago

I understand your concern, but there is no documentation that shows after filtering not cached.

It seems difficult that we say the Secure Headers will be gone when you cache the page. And if a dev changes contents in before filters or in a controllers for a logged-in user, what's the difference between them and in after filters?

The feature that changes in after filters are not cached seems difficult to justify and is unpredictable.

MGatner commented 2 years ago

Understood. Please be sure to highlight in the changelog and I will update my package docs as needed.

FWIW I almost never use page caching; I frequently have a nav bar with the user's name in it, and almost any page can have flashdata alerts displayed. It might be handy in the future to have an explicit way of caching pages before filters and view decorators, then run the cached content through those components before displaying.

kenjis commented 2 years ago

Yes, the current Web Page Caching cannot be used for personalized pages. It seems it is better to have Page Cache service and a dev can control when Response is cached.