codeigniter4 / CodeIgniter4

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

Bug: Web Page Caching Doesn't Honour HTTP Method #8363

Closed jasonkerner closed 11 months ago

jasonkerner commented 11 months ago

PHP Version

7.4, 8.0, 8.2

CodeIgniter4 Version

4.3.7

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 5.7

What happened?

When rendering an output (using a Controller, loading a View), the "this->cachePage($n);" method is called just before the view is displayed. The view displays a Contact form with a POST method on the form. When submitting the form, the method within the Controller to process the post data is never fired as the Cached version of the page is returned to the browser.

Steps to Reproduce

Expected Output

Cached pages should only trigger with GET requests; any other (POST, PUT, PATCH, DELETE, etc) should not have cached output even created. What should happen is...

This is exactly what happens when Caching is turned off.

Anything else?

No response

kenjis commented 11 months ago

This is not a bug. There is no feature to check HTTP method in Web Page Caching. https://codeigniter4.github.io/CodeIgniter4/general/caching.html

You should not cache the page with POST request.

jasonkerner commented 11 months ago

But the, the documents should atleast mention that it doesn't honour the HTTP method. And technically, I'm not caching the POST request, I'm caching the output displayed the browser (as designed and as the Web Page Caching intended), but, to use other features (such as input validation repopulation, displaying form validation rules), you need to post to the same URL for that to work - so only GET requests should be cached.

kenjis commented 11 months ago

Caching can be enabled on a per-page basis, and you can set the length of time that a page should remain cached before being refreshed.

The caching comes from CI3. So the design is too old, but the page of a per-page basis is a URI.

Can you show your spark routes?

kenjis commented 11 months ago

I sent a PR #8364 Please try if you can.

jasonkerner commented 11 months ago

Hi, @kenjis , thanks for that - but I think this fix needs to be a Config based ones because your example would work only if the routes were known ahead of time (ie, when you're developing the project). In the scenario I'm using, this project is a Content Management System, meaning various pages are created with their own URLs stored in the database, and simply routed through a common controller. This would mean you can't use CI's routing system, but if the Config to manage the Cache had an additional flag to make it only work with GET requests (or you choose the methods to cache or not), then this way, it would work in your example as global setting and/or in my scenario too

kenjis commented 11 months ago

@jasonkerner I've changed the implementation a bit.

My understanding is that that PR should be able to meet your requirements. The HTTP method will be added to the cache key, so just enable caching in the controller method that handles the GET request.

Even if it does not meet your request, in v4.5.0, the page caching will be implemented as a filter (and ResponseCache class). You can freely customize the PageCache filter as you like.

kenjis commented 11 months ago

Closed by #8364