Skip to content

[Cache] Limit cache version character range #38126

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 10, 2020
Merged

[Cache] Limit cache version character range #38126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Component/Cache/Simple/AbstractCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ private function generateValues(iterable $values, array &$keys, $default): itera
try {
foreach ($values as $id => $value) {
if (!isset($keys[$id])) {
$id = key($keys);
throw new InvalidArgumentException(sprintf('Could not match value id "%s" to keys "%s".', $id, implode('", "', $keys)));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what lead to the mix up of values. The assumption here is: if the id of the value cannot be found in the original keys, let’s just assume it to be the first one. Instead of returning a wrong value, we should create a miss here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize I wasn’t precise in my language: this is not the root cause of the issue but the subsequent error that lead to values being mixed up. The root cause was that they version ID contained a slash, that slash was replaced with a null byte in decodeKey and therefore the keys no longer matched and the piece of code above was triggered.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, we missed patching this other file, now fixed:
25941ff

}
$key = $keys[$id];
unset($keys[$id]);
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Cache/Traits/AbstractTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public function clear(/*string $prefix = ''*/)
}
}
$namespaceToClear = $this->namespace.$namespaceVersionToClear;
$namespaceVersion = substr_replace(base64_encode(pack('V', mt_rand())), static::NS_SEPARATOR, 5);
$namespaceVersion = strtr(substr_replace(base64_encode(pack('V', mt_rand())), static::NS_SEPARATOR, 5), '/', '_');
try {
$cleared = $this->doSave([static::NS_SEPARATOR.$this->namespace => $namespaceVersion], 0);
} catch (\Exception $e) {
Expand Down Expand Up @@ -268,7 +268,7 @@ private function getId($key): string
$this->namespaceVersion = $v;
}
if ('1'.static::NS_SEPARATOR === $this->namespaceVersion) {
$this->namespaceVersion = substr_replace(base64_encode(pack('V', time())), static::NS_SEPARATOR, 5);
$this->namespaceVersion = strtr(substr_replace(base64_encode(pack('V', time())), static::NS_SEPARATOR, 5), '/', '_');
$this->doSave([static::NS_SEPARATOR.$this->namespace => $this->namespaceVersion], 0);
}
} catch (\Exception $e) {
Expand Down