Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VersionProvider caching is broken #10490

Open
emteknetnz opened this issue Sep 5, 2022 · 3 comments
Open

VersionProvider caching is broken #10490

emteknetnz opened this issue Sep 5, 2022 · 3 comments

Comments

@emteknetnz
Copy link
Member

emteknetnz commented Sep 5, 2022

Discovered during #10478

The tests in CMS4 VersionedProviderTest currently pass when they kind of should not

The following $key assignments in VersionProvider generate keys with invalid characters

Where $key is a filesystem path

$key = sprintf('%s-%s', $this->getComposerLockPath(), 'all');
$key = sprintf('%s-%s', $this->getComposerLockPath(), $module);

The contains the / character, which is invalid - https://github.com/symfony/cache-contracts/blob/main/ItemInterface.php#L43
Called via:
https://github.com/symfony/cache/blob/4.4/CacheItem.php#L170
https://github.com/symfony/cache/blob/4.4/Simple/AbstractCache.php#L77

Marked as low priority because while the caching is broken, things still work perfectly fine on the front-end. So fixing this would only net a tiny performance benefit

Non issue in CMS 5 because the cache key validation logic isn't called

PR

@lekoala
Copy link
Contributor

lekoala commented Aug 1, 2023

Still an issue in SS5
#10889

@GuySartorelli
Copy link
Member

#10889 resolved this for CMS 5. Still an issue for CMS 4.

@lekoala
Copy link
Contributor

lekoala commented Aug 3, 2023

it's probably possible to apply the same fix to ss4 if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants