Problem/Motivation

Started seeing multiple errors in the logs with:

Warning: filesize(): stat failed for %fileuri in /app/web/modules/contrib/file_downloader/src/Plugin/DownloadOption/ImageStyle.php on line 64

From URLs matching /download/%imagestyle/%fileid

These appear to be cases where the file id exists but the image derivative has not been generated yet.

Steps to reproduce

View a page with the download link - the image styles are generated
Delete the image styles - e.g. 

drush if

Follow the link to download the image style - warning is logged & 404 is received

Proposed resolution

I think the 404 is fine - I don't want somebody to be able to bring the site down crawling random image ids without an image derivative token - but it would be nice if the controller do this without producing a warning.

Issue fork file_downloader-3476288

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ericgsmith created an issue. See original summary.

ericgsmith changed the visibility of the branch 3476288-check-file-exists to hidden.

ericgsmith changed the visibility of the branch 3476288-check-file-exists to active.

ericgsmith’s picture

ericgsmith commented
Title: Controller does not handle case where image derivate has not been generated » Controller does not handle case where image derivative has not been generated
ericgsmith’s picture

ericgsmith commented
Status: Active » Needs review

ericgsmith changed the visibility of the branch 3476288-on-3343547 to hidden.

ericgsmith opened merge request !4

ericgsmith’s picture

ericgsmith commented

Updated to revert previous attempt (since all this did was upgrade a warning to an error) and throw a 404 exception + explanation of why we don't want to fall back to generating the image style here.

Added branch 3476288-on-3343547 with this change onto #3343547: Make compatible with Drupal 9/10 since this module really needs that as well to work.

klidifia’s picture

klidifia commented
Status: Needs review » Reviewed & tested by the community

I tested the updated patch with the scenario outlined and confirm no warnings/errors and instead a 404 now -- looks good.

The code looks good and the comment explains nicely. Good stuff!

sjerdo made their first commit to this issue’s fork.

  • sjerdo committed 983657bd on 3.0.x authored by ericgsmith 
    Issue #3476288: Return 404 if image derivative does not exist
sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.