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

Cache keys with reserved characters are no longer accepted #5051

Closed
greg0ire opened this issue Nov 29, 2021 · 4 comments · Fixed by #5059
Closed

Cache keys with reserved characters are no longer accepted #5051

greg0ire opened this issue Nov 29, 2021 · 4 comments · Fixed by #5059

Comments

@greg0ire
Copy link
Member

greg0ire commented Nov 29, 2021

BC Break Report

Q A
BC Break yes
Version 3.2.0

Summary

Previous behaviour

It was possible to use a cache key with a reserved character, such as :

Current behavior

An exception is thrown from https://github.com/doctrine/cache/blob/331b4d5dbaeab3827976273e9356b3b453c300ce/lib/Doctrine/Common/Cache/Psr6/CacheAdapter.php#L265

How to reproduce

  1. Configure a doctrine/cache instance as a result cache (for instance, I've been using https://github.com/symfony/symfony/blob/0ac7aa230fe0177214415ee20fec113a44a261f1/src/Symfony/Component/Cache/DoctrineProvider.php#L60).
  2. Set a key with a : in it.

Possible solution

Use rawurlencode in doctrine/cache instead of throwing, release a new version, require that version.

In my case, the issue was easy to fix as we had only one occurrence of a key with a colon inside in our project, so if for some reason this is too hard to fix, we might instead just document the BC Break

@greg0ire greg0ire changed the title Cache keys with reserved character are no longer accepted Cache keys with reserved characters are no longer accepted Nov 29, 2021
@morozov
Copy link
Member

morozov commented Nov 29, 2021

While technically it is a BC break, I'd look more into the reasoning behind PSR-6 allowing its implementors not to support the special characters. It would be close to trivial to encode them but there may be reasons not to do that.

Personally, I never understood this limitation but now it might be a good time to understand it better.

@greg0ire
Copy link
Member Author

greg0ire commented Nov 30, 2021

From https://www.php-fig.org/psr/psr-6 :

The following characters are reserved for future extensions and MUST NOT be supported by implementing libraries: {}()/\@:

Apparently, special functionality might kick in if you use those (once such extensions are added to the standard).

There isn't much more detail over here: php-fig/fig-standards#786 or in the google group.

@morozov
Copy link
Member

morozov commented Nov 30, 2021

Encoding logic into values using reserved characters looks like… SQL? With all the possibilities for logic injections and thus the need for sanitization and then an API like prepared statements to avoid those? It looks like we've already been there.

Personally, I would rather document it as a minor BC break rather than try fixing something that won't work with the PSR-6 implementations anyways.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants