8000 bug #48524 [HttpKernel] Fix `CacheAttributeListener` priority (HypeMC) · symfony/symfony@0f5a556 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0f5a556

Browse files
nicolas-grekasnicolas-grekas
committed
bug #48524 [HttpKernel] Fix CacheAttributeListener priority (HypeMC)
This PR was merged into the 6.2 branch. Discussion ---------- [HttpKernel] Fix `CacheAttributeListener` priority | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Currently the `CacheAttributeListener` & the `IsGrantedAttributeListener` have the same priority: ``` Registered Listeners for "kernel.controller_arguments" Event ============================================================ ------- --------------------------------------------------------------------------------------------------------- ---------- Order Callable Priority ------- --------------------------------------------------------------------------------------------------------- ---------- #1 Symfony\Component\HttpKernel\EventListener\CacheAttributeListener::onKernelControllerArguments() 10 #2 Symfony\Component\Security\Http\EventListener\IsGrantedAttributeListener::onKernelControllerArguments() 10 #3 Symfony\Component\HttpKernel\EventListener\ErrorListener::onControllerArguments() 0 ------- --------------------------------------------------------------------------------------------------------- ---------- ``` Since the `CacheAttributeListener` is alphabetically first, it's first to get triggered. This can cause an unauthenticated user to receive a 304 Not modified instead of a 302 Redirect, resulting in the user seeing some stale content from when they were authenticated instead of getting redirected to the login page. This PR changes the priority of the `CacheAttributeListener` to be lower than that of the `IsGrantedAttributeListener`. Commits ------- 90eb89f [HttpKernel] Fix CacheAttributeListener priority
2 parents b000c05 + 90eb89f commit 0f5a556

File tree

6 files changed

+145
-2
lines changed

6 files changed

+145
-2
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/CacheAttributeListenerTest.php

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\HttpFoundation\Response;
16+
use Symfony\Component\HttpKernel\Attribute\Cache;
17+
use Symfony\Component\HttpKernel\Controller\ValueResolverInterface;
18+
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
19+
use Symfony\Component\Security\Core\User\InMemoryUser;
20+
use Symfony\Component\Security\Http\Attribute\IsGranted;
21+
22+
class CacheAttributeListenerTest extends AbstractWebTestCase
23+
{
24+
public function testAnonimousUserWithEtag()
25+
{
26+
$client = self::createClient(['test_case' => 'CacheAttributeListener']);
27+
28+
$client->request('GET', '/', server: ['HTTP_IF_NONE_MATCH' => sprintf('"%s"', hash('sha256', '12345'))]);
29+
30+
self::assertTrue($client->getResponse()->isRedirect('http://localhost/login'));
31+
}
32+
33+
public function testAnonimousUserWithoutEtag()
34+
{
35+
$client = self::createClient(['test_case' => 'CacheAttributeListener']);
36+
37+
$client->request('GET', '/');
38+
39+
self::assertTrue($client->getResponse()->isRedirect('http://localhost/login'));
40+
}
41+
42+
public function testLoggedInUserWithEtag()
43+
{
44+
$client = self::createClient(['test_case' => 'CacheAttributeListener']);
45+
46+
$client->loginUser(new InMemoryUser('the-username', 'the-password', ['ROLE_USER']));
47+
$client->request('GET', '/', server: ['HTTP_IF_NONE_MATCH' => sprintf('"%s"', hash('sha256', '12345'))]);
48+
49+
$response = $client->getResponse();
50+
51+
self::assertSame(304, $response->getStatusCode());
52+
self::assertSame('', $response->getContent());
53+
}
54+
55+
public function testLoggedInUserWithoutEtag()
56+
{
57+
$client = self::createClient(['test_case' => 'CacheAttributeListener']);
58+
59+
$client->loginUser(new InMemoryUser('the-username', 'the-password', ['ROLE_USER']));
60+
$client->request('GET', '/');
61+
62+
$response = $client->getResponse();
63+
64+
self::assertSame(200, $response->getStatusCode());
65+
self::assertSame('Hi there!', $response->getContent());
66+
}
67+
}
68+
69+
class TestEntityValueResolver implements ValueResolverInterface
70+
{
71+
public function resolve(Request $request, ArgumentMetadata $argument): iterable
72+
{
73+
return Post::class === $argument->getType() ? [new Post()] : [];
74+
}
75+
}
76+
77+
class Post
78+
{
79+
public function getId(): int
80+
{
81+
return 1;
82+
}
83+
84+
public function getEtag(): string
85+
{
86+
return '12345';
87+
}
88+
}
89+
90+
class WithAttributesController
91+
{
92+
#[IsGranted('ROLE_USER')]
93+
#[Cache(etag: 'post.getEtag()')]
94+
public function __invoke(Post $post): Response
95+
{
96+
return new Response('Hi there!');
97+
}
98+
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/CacheAttributeListener/bundles.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
13+
use Symfony\Bundle\SecurityBundle\SecurityBundle;
14+
15+
return [
16+
new FrameworkBundle(),
17+
new SecurityBundle(),
18+
];

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/CacheAttributeListener/config.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
imports:
2+
- { resource: ../config/default.yml }
3+
4+
services:
5+
Symfony\Bundle\FrameworkBundle\Tests\Functional\TestEntityValueResolver:
6+
tags:
7+
- { name: controller.argument_value_resolver, priority: 110 }
8+
9+
Symfony\Bundle\FrameworkBundle\Tests\Functional\WithAttributesController:
10+
public: true
11+
12+
security:
13+
providers:
14+
main:
15+
memory:
16+
users:
17+
the-username: { password: the-password, roles: [ 'ROLE_USER' ] }
18+
19+
firewalls:
20+
main:
21+
pattern: ^/
22+
form_login:
23+
login_path: /login
24+
provider: main

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/CacheAttributeListener/routing.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
with_attributes_controller:
2+
path: /
3+
controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\WithAttributesController

‎src/Symfony/Bundle/FrameworkBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"symfony/error-handler": "^6.1",
2727
"symfony/event-dispatcher": "^5.4|^6.0",
2828
"symfony/http-foundation": "^6.2",
29-
"symfony/http-kernel": "^6.2",
29+
"symfony/http-kernel": "^6.2.1",
3030
"symfony/polyfill-mbstring": "~1.0",
3131
"symfony/filesystem": "^5.4|^6.0",
3232
"symfony/finder": "^5.4|^6.0",

‎src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event)
7777

7878
public static function getSubscribedEvents(): array
7979
{
80-
return [KernelEvents::CONTROLLER_ARGUMENTS => ['onKernelControllerArguments', 10]];
80+
return [KernelEvents::CONTROLLER_ARGUMENTS => ['onKernelControllerArguments', 20]];
8181
}
8282

8383
private function getIsGrantedSubject(string|Expression $subjectRef, Request $request, array $arguments): mixed

0 commit comments

Comments
 (0)
0