-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Something Wrong with AbstractSessionListener #37113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Is there a way to disable I tried to disable on services:
Symfony\Component\HttpKernel\EventListener\AbstractSessionListener:
autoconfigure: false edit: I also tried to disable services:
Symfony\Component\HttpKernel\EventListener\SessionListener:
autoconfigure: false |
Maybe I misunderstand something, but the PHP Doc of the AbstractSessionListener doesn't help you?
|
@antalaron yes I missed following lines. Something can break if I disable
|
Thanks for the report. I suspect you have a template that tries to display session flash bags. That's enough to make a page uncacheable. |
@nicolas-grekas thank you for your response. Session wont empty in any regular Symfony application and it is always going to break Session object's existence is insignificant if it not send session cookie to client and we can consider the content as static in this scenario. Also we can manage this cases in Varnish or NGINX. I can give directive NGINX to bypass cache if there is a cookie but I can't create caching strategy URL by URL in NGINX configuration. Only I can create caching strategy URL by URL in Symfony's side using
You can observe same problem for following sample it not use twig, session or flashbag. namespace App\Controller;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Cache;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;
class IndexController extends AbstractController
{
/**
* @Route("/", name="index")
* @Cache(smaxage=3600)
*/
public function indexAction()
{
return new Response('test');
}
} |
Why so? With your example, the session is not used at all. In Symfony 5.1, we added a way to more easily know where the session is started/used. You might want to check https://symfony.com/blog/new-in-symfony-5-1-routing-improvements, that might help understand what's going on. |
@nicolas-grekas yes there are guard authenticators in firewall rules but there is no access control rules for the action. Also there is no user authenticated and there is no session cookie. And I found where is the session is initialized. Session requested by Then initalized by session factory callback. |
It returns public function indexAction()
{
return new Response('test');
} Then I add reference to session and set some value to session. It returns public function indexAction(SessionInterface $session)
{
$session->set('some', 'value');
return new Response('test');
} Then I convert back to previous action for third request and it returns |
@tugrul I don't know if you can go back that far, but could you try to check if everything works as you'd expect with Symfony 4.3.11 and then "something" happens as of Symfony 4.4.0? |
Seems to be related to #33663. I have an app where a controller like the one shown above... namespace App\Controller;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Cache;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;
class IndexController extends AbstractController
{
/**
* @Route("/", name="index")
* @Cache(smaxage=3600)
*/
public function indexAction()
{
return new Response('test');
}
} will be I guess it's because the application has the following (very defaults!) in security:
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
providers:
in_memory: { memory: null }
firewalls:
dev:
pattern: ^/(_(profiler|wdt)|css|images|js)/
security: false
main:
anonymous: true
# activate different ways to authenticate
# https://symfony.com/doc/current/security.html#firewalls-authentication
# https://symfony.com/doc/current/security/impersonating_user.html
# switch_user: true
# Easy way to control access for large sections of your site
# Note: Only the *first* access control that matches will be used
access_control:
# - { path: ^/admin, roles: ROLE_ADMIN }
# - { path: ^/profile, roles: ROLE_USER } Basically, that should just be a flex-provided, almost clean setup. Maybe @nicolas-grekas could advise if that is in fact intended and necessary behavior or worth further investigation? Also CC @Toflar because you've struggled with this in #33663 as well. Update: #34220 seems to be the original report. |
See my comment in #34220 (comment) |
@mpdude I don't have an idea for previous versions but this broke caching strategy of my proxy cache. @nicolas-grekas namespace App\EventSubscriber;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\EventListener\AbstractSessionListener;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\HttpKernel\KernelEvents;
class SessionCacheControlSubscriber implements EventSubscriberInterface
{
public function onKernelResponse(ResponseEvent $event)
{
if (!defined(AbstractSessionListener::class . '::NO_AUTO_CACHE_CONTROL_HEADER')) {
return;
}
$request = $event->getRequest();
// https://github.com/symfony/symfony/issues/37113#issuecomment-643176155
if (!$request->hasSession()) {
return;
}
$session = $request->getSession();
// existence of isEmpty function is not guarantee because it isn't in SessionInterface contract
if (!($session instanceof Session) || !method_exists($session, 'isEmpty')) {
$fields = $session->all();
foreach ($fields as &$field) {
if (!empty($field)) {
return;
}
}
} elseif (!$session->isEmpty()) {
return;
}
$response = $event->getResponse();
$response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, true);
}
public static function getSubscribedEvents()
{
return [
KernelEvents::RESPONSE => ['onKernelResponse', -999],
];
}
} |
Can you explain what you mean? Do you have an example? |
For example there is a |
The fact that you're accessing the session will start it. You can check if there even was a session cookie provided before you access the session. This way you can prevent it from starting: if (!$request->hasPreviousSession()) {
return;
}
if ($request->getSession()->isEmpty()) {
// ...
} |
@Toflar if there is a session without a session cookie on first request and will broke my event handler. I prefer to add |
@nicolas-grekas I created a PoC. It shows when an action stateless and has state same time. tugrul/slcc-poc@17f59f4 You can simply enable/disable |
Closing as the behavior is correct, as I explained previously. |
…ssion. This is a simpler implementation and doesn't muck around with as much of the internals of Symfony. Based on the work of Tuğrul Topuz in: - symfony/symfony#37113 (comment) - https://github.com/tugrul/slcc-poc/blob/17f59f4207f80d5ff5f7bcc62ca554ba7b36d909/src/EventSubscriber/SessionCacheControlSubscriber.php
…ssion. This is a simpler implementation and doesn't muck around with as much of the internals of Symfony. Based on the work of Tuğrul Topuz in: - symfony/symfony#37113 (comment) - https://github.com/tugrul/slcc-poc/blob/17f59f4207f80d5ff5f7bcc62ca554ba7b36d909/src/EventSubscriber/SessionCacheControlSubscriber.php
For those stumbling on this issue later, I also ran into this issue in my application and found @tugrul 's proof of concept incredibly helpful. While I appreciate that the default behavior may be desired as a safe baseline, the lack of documentation of how to successfully work around it makes it difficult to figure out how to write Symfony applications that leverage reverse-proxy caching for anonymous requests that use the same routes as authenticated requests. My implementation based on @tugrul's for Symofony 6.4 is documented in Best practice to allow reverse-proxy caching of anonymous responses?. Would there be support for further fleshing out the documentation on [HTTP Caching and User Sessions](https://symfony.com/doc/current/http_cache.html#http-caching-and-user-sessions) to include a working |
Symfony version(s) affected: 5.0.9
Description
AbstractSessionListener always modify Cache-Control header even there is no data inside of session storage.
How to reproduce
Simply create a controller with a hello world response action.
Possible Solution
We can check is there some data in session store instead of checking session mutation.
Additional context
I inspect the issue with debugging and you can see below.
The text was updated successfully, but these errors were encountered: