verbb / navigation

A Craft CMS plugin to create navigation menus for your site.
Other
90 stars 23 forks source link

Using getElement() causing folders to be created under runtime/assets/tempuploads #289

Closed jimirobaer closed 2 years ago

jimirobaer commented 2 years ago

Description

We have detected an issue where huge amounts of folders were created in the storage/runtime/tempuploads folder of Craft CMS. We tracked down the issue and noticed that it was caused by specific function used when Twig templating with this plugin.

Each folder name inside the tempuploads folder starts with "user_" with a hash after it. The function behind creating these folders is from Craft CMS, which you can find at: https://github.com/craftcms/cms/blob/7c01428969efc6f05550de5bcbf032374ee47b8b/src/services/Assets.php#L1073. It might seem to be related to Assets alone, but it the problem get triggered even if you don't query any fields from the Entry object.

As a workaround: not using getElement() but getting the node's Entry object using craft.entries.getElementById(node.elementId) does not trigger the issue.

Steps to reproduce

  1. Install clean Craft CMS 3 project (using Composer), install the latest version of the Navigation plugin from Plugin Store
  2. Create a navigation, use default settings and add an entry through the CMS (any section)
  3. Get all nodes using {% craft.navigation.nodes('navHandle').all() %}
  4. Loop over the nodes and get the Entry element using {% set element = node.getElement() %}
  5. See folders getting created under storage/runtime/tempuploads on each session/unique visit (using private window)

Additional info

Additional context

Full code example which triggers the bug:

{% set navHandle = craft.navigation.nodes('navHandle').all() %}

{% nav node in navHandle %}
    {% set element = node.getElement() %}
    {{ node.link }}
{% endnav %}

We noticed this issue in several different projects, with different Craft CMS versions (3.5.16, 3.6.16, 3.7.16) and the plugin version (1.4.16, 1.4.17, 1.4.21, 1.4.24).

These folders also don't get deleted by any cache clearing actions or housekeeping performed in the CMS, but that's probably a seperate issue with Craft CMS.

engram-design commented 2 years ago

Thanks for the detailed info, and that seems incredibly strange - but I can reproduce this. I believe this is due to how we register elements that Navigation supports, where we were fetching all the available sources for the element, which was causing the user uploads folders to be created.

Plainly an oversight on my part, that I didn't realise (or notice) that these folders would be created. But there's also no benefit to querying the sources of each element in this context, for the front-end. So there's also some performance benefits to removing this, as we only need this in the control panel.

Should be fixed for the next release. To get the fix early, change your verbb/navigation requirement in composer.json to:

"require": {
  "verbb/navigation": "dev-craft-3 as 1.4.25",
  "...": "..."
}

Then run composer update.

jimirobaer commented 2 years ago

Awesome, thanks for the prompt reply.

engram-design commented 2 years ago

Fixed in 1.4.26