Quicksilver icon indicating copy to clipboard operation
Quicksilver copied to clipboard

Querystrings proposal for discussion

Open robballantyne opened this issue 4 years ago • 4 comments

Hello,

This is an initial proposal for enabling query string support for all URL's as well as introducing support for json, xml, txt and rss extensions.

This is tested and appears to be working well with Nginx. I do not use Apache so am unsure on how to proceed with modifying the rewrite rules here. Perhaps someone can advise? hopefully it's as simple as Nginx!

Security considerations

Per the design of the plugin (and the original Laravel package) all routes which return a 200 code will be cached. This behaviour is unchanged. An attacker could potentially spam URL's with unused params thereby writing many files to disk.

I would suggest that the end-user of the plugin should be throttling requests with a WAF and returning a 404 if the params sent are invalid, but this is often not the case. Mitigating this attack is probably out of scope for this plugin. The concern should be highlighted, however. Perhaps even make caching for query strings optional via settings.

robballantyne avatar Nov 25 '21 14:11 robballantyne

Thanks for your PR! Will test with apache and nginx soon.

My main concern is that this query strings can cause disk space bombing. And website-adminstrator can't do anything about it. Maybe we should add some kind of checkbox in settings?

FlusherDock1 avatar Nov 25 '21 14:11 FlusherDock1

That is definitely a valid concern. A checkbox to enable query string caching would help so users know what they're getting into.

Other potential solutions I've thought of include:

  • Set a hard limit on how many files can be saved matching the query pattern - This is not ideal, but it would mitigate.
  • Set a limit set on how many page caches can be caused by a specific client in a pre-defined time period
  • Flush the cache periodically
  • have an 'allowed params' list
  • Pause caching if cache weight grows above X MB

All of these solutions are just a band aid, however. Really, suspicious activity should result in an ip ban from whatever firewall might be running - But do all of the users have such protection? Unlikely :-(

robballantyne avatar Nov 25 '21 15:11 robballantyne

Did you get chance to test this against Apache?

I've been away a while but I'm looking to integrate my posts plugin with static caching soon so I'm wondering if you still have any interest in taking this forward with query strings or whether I need to develop something separately. I should have time to test and tidy up my code for a proper integration shortly.

I spoke with the maintainer of the original laravel package https://github.com/JosephSilber/page-cache/issues/90 and there's no interest in query strings there so I will probably want to maintain a variant of that separately.

Cheers Rob

robballantyne avatar Dec 15 '21 10:12 robballantyne

Hi! The end of year is always very intence at work, so I don't have much time to check your PR. I will try to find some time soon, and will come back to you as soon as possible. Thanks.

FlusherDock1 avatar Jan 09 '22 21:01 FlusherDock1