bug #52806 [Routing] Fix removing aliases pointing to removed route in RouteCollection::remove() (fancyweb)

nicolas-grekas · nicolas-grekas · commit ce95b8763a16 · 2023-11-29T18:08:03.000+01:00
This PR was merged into the 5.4 branch. Discussion ---------- [Routing] Fix removing aliases pointing to removed route in `RouteCollection::remove()` | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | | Issues | #52802 | License | MIT When a route is removed from the collection, we want to remove all aliases pointing to it. The implementation is flawed because the `$aliases` array is indexed by the alias name, and not by the route since several aliases can point to the same route. It'll fix the related issue because added FQCN aliases will be correctly removed when `PrefixTrait::addPrefix()` removes the route before reading it for each locale. Commits ------- 238894b [Routing] Fix removing aliases pointing to removed route in RouteCollection::remove()
diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php
@@ -157,8 +157,15 @@ public function get(string $name)
      */
     public function remove($name)
     {
-        foreach ((array) $name as $n) {
-            unset($this->routes[$n], $this->priorities[$n], $this->aliases[$n]);
+        $names = (array) $name;
+        foreach ($names as $n) {
+            unset($this->routes[$n], $this->priorities[$n]);
+        }
+
+        foreach ($this->aliases as $k => $alias) {
+            if (\in_array($alias->getId(), $names, true)) {
+                unset($this->aliases[$k]);
+            }
         }
     }
 
diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php
@@ -225,11 +225,13 @@ public function testRemove()
         $collection1->add('bar', $bar = new Route('/bar'));
         $collection->addCollection($collection1);
         $collection->add('last', $last = new Route('/last'));
+        $collection->addAlias('ccc_my_custom_alias', 'foo');
 
         $collection->remove('foo');
         $this->assertSame(['bar' => $bar, 'last' => $last], $collection->all(), '->remove() can remove a single route');
         $collection->remove(['bar', 'last']);
         $this->assertSame([], $collection->all(), '->remove() accepts an array and can remove multiple routes at once');
+        $this->assertNull($collection->getAlias('ccc_my_custom_alias'));
     }
 
     public function testSetHost()

Original file line number	Diff line number	Diff line change
`@@ -157,8 +157,15 @@ public function get(string $name)`
`157`	`157`	`*/`
`158`	`158`	`public function remove($name)`
`159`	`159`	`{`
`160`		`- foreach ((array) $name as $n) {`
`161`		`- unset($this->routes[$n], $this->priorities[$n], $this->aliases[$n]);`
	`160`	`+ $names = (array) $name;`
	`161`	`+ foreach ($names as $n) {`
	`162`	`+ unset($this->routes[$n], $this->priorities[$n]);`
	`163`	`+ }`
	`164`	`+`
	`165`	`+ foreach ($this->aliases as $k => $alias) {`
	`166`	`+ if (\in_array($alias->getId(), $names, true)) {`
	`167`	`+ unset($this->aliases[$k]);`
	`168`	`+ }`
`162`	`169`	`}`
`163`	`170`	`}`
`164`	`171`