State leak in worker mode: objectHashes not cleared on exception in TypedMapperInterface

[Cafeine42]

Hello,

Using FrankenPHP in worker mode with Caddy, objectHashes is not cleared if an error occurs during MicroMapper usage.

When an error is thrown, the map code is not wrapped in a try/finally, so objectHashes isn't reset. This can cause spl_object_hash() to return a reused value on a new object in a future request, leading to side effects.

In our case, this happens when validation exceptions are thrown from implementations of TypedMapperInterface, which is part of the expected behavior.

Suggested fix: maybe wrap processing in try/finally

[JParkinson1991]

I'm also experiencing something similar using frankenphp in worker mode.

Unsure of the cause as yet but seeing instances where objects are missing properties set in the populate method which is indicative of the current depth being greater than max depth.. i.e. the max depth/state not being cleared as expected.

I'll continue to look into this but I do think there is an issue here.

[Cafeine42]

I used this variation of the MicroMapper service with a try/catch that resets the state and the ResetInterface.
We haven't had any new mistakes since but it's still recent.

namespace App\Service;

use Symfony\Component\DependencyInjection\Attribute\AsAlias;
use Symfony\Contracts\Service\ResetInterface;
use Symfonycasts\MicroMapper\MapperConfig;
use Symfonycasts\MicroMapper\MicroMapperInterface;

#[AsAlias(MicroMapperInterface::class)]
#[AsAlias('symfonycasts.micro_mapper')]
class MicroMapper implements MicroMapperInterface, ResetInterface
{
    /**
     * @var string[]
     */
    private array $objectHashes = [];

    private int $currentDepth = 0;
    private ?int $maxDepth = null;

    /**
     * @param MapperConfig[] $mapperConfigs
     */
    public function __construct(private array $mapperConfigs = [])
    {
    }

    public function addMapperConfig(MapperConfig $mapperConfig): void
    {
        $this->mapperConfigs[] = $mapperConfig;
    }

    public function reset(): void
    {
        $this->objectHashes = [];
        $this->currentDepth = 0;
        $this->maxDepth = null;
    }

    /**
     * @param mixed[]&array{max_depth?: int} $context
     */
    public function map(object $from, string $toClass, array $context = []): object
    {
        try {
            ++$this->currentDepth;

            if ($this->currentDepth > 50) {
                throw new \Exception('Max depth reached');
            }

            // set the max depth if not already set
            // the max depth is recorded as MAX_DEPTH + the current depth
            $previousMaxDepth = $this->maxDepth;
            if (isset($context[self::MAX_DEPTH])
                && (null === $this->maxDepth || ($context[self::MAX_DEPTH] + $this->currentDepth) < $this->maxDepth)
            ) {
                $this->maxDepth = $context[self::MAX_DEPTH] + $this->currentDepth;
            }

            $shouldFullyPopulate = null === $this->maxDepth || $this->currentDepth < $this->maxDepth;

            // watch for circular references, but only if we're fully populating
            // if we are not fully populating, this is already the final depth/level
            // through the micro mapper.
            if (isset($this->objectHashes[spl_object_hash($from)]) && $shouldFullyPopulate) {
                throw new \Exception(\sprintf('Circular reference detected with micro mapper: %s. Try passing [MicroMapperInterface::MAX_DEPTH => 1] when mapping relationships.', implode(' -> ', array_merge($this->objectHashes, [$from::class]))));
            }

            $this->objectHashes[spl_object_hash($from)] = $from::class;

            foreach ($this->mapperConfigs as $mapperConfig) {
                if (!$mapperConfig->supports($from, $toClass)) {
                    continue;
                }

                $toObject = $mapperConfig->getMapper()->load($from, $toClass, $context);

                // avoid fully populated objects if max depth is reached
                if (null === $this->maxDepth || $this->currentDepth < $this->maxDepth) {
                    $toObject = $mapperConfig->getMapper()->populate($from, $toObject, $context);
                }

                unset($this->objectHashes[spl_object_hash($from)]);
                --$this->currentDepth;
                $this->maxDepth = $previousMaxDepth;

                return $toObject;
            }
        } catch (\Exception $e) {
            $this->objectHashes = [];
            $this->currentDepth = 0;
            $this->maxDepth = null;

            throw $e;
        }

        throw new \Exception(\sprintf('No mapper found for %s -> %s', $from::class, $toClass));
    }

    /**
     * @param iterable<object>               $fromIterable
     * @param mixed[]&array{max_depth?: int} $context
     */
    public function mapMultiple(iterable $fromIterable, string $toClass, array $context = []): array
    {
        $toObjects = [];

        foreach ($fromIterable as $from) {
            $toObjects[] = $this->map($from, $toClass, $context);
        }

        return $toObjects;
    }
}

[sadikoff]

@Cafeine42

Thanks for sharing your solution, BTW if it's a real issue to you feel free to make a PR it's always a good way to improve the code!

Cheers!