Skip to content

Commit b368ce1

Browse files
nicolas-grekasnicolas-grekas
committed
bug #46262 [EventDispatcher] Fix removing listeners when using first-class callable syntax (javer)
This PR was merged into the 4.4 branch. Discussion ---------- [EventDispatcher] Fix removing listeners when using first-class callable syntax | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46260 | License | MIT | Doc PR | - Closures should be compared using non-strict comparison to account for the first-class callable syntax. See https://github.com/php/php-src/blob/9a90bd705483004c2ef408ee9e9bb0902beade3f/Zend/zend_closures.c#L387-L423 Commits ------- df4c003 [EventDispatcher] Fix removing listeners when using first-class callable syntax
2 parents ce4a1a9 + df4c003 commit b368ce1

File tree

5 files changed

+71
-5
lines changed

5 files changed

+71
-5
lines changed

‎src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public function removeListener($eventName, $listener)
7575
{
7676
if (isset($this->wrappedListeners[$eventName])) {
7777
foreach ($this->wrappedListeners[$eventName] as $index => $wrappedListener) {
78-
if ($wrappedListener->getWrappedListener() === $listener) {
78+
if ($wrappedListener->getWrappedListener() === $listener || ($listener instanceof \Closure && $wrappedListener->getWrappedListener() == $listener)) {
7979
$listener = $wrappedListener;
8080
unset($this->wrappedListeners[$eventName][$index]);
8181
break;
@@ -110,8 +110,8 @@ public function getListenerPriority($eventName, $listener)
110110
// we might have wrapped listeners for the event (if called while dispatching)
111111
// in that case get the priority by wrapper
112112
if (isset($this->wrappedListeners[$eventName])) {
113-
foreach ($this->wrappedListeners[$eventName] as $index => $wrappedListener) {
114-
if ($wrappedListener->getWrappedListener() === $listener) {
113+
foreach ($this->wrappedListeners[$eventName] as $wrappedListener) {
114+
if ($wrappedListener->getWrappedListener() === $listener || ($listener instanceof \Closure && $wrappedListener->getWrappedListener() == $listener)) {
115115
return $this->dispatcher->getListenerPriority($eventName, $wrappedListener);
116116
}
117117
}

‎src/Symfony/Component/EventDispatcher/EventDispatcher.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public function getListenerPriority($eventName, $listener)
122122
$v[0] = $v[0]();
123123
$v[1] = $v[1] ?? '__invoke';
124124
}
125-
if ($v === $listener) {
125+
if ($v === $listener || ($listener instanceof \Closure && $v == $listener)) {
126126
return $priority;
127127
}
128128
}
@@ -178,7 +178,7 @@ public function removeListener($eventName, $listener)
178178
$v[0] = $v[0]();
179179
$v[1] = $v[1] ?? '__invoke';
180180
}
181-
if ($v === $listener) {
181+
if ($v === $listener || ($listener instanceof \Closure && $v == $listener)) {
182182
unset($listeners[$k], $this->sorted[$eventName], $this->optimized[$eventName]);
183183
}
184184
}

‎src/Symfony/Component/EventDispatcher/Tests/EventDispatcherTest.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,35 @@ public function testMutatingWhilePropagationIsStopped()
421421
$this->assertTrue($testLoaded);
422422
}
423423

424+
/**
425+
* @requires PHP 8.1
426+
*/
427+
public function testNamedClosures()
428+
{
429+
$listener = new TestEventListener();
430+
431+
$callback1 = \Closure::fromCallable($listener);
432+
$callback2 = \Closure::fromCallable($listener);
433+
$callback3 = \Closure::fromCallable(new TestEventListener());
434+
435+
$this->assertNotSame($callback1, $callback2);
436+
$this->assertNotSame($callback1, $callback3);
437+
$this->assertNotSame($callback2, $callback3);
438+
$this->assertTrue($callback1 == $callback2);
439+
$this->assertFalse($callback1 == $callback3);
440+
441+
$this->dispatcher->addListener('foo', $callback1, 3);
442+
$this->dispatcher->addListener('foo', $callback2, 2);
443+
$this->dispatcher->addListener('foo', $callback3, 1);
444+
445+
$this->assertSame(3, $this->dispatcher->getListenerPriority('foo', $callback1));
446+
$this->assertSame(3, $this->dispatcher->getListenerPriority('foo', $callback2));
447+
448+
$this->dispatcher->removeListener('foo', $callback1);
449+
450+
$this->assertSame(['foo' => [$callback3]], $this->dispatcher->getListeners());
451+
}
452+
424453
/**
425454
* @group legacy
426455
* @expectedDeprecation Calling the "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead.

‎src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,30 @@ public function testWithPreviousNotStartedSession()
342342
$this->assertSame($usageIndex, $session->getUsageIndex());
343343
}
344344

345+
public function testOnKernelResponseRemoveListener()
346+
{
347+
$tokenStorage = new TokenStorage();
348+
$tokenStorage->setToken(new UsernamePasswordToken('test1', 'pass1', 'phpunit', ['ROLE_USER']));
349+
350+
$request = new Request();
351+
$request->attributes->set('_security_firewall_run', '_security_session');
352+
353+
$session = new Session(new MockArraySessionStorage());
354+
$request->setSession($session);
355+
356+
$dispatcher = new EventDispatcher();
357+
$httpKernel = $this->createMock(HttpKernelInterface::class);
358+
359+
$listener = new ContextListener($tokenStorage, [], 'session', null, $dispatcher, null, \Closure::fromCallable([$tokenStorage, 'getToken']));
360+
$this->assertEmpty($dispatcher->getListeners());
361+
362+
$listener(new RequestEvent($httpKernel, $request, HttpKernelInterface::MASTER_REQUEST));
363+
$this->assertNotEmpty($dispatcher->getListeners());
364+
365+
$listener->onKernelResponse(new ResponseEvent($httpKernel, $request, HttpKernelInterface::MASTER_REQUEST, new Response()));
366+
$this->assertEmpty($dispatcher->getListeners());
367+
}
368+
345369
protected function runSessionOnKernelResponse($newToken, $original = null)
346370
{
347371
$session = new Session(new MockArraySessionStorage());

‎src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Security\Http\Tests\Firewall;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\EventDispatcher\EventDispatcher;
1516
use Symfony\Component\HttpFoundation\Request;
1617
use Symfony\Component\HttpFoundation\Response;
1718
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
@@ -170,6 +171,18 @@ public function testLogoutException()
170171
$this->assertEquals(403, $event->getThrowable()->getStatusCode());
171172
}
172173

174+
public function testUnregister()
175+
{
176+
$listener = $this->createExceptionListener();
177+
$dispatcher = new EventDispatcher();
178+
179+
$listener->register($dispatcher);
180+
$this->assertNotEmpty($dispatcher->getListeners());
181+
182+
$listener->unregister($dispatcher);
183+
$this->assertEmpty($dispatcher->getListeners());
184+
}
185+
173186
public function getAccessDeniedExceptionProvider()
174187
{
175188
return [

0 commit comments

Comments
 (0)