craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.23k stars 630 forks source link

[3.x]: Unexpected behavior when an Asset field is using a volume that has been set as the "Temp Uploads Location" #11405

Closed angrybrad closed 8 months ago

angrybrad commented 2 years ago

What happened?

Steps to reproduce

  1. On a fresh Craft 3.7.44 install, create a local Asset volume.
  2. Go to Settings->Assets->Settings and select that volume as the "Temp Uploads Location"
  3. Go to Assets Index page and create a subfolder. Upload an image to that subfolder.
  4. Create an Assets field and add it to a section's field layout.
  5. Create a new entry, click the Asset field, and select the uploaded asset from the subfolder.
  6. Save the entry.
  7. Notice, on the Assets Index page, that the asset now lives in the root of the volume, not the subfolder.

Expected behavior

The asset should be in the subfolder.

Actual behavior

The asset is at the root of the volume.

Notes

This is likely a change in behavior caused by https://github.com/craftcms/cms/commit/fa6fd6351d0e8d3453667b072d729c0e9dfd3367 that was released in 3.7.39.

Ideally, no one would select a volume that was intended to serve public assets, which mitigates this problem.

Things like the Asset field's source settings should remove any volumes so they can't be selected if "Temp Uploads Location" is set to a volume.

Craft CMS version

3.7.4.4

PHP version

7.4

Operating system and version

macOS

Database type and version

MariaDB 10

Image driver and version

imagick

Installed plugins and versions

None

jamie-s-white commented 10 months ago

This is still affecting Craft 4.5.9 and 3.9.5.

Can confirm it doesn't affect 3.7.16.

I wish I had seen this two days ago, I was just about to file a bug report with these very details... Although I absolutely agree that "no one would select a volume that was intended to serve public assets, which mitigates this problem", evidently my predecesor did, and lay this trap waiting for me when I started upgrading our Craft CMS websites past 3.7.16.

This issue also manifests itself when clicking preview, for whatever that is worth.

brandonkelly commented 8 months ago

We’ve added guardrails to Craft 4.7, to help avoid this going forward (#14141 + 97a08f8094a8c389253576228c0ff86eae283de7):

Craft 5 goes further, by removing the “Temp Uploads Location” setting altogether, in favor of a new tempAssetUploadFs config setting, which defines a filesystem handle (explicitly or via an environment variable) that should be used to store temp asset uploads. (#13957)

brandonkelly commented 8 months ago

Craft 4.7.0 is now tagged with those changes.