Problem/Motivation
This a regression from #3292350: Update the file size in file_validate_image_resolution after resizing. This fix was not kept when migrating from file_validate_image_resolution()to FileValidationEvent.
When uploading an image that is larger than constraints, the image is resized, the file size reported by the file object in FileValidationEvent does not match the actual file size, all following FileValidationEvents will get the incorrect file size.
Steps to reproduce
- Set an image field with max_resolution settings (for example user_picture in config/people/accounts/fields)
- Add a xdebug break on core/modules/file/src/Plugin/Validation/Constraint/FileImageDimensionsConstraintValidator.php line 67
- Upload a file bigger than the max resolution
- When the break point is triggered, check that
$file->getSize()(original size) and$image->getFileSize()(resized size) are equal - Step to line 69 (after the save), compare again the values:
$file->getSize()(original size) is bigger than$image->getFileSize()(resized size) - After image is saved, check that the size is correct in the file_managed table
Another test is to install and enable the ClamAV contrib module, the image upload will timeout with an error message from ClamAV (because send filesize and actual file size are different)
Proposed resolution
Port the previous fix to FileImageDimensionsConstraintValidator.
Remaining tasks
Test out the merge request
User interface changes
None
Introduced terminology
None
API changes
None
Issue fork drupal-3522463
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 #2
quietone commentedComment #4
o'briatDon't know if it's revelant, but file_validate_image_resolution was removed without replacement, see https://www.drupal.org/node/3363745 & https://www.drupal.org/project/drupal/issues/3221793
Comment #5
klemendev commentedDon't think it is relavant, as ClamAV only uses/used hook_file_validate where the file size passed is incorrect if the file is resized
Comment #6
o'briatI push the same patch to the new constraint part:
Clamav with a "Maximum image dimensions" resized image works for me.
kerneltest is ok.
Comment #7
o'briatThere's another usage of
$image->save()in "navigation" experimental module core/modules/navigation/src/Form/SettingsForm.php:286.But the resize is done after the validation.
Comment #9
smustgrave commentedopened the MR but putting into NW because the summary is incomplete, please keep the full template and fill in relevant sections
Comment #10
klemendev commentedComment #11
klemendev commentedUpdated issue to use full template and filled out to best of my knowledge
Comment #12
o'briatUpdate description and put back the issue to needs review.
The "PHPUnit Build" pipeline task failed, but it doesn't seem to be related to this issue.
Comment #13
o'briatComment #14
avpadernoComment #15
klemendev commentedI confirm the merge request fixes the issue with ClamAV not working with resized images.
The MR also applied cleanly to 10.5.x
Comment #16
kim.pepperTests are failing and comments in #9 need to be addressed. See https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... for details on what makes an issue RTBC.
Comment #17
kim.pepperRe-ran the tests and they are back to green.
Comment #18
klemendev commentedI do believe as per rules you linked it was RTBC, also as per o'briat judgement of why tests were failing, but I will now leave to someone else to mark this issue/MR as RTBC
Comment #19
o'briat@kim.pepper
To me #9 was fixed with #12.
Since the tests are now OK, I put back the issue as "Needs review".
Comment #20
klemendev commentedAs per my comment #15, RTBC from my side
Comment #21
kim.pepperI think this is ready to go.
Comment #22
alexpottCommitted and pushed 29704ed8d21 to 11.x and 74feffe03e3 to 11.2.x and d60071fb257 to 10.6.x and cbd00a5f38a to 10.5.x. Thanks!