Circular reference detection broken for HAL format

[carlobeltrame]

API Platform version(s) affected: 2.6.5, but probably since the introduction of HAL or circular reference limits, whichever came last

Description
Symfony's circular reference detection works by storing counters keyed by object hashes in the context. Every time an object is normalized, there is a check in AbstractObjectNormalizer#normalize that the circular reference counter of the normalized object is not too high already, and if it is, the circular reference handler is called. If it's not, the normalizer continues normally, and recursively normalizes nested relations using the getAttributeValue method, passing in the context including the counters. This all works well, also for the JSON+LD format in API platform.
When using the HAL JSON format however, the relations are not serialized as attributes, presumably because they need to be sorted into the _links and _embedded arrays. For this purpose, the HAL ItemNormalizer uses the standard Symfony normalizer for serializing all non-resource attributes, without any recursion happening. The context outside the Symfony normalizer is never mutated. Later, the HAL ItemNormalizer calls getAttributeValue on its own for all relations, passing in the untouched context without any counters. Therefore, the circular reference handler never kicks in, and we get memory / recursion problems when there are circular references (unless we use a very shallow max depth, which is clearly inferior to readableLink + automatic circular reference detection)

How to reproduce
Have two entities that reference each other and embed each other (readableLink: true in both directions). Make sure to also add a max depth, so you don't run into infinite eager loaded joins, see #2444.

/**
 * @ORM\Entity
 **/
#[ApiResource(normalizationContext: ['groups' => ['A:read'], 'enable_max_depth' => true])]
class A {
  #[ApiProperty(readableLink: true)]
  #[Groups(['A:read', 'B:read'])]
  #[MaxDepth(6)]
  public B $b;
}

/**
 * @ORM\Entity
 **/
#[ApiResource(normalizationContext: ['groups' => ['B:read'], 'enable_max_depth' => true])]
class B {
  #[ApiProperty(readableLink: true)]
  #[Groups(['A:read', 'B:read'])]
  #[MaxDepth(6)]
  public A $a;
}

Create an A and a B that reference each other, and try to GET either of them using the application/hal+json format. They will be repeatedly embedded within each other, until they hit the max depth.

Possible Solution
I don't have an immediate solution proposal, it seems to me like the HAL serializer shouldn't separate the serialization of the relations from the rest of the attributes. But then again, it probably needs to, because the same relation sometimes needs to be serialized in two different ways for HAL, for relations that are embedded as well as linked at the same time.

I have created a decorator for the HAL ItemNormalizer that works around the problem by duplicating the Symfony circular reference detection logic for just the HAL normalizer:

final class CircularReferenceDetectingHalItemNormalizer extends AbstractItemNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface {
    /**
     * @internal
     */
    protected const HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS = 'hal_circular_reference_limit_counters';

    public function __construct(private NormalizerInterface $decorated, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, IriConverterInterface $iriConverter, ResourceClassResolverInterface $resourceClassResolver, PropertyAccessorInterface $propertyAccessor = null, NameConverterInterface $nameConverter = null, ClassMetadataFactoryInterface $classMetadataFactory = null, ItemDataProviderInterface $itemDataProvider = null, bool $allowPlainIdentifiers = false, array $defaultContext = [], iterable $dataTransformers = [], ResourceMetadataFactoryInterface $resourceMetadataFactory = null, ResourceAccessCheckerInterface $resourceAccessChecker = null) {
        $defaultContext[AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER] = function ($object) {
            return ['_links' => ['self' => ['href' => $this->iriConverter->getIriFromItem($object)]]];
        };
        parent::__construct($propertyNameCollectionFactory, $propertyMetadataFactory, $iriConverter, $resourceClassResolver, $propertyAccessor, $nameConverter, $classMetadataFactory, $itemDataProvider, $allowPlainIdentifiers, $defaultContext, $dataTransformers, $resourceMetadataFactory, $resourceAccessChecker);
    }

    /**
     * {@inheritdoc}
     */
    public function supportsNormalization($data, $format = null): bool {
        return $this->decorated->supportsNormalization($data, $format);
    }

    /**
     * {@inheritdoc}
     */
    public function normalize($object, $format = null, array $context = []) {
        if ($this->isHalCircularReference($object, $context)) {
            return $this->handleHalCircularReference($object, $format, $context);
        }

        return $this->decorated->normalize($object, $format, $context);
    }

    /**
     * {@inheritdoc}
     */
    public function supportsDenormalization($data, $type, $format = null): bool {
        return $this->decorated->supportsDenormalization($data, $type, $format);
    }

    /**
     * {@inheritdoc}
     *
     * @throws LogicException
     */
    public function denormalize($data, $class, $format = null, array $context = []) {
        return $this->decorated->denormalize($data, $class, $format, $context);
    }

    public function setSerializer(SerializerInterface $serializer) {
        if ($this->decorated instanceof SerializerAwareInterface) {
            $this->decorated->setSerializer($serializer);
        }
    }

    /**
     * Detects if the configured circular reference limit is reached.
     *
     * @return bool
     *
     * @throws CircularReferenceException
     */
    protected function isHalCircularReference(object $object, array &$context)
    {
        $objectHash = spl_object_hash($object);

        $circularReferenceLimit = $context[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT] ?? $this->defaultContext[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT];
        if (isset($context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash])) {
            if ($context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash] >= $circularReferenceLimit) {
                unset($context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash]);

                return true;
            }

            ++$context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash];
        } else {
            $context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash] = 1;
        }

        return false;
    }

    /**
     * Handles a circular reference.
     *
     * If a circular reference handler is set, it will be called. Otherwise, a
     * {@class CircularReferenceException} will be thrown.
     *
     * @final
     *
     * @return mixed
     *
     * @throws CircularReferenceException
     */
    protected function handleHalCircularReference(object $object, string $format = null, array $context = [])
    {
        $circularReferenceHandler = $context[AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER] ?? $this->defaultContext[AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER];
        if ($circularReferenceHandler) {
            return $circularReferenceHandler($object, $format, $context);
        }

        throw new CircularReferenceException(sprintf('A circular reference has been detected when serializing the object of class "%s" (configured limit: %d).', get_debug_type($object), $context[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT] ?? $this->defaultContext[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT]));
    }
}

Service configuration:

# config/services.yaml
services:
    App\Serializer\Normalizer\CircularReferenceDetectingHalItemNormalizer:
        decorates: 'api_platform.hal.normalizer.item'

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[bendavies]

well this is still an issue. it's not stale because a bot says so