use_imagecache_root TRUE doesn't work with webp any more

I can confirm downgrading to 3.1.0 fixes this issue for us.

We had the same issue with WebP module and Stage File Proxy. After rolling back to 3.1.0 the issue is gone.

Same case. 3.1.0 is the last working version.

MRs are welcome

I think the image download controller has to similar things. \Drupal\image\Controller\ImageStyleDownloadController::deliver uses \Drupal\image\Controller\ImageStyleDownloadController::getUriWithoutConvertedExtension

Being hit by the same issue.

Is going back to 3.1.0 currently the only solution?

Besides disabling image style Root ofc.

I've pushed the potential fix for this problem with the explanations in comments.

Someone experiencing this issue want to test out the MR?

Hi, I just came here because after enabling Webp module on the develop environment the images were no longer downloaded through Stagefileproxy. Production has no Webp module enabled at this point so a web style cannot be downloaded.
Only after using the patch from the MR the original image started to download.

I did hit a timeout though, some files may be quite large to download. Not sure if tweaking the timeout is something the Stage File Proxy module should provide?

using 99 MR, original jpeg is downloaded as expected, however file is saved using .webp extension.

edit: added a check for existence of source image and saving the file with the correct extension. codes not pretty.

https://git.drupalcode.org/project/stage_file_proxy/-/merge_requests/102...

Now we have 2 MRs, should only have 1

I think we want to handle all image types. Not just webp. AVIF just got added to 11.2 and will face a similar issue to webp if we don't address this in a more generic manner. As mentioned in #20, the competing MRs 99 vs 102 should be resolved.

I updated the #99 branch with the solution of #102.
Let Claude code handle the code, tested it myself.

Solution Evolution:

MR #99 introduced the foundation:
- Refactored fetchResponse() to attempt multiple image extensions (.jpg, .jpeg, .png,
.svg, .webp)
- Added Content-Type validation to prevent error pages from being treated as images
- Improved exception handling

MR #102 refined the approach:
- Critical change: Modified fetchResponse() to return an array containing both the HTTP
response and detected file extension
- Implemented destination filename adjustment to match the actual downloaded format
- Enhanced EventSubscriber with WebP source file detection for alternative formats

Current Implementation adds:
- Full .avif support across detection logic
- Complete modern image optimization workflow support (JPG → AVIF)

Key Benefit:
Images that are style-converted on production servers (JPG → WebP/AVIF) are now correctly
detected, downloaded with the proper file format, and saved with the correct extension
for subsequent image style processing.

I did not test the AVIF support.

Can confirm the updated MR #99 is working for webp files. Thank you. I can see the code to handle avif files there as well but I don't use avif so haven't had a chance to test. Will leave some comments on the MR.

Patch from MR # 99

I've hidden the !102 MR now that it is integrated in !99.

There's some review comments, from yours truly and others.

Patch from the MR wasn't working for me. So I threw Claude Code at it.

pathinfo()['extension'] returns 'webp' (no leading dot), but $this->imageSourceExtensions contains ['.jpg', '.jpeg', '.png', '.svg', '.webp', '.avif'] (with leading dots). So in_array('webp', ['.webp', ...]) is always FALSE, meaning $is_image is never true, and the patch's
extension-trying loop never tries alternative extensions.

The fix is simple — prepend a dot before the comparison:

So I jury rigged the patch to add a dot in:

+    $is_image = in_array('.' . $path_info['extension'], $this->imageSourceExtensions, TRUE);

And now it works for me. Apologies for not updating the MR myself, but if someone with the project cloned can easily do so, please do.

I encountered another small issue in that if the original image has different capitalization on the extension, Stage File Proxy cannot find the original image. For example an image located at example.JPG or example.Jpg cannot be located once its extension has been changed to example.webp.

I'm not sure if that's a solvable/worthwhile problem for this issue to handle, but it's an additional caveat in using webp image derivatives.

Still some old unaddressed review comments, and I also added a few new ones. Also, I think this issue needs some tests.

Can we also update the issue summary here please, seems to be doing way more then what the title says. But am intrigued and will apologize stage_file_proxy doesn't get as much of my attention.

Just throwing it out there but instead of Settings config what about some kind of hook? May overlap with the idea from https://www.drupal.org/project/stage_file_proxy/issues/3173364

I could be wrong so I did a new MR to not muck up the exiting one but what do we think about an event dispatcher?

There's also another issue about web: #3529745: Add support of ImageAPI Optimize WebP

I could be wrong so I did a new MR to not muck up the exiting one but what do we think about an event dispatcher?

Yeah, I guess that could work. The reasoning being that adding extra extensions probably means extra file types, which may need more that "just" listing the file type in a setting? From a 1000ft view, the other MR has some additions to the README.md that !109 probably also needs. Other than that, looks good.

Updated !99 for easier comparison to !109 (by merging in 4.0.x main branch). I need both a patch for this issue and for #2928564: Add stage_file_proxy stream wrapper and they conflict hard :) I was looking into whether it makes sense to introduce the FileFetchInfo pattern from #2928564: Add stage_file_proxy stream wrapper here as well. I'm not a great fan of the array being returned from fetchResponse() in both the MRs for this issue. One suggestion I came across was to update the FileFetchInfo object (that replaces the separate arguments for fetchResponse in the other issue) with the correct information for the original file that is downloaded, including the correct extension. I just realised I am no longer using the webp module, so I may actually not need to patch this issue, which would make life easier, at least for now.

Again, not to step on anything, !113 is a continuation of !109, but with the fetchInfo DTO from #2928564: Add stage_file_proxy stream wrapper. There was no real need for a FetchInfoService yet, since it is currently just encapsulating the fetch data that is hard coded. The main advantage this gives us now is that it gives us a way to pass information about the actually fetched file upwards without introducing an array holding multiple return values in DownloadManager::fetchResponse(). Also, as a secondary benefit, hopefully it paves the way for a somewhat cleaner merge of #2928564: Add stage_file_proxy stream wrapper.

There are a few changes to the version of FileFetchInfo from #2928564: Add stage_file_proxy stream wrapper:

• Domain is now accepted as an argument to the constructor; in the original version, the construction and setting of the domain happened in different places, so a separate method made a certain amount of sense, but here it was just looking silly, with the constructor and than a separate setDomain call. I can even imagine that a StreamWrapper in the architecture from #2928564: Add stage_file_proxy stream wrapper might actually know a better origin domain (e.g. an S3 setup where there is a CDN in front of the bucket that bypasses the production Drupal instance).
• I dropped the access type; this wasn't being used in the other issue either, and seems like a bit of a code smell, since it would seem to map 1:1 to public and private storage schemes, but the differences between those are supposed to be encapsulated in the stream wrappers. If it were intended as a flag to indicate that SFP needs to do something extra to obtain authentication, or something, again, it wasn't used, and I think it might make plenty of sense to implement that as a separate flag that can be flipped with a setter method. However, TBH, I find the idea that SFP would need to punch a hole in access control to certain files on production scarrier by the day and am wondering if that is something SFP should want to solve at all).