Users are able to upload 0-byte images

Confirm bug and that easy to fix.

This is probably still an issue in D8. It would need to get fixed there and backported.

no, in D8 you can't save node with 0-byte JPG file

The patch here looks very similar in the solution proposed to #2377747: Incorrect node create validation error when an invalid image is attached to a field, which is for D8 and has tests. I suggest to close this and mark the other for backport.

You can upload the image and aren't given a useful error message.

I think it's reasonably to try to fix it in #2377747: Incorrect node create validation error when an invalid image is attached to a field and backport a solution.

#2377747: Incorrect node create validation error when an invalid image is attached to a field was committed. I am reopening this to be a D7 backport of that since it appears that here the starting point is different (in d8 it was not possible to upload 0-byte files)

This looks like a simple and sensible solution to me.

I applied the patch to Drupal 7.73-dev and tested with and text file renamed with a .png extention and with a png file of zero bytes. In both cases this patch worked, I no longer received a PDO exception when saving the image.

PDOException: SQLSTATE[22007]: Invalid datetime format: 1366 Incorrect integer value: '' for column `db`.`field_data_field_image2`.`field_image2_width` at row 1: INSERT INTO {field_data_field_image2} (entity_type, entity_id, revision_id, bundle, delta, language, field_image2_fid, field_image2_alt, field_image2_title, field_image2_width, field_image2_height) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10); Array ( [:db_insert_placeholder_0] => node [:db_insert_placeholder_1] => 2 [:db_insert_placeholder_2] => 2 [:db_insert_placeholder_3] => article [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => und [:db_insert_placeholder_6] => 4 [:db_insert_placeholder_7] => [:db_insert_placeholder_8] => [:db_insert_placeholder_9] => [:db_insert_placeholder_10] => ) in field_sql_storage_field_storage_write() (line 514 of /var/www/html/modules/field/modules/field_sql_storage/field_sql_storage.module).

Updated the patch with a filesize check on image_style_deliver. This should prevent any 500 errors in case any 0-byte files were uploaded pre-patch.

The patch in #1 looks good and it is working, but I think we need to update the error message in file_validate_is_image() (see D8 patch) and also add the test from D8 patch.

For the point raised in #9, I think it should be a follow-up issue.

I have backported the test from D8 patch and also updated the error message in file_validate_is_image() (here I have tried to keep the current string so it does not need to be translated again, so I have chosen this kind of solution). This patch is based on patch #1, only mentioned parts added.

I think this is as close as we can get comparing to the D8 patch.

The patch in #11 should be safe in the aspect of adding file_validate_is_image(), because in the image_field_widget_form() there is already a check for 4 hardcoded image extensions:

$supported_extensions = array('png', 'gif', 'jpg', 'jpeg');

So it should not make the resulting array of extensions (which is being checked) more restrictive. But the question is, if we have to change this error message here, or there: #2539478: [D7] Allow image fields to use any extensions the current image toolkit supports (instead of hard-coding jpg, png and gif only) (possibly it also depends on which issue will be fixed first).

$errors[] = t('Only JPEG, PNG and GIF images are allowed.');

Logically it makes sense to fix the error message here, as in the second issue it will be out of scope. That issue does not care if the image is invalid, it only removes the hardcoded extensions for image widget.

Excellent, thanks for adding the test.

This reminded me that I wanted to have a look at a weird upload error that D7 emits when the browser sends 0 bytes because e.g. something like AppArmor blocked it from reading the file locally - see: https://www.mcdruid.co.uk/article/stopping-apparmor-blocking-firefox-rea...

I don't think I got round to doing that (yet) though; it's probably more of an edge-case than this one, but relatively similar.