[qmeister]

This pull request fixes an infinite recursion when entries have link fields that lead back to each other (A → B → A, a longer cycle, or an entry linking to itself) and the entry is converted to an array or JSON, e.g. via Entry::toArray(), json_encode($entry) or {{ entry | to_json }}.

Fixes #9105. Also reported in #14696.

Cause

AugmentedCollection::toArray() evaluates nested arrays with (new self($value))->withEvaluation()->toArray(). When it meets an ArrayableLink it calls ArrayableLink::toArray(), which returns toAugmentedArray() of the linked entry: the full augmented entry, including its own links. If one of those links leads back to an entry that is already being expanded, it never ends.

Fix

AugmentedCollection keeps track of the linked items that are currently being expanded. A link that leads back to one of them is represented by the shallow array (what ArrayableLink::jsonSerialize() already returns), all other links are expanded in full as before.

This is the approach suggested in #15386, which was closed because always using the shallow array would change the output for links that don't form a cycle. With the guard, the output is unchanged for everyone without a cycle. The tracking state is removed in a finally block, so an exception during evaluation doesn't leave anything behind.

Tests

tests/Data/Entries/CyclicLinkTest.php converts real entries to arrays:

• two entries that link to each other
• an entry that links to itself
• a cycle of three entries
• a chain without a cycle, which must still return the full data of every linked entry
• converting the same entry twice returns the same result
• The link fieldtype in the test counts how often it augments and throws after 25 calls, so without the fix the tests fail with a clear message instead of exhausting memory. Without the change in AugmentedCollection four of the five tests fail, with it they all pass.

How to test

Automated:

vendor/bin/phpunit tests/Data/Entries/CyclicLinkTest.php

To see the tests fail without the fix, restore src/Data/AugmentedCollection.php to its state on 6.x (git checkout 6.x -- src/Data/AugmentedCollection.php) and run the same command: four of the five tests fail with "Links are being augmented recursively.".

Manually:

1. Add a link field to a collection's blueprint.
2. Create two entries, A and B, where A links to B and B links to A.
3. Convert A to an array or JSON, e.g. with {{ collection:pages as="pages" }}{{ pages | to_json }}{{ /collection:pages }} in a template, or json_encode(Entry::find($id)) in tinker.

Before this change the request runs until PHP's time or memory limit is hit, after this change it returns, with the link that leads back to an entry that is already being expanded as its shallow array. I also ran this change against a real-world site where the REST API ran into this (a page linking to an event with a taxonomy terms field, which brought the cycle back to the page): the recursion no longer occurs.

[duncanmcclean]

This seems to fix the same problem as #15288 🤔

[qmeister]

This seems to fix the same problem as #15288 🤔

Thanks for flagging @duncanmcclean. I do think these differ though: #15288 guards cycles in an entry’s origin (translations) at save time, while this PR fixes link fields that point back to each other (valid content) while converting an entry to an array/JSON in AugmentedCollection::toArray().

I ran both on current 6.x: with only #15288, 4 of the 5 tests in CyclicLinkTest still fail (same as plain 6.x); with both applied, everything passes.