JosephSilber / page-cache

Caches responses as static files on disk for lightning fast page loads.
MIT License
1.21k stars 118 forks source link

Query Strings #90

Closed robballantyne closed 2 years ago

robballantyne commented 2 years ago

Hello,

I notice that issue #25 was closed as query strings do not always indicate a unique endpoint, but I believe that it's still worth enabling caching in these instances. Of course, additional disk space is required.

There are some security considerations to prevent an attacker filling the disk by trying endpoints that will return a 200 code, but this should probably be handled by a WAF anyway if it's a concern (my feeling is that an attacker will DoS the site before they fill the disk).

Would you be open to a pull request that enables caching of query strings if the functionality can be enabled via env configuration?

My intention would be to clear all variants of a page (e.g. /mypage, /mypage?page=1, /mypage?page=2 etc) when the request is made to purge /mypage so there would be no need to key based on the query string.

I would propose saving files in a format similar to

mypage[q_].html
mypage[q_page=1].html

which is fairly simple for the webserver to match on.

I want this feature as I feel the benefits of serving static content, particularly on an API response have the potential to greatly improve user experience as well as providing a significant energy-saving benefit when bypassing PHP and reducing the need to scale.

If this is something you remain opposed to I'll look at other solutions.

Thanks for providing this to the community, it's an excellent package.

Regards

JosephSilber commented 2 years ago

This is really trivial to add user-side. Why do you feel like it has to be in core?

robballantyne commented 2 years ago

I think that given Laravel's decision to handle pagination with query strings by default it's an important feature. Also, API's generally take params and many common requests will receive the same response many times over (even over short timeframes).

I feel this package can help educate users to avoid wasting resources. If query strings remain excluded from core then a user may simply not consider trying to implement this (or any) type of cache.

JosephSilber commented 2 years ago

You'd also need a list of query string keys to ignore (such as UTM and friends). This could get very specific, based on the kind of tracking a given site uses (or the lack thereof).

I still think this is best left to be implemented in your app.

robballantyne commented 2 years ago

No problem.

I would actually prefer not to strip any tags at all despite the potential for more disk usage, but I respect your opinion.

Thanks for taking the time to consider my proposal.

Cheers!