Skip to content
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.

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

ArrayableLink in Statamic >4.32 causes infinite loop due to array augmentation #9105

Open
mkwia opened this issue Dec 1, 2023 · 10 comments
Open

Comments

@mkwia
Copy link

mkwia commented Dec 1, 2023

Bug description

After upgrading to Statamic 4.33, some of our sites became broken as a result of the changes introduced in #8911

In a situation where
Link A -> Link B -> Link C -> Link A
the toAugmentedArray method causes an infinite loop which breaks the site.

When testing locally we were able to resolve this by changing toAugmentedArray to toShallowAugmentedArray on

? $this->value->toAugmentedArray()

This issue seems similar to #9012

If we verify this fully we will submit a pull request.

How to reproduce

Create a link recursion as described above on versions of statamic > 4.32.

We have not been able to consistently reproduce this on all sites but it seems to affect both regex and runtime parsers.

We will update this issue if we find any additional information on this

Logs

No response

Environment

Environment
Application Name: abcdabcdabcdabcd
Laravel Version: 10.32.1
PHP Version: 8.2.12
Composer Version: 2.6.5
Environment: local
Debug Mode: ENABLED
URL: localhost:8080/
Maintenance Mode: OFF

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: null
Cache: redis
Database: mysql
Logs: daily
Mail: ses
Queue: redis
Session: file

Statamic
Addons: 6
Antlers: runtime
Stache Watcher: Enabled
Static Caching: Disabled
Version: 4.33.0 PRO

Statamic Addons
doublethreedigital/runway: 5.5.0
edalzell/forma: 2.1
silentz/akismet: 4.0.1
statamic/eloquent-driver: 2.6.1
withcandour/aardvark-seo: 3.0.0
withcandour/statamic-markdown-table: 1.0.0-beta

Statamic Eloquent Driver
Asset Containers: file
Assets: file
Blueprints: file
Collection Trees: file
Collections: file
Entries: eloquent
Forms: file
Global Sets: file
Global Variables: file
Navigation Trees: file
Navigations: file
Revisions: file
Taxonomies: file
Terms: file

Installation

Fresh statamic/statamic site via CLI

Antlers Parser

None

Additional details

No response

@duncanmcclean
Copy link
Member

duncanmcclean commented Dec 1, 2023

Hey! 👋

If you revert your changes in ArrayableLink.php and try commenting out line 869 of vendor/statamic/cms/src/Entries/Collection.php, are you still experiencing the infinite loop issue?

If you are, then this is the same issue as #9012.

@duncanmcclean
Copy link
Member

We've reverted #8928 in the latest release which I believe was the cause of this issue.

@afonic
Copy link

afonic commented Jan 10, 2024

@duncanmcclean I have the same issue at exactly 4.32.0 and I found my way here. After upgrading my site times out.

The fix @mkwia suggests works.

I am not sure I understand why this fails, in my case it seems like it's part of a code that uses json_encode in some entries that I use as part of a collection that defines a "mega menu" like part of the website. I am guessing json_encode causes some kind of infinite loop.

Any ideas?

To be honest, I am not sure why this was changed at the first place, a link should be a link if I wanted to get the entry data I would use an Entry relationship field instead. I heavily use links and augmenting each of them to bring the whole entry seems like quite a huge performance hit. Not really blaming anyone (tone is sometimes hard to read in text) just trying to understand how I should move forward.

@duncanmcclean
Copy link
Member

duncanmcclean commented Jan 11, 2024

Okay, re-opening and we can take a further look. There was a couple other augmentation / infinite loop issues that PR caused so I assumed it would fix this one too but obviously not.

To be honest, I am not sure why this was changed at the first place, a link should be a link if I wanted to get the entry data I would use an Entry relationship field instead. I heavily use links and augmenting each of them to bring the whole entry seems like quite a huge performance hit. Not really blaming anyone (tone is sometimes hard to read in text) just trying to understand how I should move forward.

I think it was added so you can do things like {{ link_field:title }} to get the title of the page you're linking to, whilst not needing a separate Entries field.

There's probably something we can do to avoid the infinite loop.

@duncanmcclean
Copy link
Member

duncanmcclean commented Jan 11, 2024

I am not sure I understand why this fails, in my case it seems like it's part of a code that uses json_encode in some entries that I use as part of a collection that defines a "mega menu" like part of the website. I am guessing json_encode causes some kind of infinite loop.

@afonic What does your template look like? I can't reproduce the bug using the original reproduction steps.

@afonic
Copy link

afonic commented Jan 11, 2024

@duncanmcclean it's something like this:

{{ collection:menu as="menu" }}
<component
    :menu='{{ menu | to_json | entities }}'
    :site='{{ site:handle | to_json }}'
>
</component>
{{ /collection:menu }}

I am trying to filter out items until I find what causes it, in the menu collection I heavily use link fields to entries.

@duncanmcclean
Copy link
Member

Just to confirm, it isn't fixed if you try upgrading to the latest version?

@afonic
Copy link

afonic commented Jan 11, 2024

Nope, it is fixed after reverting back to v4.31 or just reverting the changes made to Link.php

I will investigate further during the weekend.

@afonic
Copy link

afonic commented Mar 1, 2024

I managed to do some tests for this, it seems to happen only in nested Link fields, for example I have a Replicator field which has a set that has an Entries field and these entries have a Link field that points to another entry.

However I failed to reproduce it in a clean install so I just changed my blueprints around to move past this.

@n-va
Copy link

n-va commented Apr 15, 2024

We're able to recreate this with links when they're referring to each other.

Ex: Page A links to Page B, and Page B links to Page A, they are also nested inside sections.

The fix mkwia suggested with swapping to a ->shallowAugementedArray() in Link\ArrayableLink

Page A

---
id: 633fd516-fa95-401e-824b-48ff972293bb
blueprint: page
title: 'Page A'
sections:
  -
    id: lv0cygaq
    header: test
    buttons:
      -
        id: lv0cyhlk
        text: asdad
        link: 'entry::ee0d09b8-b33f-4541-9d9f-08ece596f2d8'
    invert_colour: false
    include_icon: false
    type: call_to_action
    enabled: true
updated_by: 6
updated_at: 1713149938
parent: cd3cda89-07b6-4ce0-a79d-cfbab52d2931
---

Page B

---
id: ee0d09b8-b33f-4541-9d9f-08ece596f2d8
blueprint: page
title: 'Page B'
sections:
  -
    id: lv0cxm72
    header: asda
    buttons:
      -
        id: lv0cxn1k
        text: ee
        link: 'entry::633fd516-fa95-401e-824b-48ff972293bb'
    invert_colour: false
    include_icon: false
    type: call_to_action
    enabled: true
parent: cd3cda89-07b6-4ce0-a79d-cfbab52d2931
updated_by: 6
updated_at: 1713149951
---

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants