bug symfony#52502 [Config] Prefixing FileExistenceResource::__toString() to avoid conflict with FileResource (weaverryan)

nicolas-grekas · nicolas-grekas · commit 9b2c2a4c2c36 · 2023-11-09T09:22:50.000+01:00
This PR was submitted for the 6.3 branch but it was merged into the 5.4 branch instead. Discussion ---------- [Config] Prefixing `FileExistenceResource::__toString()` to avoid conflict with `FileResource` | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fixes issue in AssetMapper 6.4 | License | MIT Hi! This bug causes a pretty critical AssetMapper 6.4 bug. The problem is that both `FileResource` and `FileExistenceResource` return the same `__toString()` for the same file: * [FileResource::__toString()](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/Config/Resource/FileResource.php#L43-L46) * [FileExistenceResource::__toString()](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/Config/Resource/FileExistenceResource.php#L39-L41) [SelfCheckingResourceChecker](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/Config/Resource/SelfCheckingResourceChecker.php#L40-L45) uses a static cache where the `ResourceInterface::__toString()` is the key for that string. That creates the following bug situation: A) Something checks for `FileExistenceResource` for `foo/bar.php`. This returns true and `SelfCheckingResourceChecker` now has `true` for fresh in its static cache B) Something else checks for `FileResource` for `foo/bar.php`: they are checking to see if the file has been *modified*. But in instead of calling the actual `FileResource::isFresh()` method, it uses the `true` value from the static cache. Cheers! Commits ------- 9e8db58 [Config] Prefixing FileExistenceResource::__toString() to avoid conflict with FileResource
diff --git a/src/Symfony/Component/Config/Resource/FileExistenceResource.php b/src/Symfony/Component/Config/Resource/FileExistenceResource.php
@@ -38,7 +38,7 @@ public function __construct(string $resource)
 
     public function __toString(): string
     {
-        return $this->resource;
+        return 'existence.'.$this->resource;
     }
 
     public function getResource(): string
diff --git a/src/Symfony/Component/Config/Tests/Resource/FileExistenceResourceTest.php b/src/Symfony/Component/Config/Tests/Resource/FileExistenceResourceTest.php
@@ -36,7 +36,7 @@ protected function tearDown(): void
 
     public function testToString()
     {
-        $this->assertSame($this->file, (string) $this->resource);
+        $this->assertSame('existence.'.$this->file, (string) $this->resource);
     }
 
     public function testGetResource()

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ public function __construct(string $resource)`
`38`	`38`
`39`	`39`	`public function __toString(): string`
`40`	`40`	`{`
`41`		`- return $this->resource;`
	`41`	`+ return 'existence.'.$this->resource;`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`public function getResource(): string`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ protected function tearDown(): void`
`36`	`36`
`37`	`37`	`public function testToString()`
`38`	`38`	`{`
`39`		`- $this->assertSame($this->file, (string) $this->resource);`
	`39`	`+ $this->assertSame('existence.'.$this->file, (string) $this->resource);`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`public function testGetResource()`