Problem/Motivation
This module does not work in Drupal 10.3, due to ImageStyleDownloadController::deliver() now takes a string of the expected scheme for derivatives as a required parameter. This results in the following error:
ArgumentCountError: Too few arguments to function Drupal\image\Controller\ImageStyleDownloadController::deliver(), 3 passed in /path/to/website/web/modules/contrib/drimage/src/Controller/DrImageController.php on line 415 and exactly 4 expected in Drupal\image\Controller\ImageStyleDownloadController->deliver() (line 111 of core/modules/image/src/Controller/ImageStyleDownloadController.php).
Steps to reproduce
Test on Drupal 10.3
Proposed resolution
Refers to similar issue about stage_file_proxy module : https://www.drupal.org/project/stage_file_proxy/issues/3450243
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3456085-13.patch | 794 bytes | qusai taha |
| #6 | drimage--2024-06-21--3456085--mr-14.patch | 861 bytes | rajab natshah |
Issue fork drimage-3456085
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:
Comments
Comment #3
rajab natshahComment #5
rajab natshahComment #6
rajab natshahAttache a static patch file for drimage 2024-06-21 MR 14
To be used with composer patches
Comment #7
rajab natshahThanks, Alexandre, for reporting and hinting.
You gave a quick hit on how to fix this issue.
Sorry, if you had an attend to provide an MR or a Patch.
It is needed to have a quick fix for this issue in our product.
Please, review, still we may need to check if in some cases,
The fix may need to provide a
'private'not to have'public'all the time.Comment #8
alxgl commentedThanks to you Rajab, for this quick patch. I'll write a feedback asap.
Comment #9
finex commentedHi, thank you for the patch. It works correctly. Tested on three websites without any issue.
Comment #10
alxgl commentedI don't have the time right now to give a better proposition, but I think we should be closer than what have been proposed here for stage_file_proxy, in terms of coding standard : https://git.drupalcode.org/project/stage_file_proxy/-/commit/a26b05f77ad...
Comment #11
rajab natshahThanks, Leonardo and Alexandre, for following up.
I had 2nd round of testing and reviewed the code in both modules.
The Stag File Proxy module has extended the ImageStyleDownloadController class, with custom override implementation for the
delivermethod.they had to update that.
The Drimage module has extended the ImageStyleDownloadController class but did not override the
delivermethod.It is only calling it.
My only concerns at this time are:
'private'and when to pass'public'?"public"all the time is the optimal logic?delivermethod to allow for public or private cases?Comment #12
weseze commentedI think the public/private detection is already done somewhere in the image() function in the controller. We should be able to pass it correctly.
Quick question, since I'm still running 10.2 atm, this change would make it break with 10.2?
Will have a look at this at the end of the week and try to get it released.
Comment #13
qusai taha commentedWe should use $scheme variable as in this way will be more dynamic as I have a case the scheme will be s3 so it needs to be dynamic
Comment #14
lorenzs commentedTested the latest patch from #13 and works well.
This is way better than putting it hardcoded to 'public'. Only small drawback I see is that the derivative image will always have the same scheme as the original image (where the scheme is derived from the uri).
So I think only very specific use cases where sites are putting derivatives and originals in different locations (f.e. public & s3 if no general takeover of public) will encounter issues with this but all others can work with this already.
Comment #16
weseze commented