laminas-cache icon indicating copy to clipboard operation
laminas-cache copied to clipboard

`Serializer` plugin does not serialize `token` argument for `checkAndSetItem`

Open boesing opened this issue 4 years ago • 0 comments

Bug Report

Q A
Version(s) 2.13.0

Summary

When a cache storage is used with the Serializer plugin, the Serializer plugin is pre-processing arguments for further usage. When checkAndSetItem is used, the Serializer plugin is listening on the checkAndSetItem.pre event and delegates the arguments to the onWriteItemPre handler. This handler is re-used from replaceItem(s), addItem(s) and setItem(s). None of these methods have a token argument and thus, it is not pre-processing the token at all, which results into an invalid assertion in the AbstractAdapter#internalCheckAndSetItem or by the method implemented by a storage.

protected function internalCheckAndSetItem(& $token, & $normalizedKey, & $value)
{
    $oldValue = $this->internalGetItem($normalizedKey);
    if ($oldValue !== $token) {
        return false;
    }

    return $this->internalSetItem($normalizedKey, $value);
}

Current behavior

Every oldValue with token comparison fails due to unserialized token vs serialized oldValue (due to the usage of internalGetItem rather than getItem).

How to reproduce

    public function testCanCompareOldValueWithTokenWhenUsedWithSerializerPlugin(): void
    {
        $storage = $this->getMockForAbstractAdapter();
        $storage
            ->addPlugin(new Serializer());

        $storage
            ->expects(self::once())
            ->method('internalGetItem')
            ->with('foo')
            ->willReturn(serialize('bar'));

        self::assertTrue($storage->checkAndSetItem('bar', 'foo', 'baz'));
    }

Expected behavior

Assertion passes. The Serializer plugin should convert the token the same way as it does for the value. I am experiencing this with the RedisCluster adapter at the moment but this might be the case for other adapters as well. To workaround, I'll use RedisCluster::OPT_SERIALIZER instead and thus, I just create this Issue while not fixing it as this is the behavior since at least many years.

boesing avatar Aug 12 '21 07:08 boesing