[EventDispatcher] Fix removing listeners when using first-class callable syntax

javer · nicolas-grekas · commit 74289aadf822 · 2022-05-05T14:58:14.000+02:00
diff --git a/src/Symfony/Component/EventDispatcher/EventDispatcher.php b/src/Symfony/Component/EventDispatcher/EventDispatcher.php
@@ -108,7 +108,7 @@ public function getListenerPriority(string $eventName, callable|array $listener)
                     $v[0] = $v[0]();
                     $v[1] ??= '__invoke';
                 }
-                if ($v === $listener) {
+                if ($v === $listener || ($listener instanceof \Closure && $v == $listener)) {
                     return $priority;
                 }
             }
@@ -164,7 +164,7 @@ public function removeListener(string $eventName, callable|array $listener)
                     $v[0] = $v[0]();
                     $v[1] ??= '__invoke';
                 }
-                if ($v === $listener) {
+                if ($v === $listener || ($listener instanceof \Closure && $v == $listener)) {
                     unset($listeners[$k], $this->sorted[$eventName], $this->optimized[$eventName]);
                 }
             }
diff --git a/src/Symfony/Component/EventDispatcher/Tests/EventDispatcherTest.php b/src/Symfony/Component/EventDispatcher/Tests/EventDispatcherTest.php
@@ -405,6 +405,32 @@ public function testMutatingWhilePropagationIsStopped()
 
         $this->assertTrue($testLoaded);
     }
+
+    public function testNamedClosures()
+    {
+        $listener = new TestEventListener();
+
+        $callback1 = $listener(...);
+        $callback2 = $listener(...);
+        $callback3 = (new TestEventListener())(...);
+
+        $this->assertNotSame($callback1, $callback2);
+        $this->assertNotSame($callback1, $callback3);
+        $this->assertNotSame($callback2, $callback3);
+        $this->assertTrue($callback1 == $callback2);
+        $this->assertFalse($callback1 == $callback3);
+
+        $this->dispatcher->addListener('foo', $callback1, 1);
+        $this->dispatcher->addListener('foo', $callback2, 2);
+        $this->dispatcher->addListener('foo', $callback3, 3);
+
+        $this->assertSame(1, $this->dispatcher->getListenerPriority('foo', $callback1));
+        $this->assertSame(1, $this->dispatcher->getListenerPriority('foo', $callback2));
+
+        $this->dispatcher->removeListener('foo', $callback1);
+
+        $this->assertSame(['foo' => [$callback3]], $this->dispatcher->getListeners());
+    }
 }
 
 class CallableClass
diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php
@@ -322,6 +322,30 @@ public function testSessionIsNotReported()
         $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST));
     }
 
+    public function testOnKernelResponseRemoveListener()
+    {
+        $tokenStorage = new TokenStorage();
+        $tokenStorage->setToken(new UsernamePasswordToken(new InMemoryUser('test1', 'pass1'), 'phpunit', ['ROLE_USER']));
+
+        $request = new Request();
+        $request->attributes->set('_security_firewall_run', '_security_session');
+
+        $session = new Session(new MockArraySessionStorage());
+        $request->setSession($session);
+
+        $dispatcher = new EventDispatcher();
+        $httpKernel = $this->createMock(HttpKernelInterface::class);
+
+        $listener = new ContextListener($tokenStorage, [], 'session', null, $dispatcher, null, $tokenStorage->getToken(...));
+        $this->assertEmpty($dispatcher->getListeners());
+
+        $listener(new RequestEvent($httpKernel, $request, HttpKernelInterface::MAIN_REQUEST));
+        $this->assertNotEmpty($dispatcher->getListeners());
+
+        $listener->onKernelResponse(new ResponseEvent($httpKernel, $request, HttpKernelInterface::MAIN_REQUEST, new Response()));
+        $this->assertEmpty($dispatcher->getListeners());
+    }
+
     protected function runSessionOnKernelResponse($newToken, $original = null)
     {
         $session = new Session(new MockArraySessionStorage());
diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Security\Http\Tests\Firewall;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ExceptionEvent;
@@ -179,6 +180,18 @@ public function testLogoutException()
         $this->assertEquals(403, $event->getThrowable()->getStatusCode());
     }
 
+    public function testUnregister()
+    {
+        $listener = $this->createExceptionListener();
+        $dispatcher = new EventDispatcher();
+
+        $listener->register($dispatcher);
+        $this->assertNotEmpty($dispatcher->getListeners());
+
+        $listener->unregister($dispatcher);
+        $this->assertEmpty($dispatcher->getListeners());
+    }
+
     public function getAccessDeniedExceptionProvider()
     {
         return [
diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json
@@ -32,7 +32,7 @@
         "psr/log": "^1|^2|^3"
     },
     "conflict": {
-        "symfony/event-dispatcher": "<5.4",
+        "symfony/event-dispatcher": "<6.1",
         "symfony/security-bundle": "<5.4",
         "symfony/security-csrf": "<5.4"
     },

Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ public function getListenerPriority(string $eventName, callable\|array $listener)`
`108`	`108`	`$v[0] = $v[0]();`
`109`	`109`	`$v[1] ??= '__invoke';`
`110`	`110`	`}`
`111`		`- if ($v === $listener) {`
	`111`	`+ if ($v === $listener \|\| ($listener instanceof \Closure && $v == $listener)) {`
`112`	`112`	`return $priority;`
`113`	`113`	`}`
`114`	`114`	`}`
`@@ -164,7 +164,7 @@ public function removeListener(string $eventName, callable\|array $listener)`
`164`	`164`	`$v[0] = $v[0]();`
`165`	`165`	`$v[1] ??= '__invoke';`
`166`	`166`	`}`
`167`		`- if ($v === $listener) {`
	`167`	`+ if ($v === $listener \|\| ($listener instanceof \Closure && $v == $listener)) {`
`168`	`168`	`unset($listeners[$k], $this->sorted[$eventName], $this->optimized[$eventName]);`
`169`	`169`	`}`
`170`	`170`	`}`