Closed dbu closed 4 months ago
For reference here I added a comment to: https://github.com/sulu/sulu/issues/7441#issuecomment-2245685753 which also is little bit related to new flag CustomTtlListener(fallbackToSmaxage: false)
from https://github.com/FriendsOfSymfony/FOSHttpCache/pull/575
Copy of the comment: so changes here are understandable in future:
The only part which we may need to recheck is the CustomTtlListener
makes now private
requests always public, specially in case of $fallbackToSmaxAge = false
.
If we want that private
responses are never cached also when CustomTtlListener
is used we would require:
diff --git a/src/SymfonyCache/CustomTtlListener.php b/src/SymfonyCache/CustomTtlListener.php
index 4be17d9..9ff303e 100644
--- a/src/SymfonyCache/CustomTtlListener.php
+++ b/src/SymfonyCache/CustomTtlListener.php
@@ -77,12 +77,18 @@ class CustomTtlListener implements EventSubscriberInterface
? $response->headers->getCacheControlDirective('s-maxage')
: 'false'
;
+
$response->headers->set(static::SMAXAGE_BACKUP, $backup);
+ $isPrivate = $response->headers->getCacheControlDirective('private');
+ // calling setTtl will make a response public, so we need to set it back to private if it was private before
$response->setTtl(
$response->headers->has($this->ttlHeader)
? $response->headers->get($this->ttlHeader)
: 0
);
+ if ($isPrivate) {
+ $response->setPrivate();
+ }
}
/**
Or alternative us addCacheControlDirective
instead of setTtl
:
diff --git a/src/SymfonyCache/CustomTtlListener.php b/src/SymfonyCache/CustomTtlListener.php
index 4be17d9..31126f3 100644
--- a/src/SymfonyCache/CustomTtlListener.php
+++ b/src/SymfonyCache/CustomTtlListener.php
@@ -77,8 +77,11 @@ class CustomTtlListener implements EventSubscriberInterface
? $response->headers->getCacheControlDirective('s-maxage')
: 'false'
;
+
$response->headers->set(static::SMAXAGE_BACKUP, $backup);
- $response->setTtl(
+
+ $response->headers->addCacheControlDirective(
+ 's-maxage',
$response->headers->has($this->ttlHeader)
? $response->headers->get($this->ttlHeader)
: 0
What do you think would be expected behaviour for:
$response = new Response();
$response->setPrivate(0);
$response->header->set('X-Reverse-Proxy-TTL', 1200);
Current version (2.15.3) would not cache it. With https://github.com/FriendsOfSymfony/FOSHttpCache/pull/577 and CustomTtlListener(fallbackToSmaxage: false)
it will now be cached. But I think explicit set private responses should not be cached even when a custom ttl is set? As example private response with a s-maxage defined is also not cached.
For reference here I added a comment to: sulu/sulu#7441 (comment) which also is little bit related to new flag
CustomTtlListener(fallbackToSmaxage: false)
from #575
uh, very good catch! i change the code to use setCacheControlDirective. i guess until now this mistake was hidden by the listener not being called at all for a private response. i think we should keep that behaviour.
indeed targeted cache control would be very useful to avoid this mess: https://github.com/symfony/symfony/issues/47288 (we could also provide that functionality with FOSHttpCache rules, but first should try to do the pull request to core symfony)
@dbu retested the change and looks good for me.
fix #576