statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
4.09k stars 534 forks source link

Ignore query strings parameter not working for full static caching #11142

Closed JoeriE closed 3 days ago

JoeriE commented 3 days ago

Bug description

Hi, I would like to ignore query strings but allow some for filtering purposes. This is my config but it doesn't seem to work on the latest version of Statamic.

    'ignore_query_strings' => true,

    'allowed_query_strings' => [
        'audience', 'tag'
    ],

    'disallowed_query_strings' => [
    ],

How to reproduce

  1. Enable 'Full static caching'
  2. Add config settings in static_caching.php

When I go to these urls:

There's only one static file generated for _blog.html. But there should be a second one with _blog?audience=audience-name.html because pages with the audience query string should be cached too.

Logs

No response

Environment

Environment
Application Name: Delizio
Laravel Version: 11.31.0
PHP Version: 8.3.4
Composer Version: 1.10.27
Environment: dev
Debug Mode: ENABLED
URL: delizio.test/
Maintenance Mode: OFF
Timezone: UTC
Locale: en

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: log
Cache: file
Database: mysql
Logs: stack / single
Mail: smtp
Queue: sync
Session: file

Statamic
Addons: 2
Sites: 4 (Nederlands, English, Français, Portuguese)
Stache Watcher: Enabled
Static Caching: full
Version: 5.38.0 PRO

Statamic Addons
aerni/advanced-seo: 2.9.3
stillat/relationships: 2.2.1

Installation

Fresh statamic/statamic site via CLI

Additional details

No response

ryanmitchell commented 3 days ago

What is your NGINX config?

JoeriE commented 3 days ago

@ryanmitchell This is my htaccess config

    RewriteCond %{HTTP:Authorization} .
    RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]

    # Redirect Trailing Slashes If Not A Folder...
    RewriteCond %{REQUEST_FILENAME} !-d
    RewriteCond %{REQUEST_URI} (.+)/$
    RewriteRule ^ %1 [L,R=301]

    RewriteCond %{HTTP_HOST} ^www\.(.*)$ [NC]
    RewriteRule ^(.*)$ http://%1/$1 [R=301,L]

    RewriteCond %{DOCUMENT_ROOT}/static/%{REQUEST_URI}_%{QUERY_STRING}\.html -s
    RewriteCond %{REQUEST_METHOD} GET
    RewriteRule .* static/%{REQUEST_URI}_%{QUERY_STRING}\.html [L,T=text/html]

    # Handle Front Controller...
    RewriteCond %{REQUEST_FILENAME} !-d
    RewriteCond %{REQUEST_FILENAME} !-f
    RewriteRule ^ index.php [L]
ryanmitchell commented 3 days ago

I've just noticed you have ignore_query_strings: true - it would need to be false otherwise all query strings are ignored, even the ones you've safe listed.

JoeriE commented 3 days ago

@ryanmitchell So there's no way to ignore all query string except a few that are listed in the config?

ryanmitchell commented 3 days ago

No, you can do that, but you have told it to ignore all query strings.

If you set that to false, then it will only keep the query strings you've specified in allowed_query_strings

JoeriE commented 3 days ago

@ryanmitchell I've changed my config to this:

    'ignore_query_strings' => false,

    'allowed_query_strings' => [
        'audience', 'tag'
    ],

    'disallowed_query_strings' => [
    ],

It keeps caching other query strings:

image
ryanmitchell commented 3 days ago

Yeah you're right - not sure why. I've opened a PR here with the changes needed: https://github.com/statamic/cms/pull/11143

JoeriE commented 3 days ago

@ryanmitchell Thanks a lot!!

ryanmitchell commented 3 days ago

Going to close it actually, just noticed this comment in the original PR: https://github.com/statamic/cms/pull/10701#issuecomment-2329319842

duncanmcclean commented 3 days ago

The allowed_query_strings and disallowed_query_strings config options aren't compatible with full-measure static caching due to the way Apache/Nginx rules work.

They're only compatible with half-measure static caching, as per the original PR (#10701):

CleanShot 2024-11-19 at 10 23 58

CleanShot 2024-11-19 at 10 23 48