Skip to content

Commit

Permalink
bug #52502 [Config] Prefixing FileExistenceResource::__toString() t…
Browse files Browse the repository at this point in the history
…o avoid conflict with `FileResource` (weaverryan)

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
  • Loading branch information
nicolas-grekas committed Nov 9, 2023
2 parents 0ff9ed4 + 9e8db58 commit 9b2c2a4
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
Expand Up @@ -38,7 +38,7 @@ public function __construct(string $resource)

public function __toString(): string
{
return $this->resource;
return 'existence.'.$this->resource;
}

public function getResource(): string
Expand Down
Expand Up @@ -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()
Expand Down

0 comments on commit 9b2c2a4

Please sign in to comment.