verot / class.upload.php

This PHP class uploads files and manipulates images very easily. It is in fact as much as an image processing class than it is an upload class. Compatible with PHP 4, 5, 7 and 8. Supports processing of local files, uploaded files, files sent through XMLHttpRequest.
http://www.verot.net/php_class_upload.htm
GNU General Public License v2.0
847 stars 358 forks source link

Notices generated in imagecolorat(), line 4579 #101

Closed czachor closed 5 years ago

czachor commented 5 years ago

Sometimes when I'm trying to upload an image, a notice is generated: Notice: imagecolorat(): 821,0 is out of bounds in /www/foobar_site/vendor/verot/class.upload.php/src/class.upload.php on line 4759 (notice is generated for each image pixel). The code is trying to loop through non existing image pixels.

Uploaded image: JPG, saved in photoshop using 'save for web' option, 821*305px. If you need, it's here. When $upload->image_x value is bigger (eg. 822) than uploaded image width (821), the notice is generated. Solution: set $upload->image_x to <= 821.

I suppose it could be connected with image_resize (true) or image_ratio_* settings.

PHP (tested): 7.1, 7.2, GD ver: bundled (2.1.0 compatible). class.upload.php ver: latest - 0.35dev, using composer

My processing code:

        $upload = new upload($_FILES['file2upload']);

        if ($upload->uploaded) {
            $upload->file_new_name_body = 'random_string_here';
            $upload->allowed = 'image/*';

            $upload->image_resize = true;
            $upload->image_ratio_y = true;
            $upload->image_x = 822; // here!! should be 821 or lower

            $upload->process('upload/');

            if (!$upload->processed) {
                echo 'ops';
            } else {
                echo 'uploaded: ' . $upload->file_dst_name;
            }
        }

BTW it's little confusing, line 4759 is responsible for converting image to BMP, but I don't want to do it.

verot commented 5 years ago

Thank you for your precise bug report. And indeed, you are right, there was a bug. image_x was producing PHP notices only for value 822 (not 821, the size of your image, and not 823, or 824, etc...). It is because if you change the width by one pixel, the height may not change at all. And in that case, the resizing code wasn't called, thus producing out of bound errors later in the code. If you were to change the width by more than 1 pixel, it is likely that the height would change also, triggering resizing, and avoiding out of bound errors. So thank you for finding out this edge case.

Fix is on master 6a4bcb538

And regarding line 4759, it is for BMP and JPEG (see line above). In fact for non-transparent images. So it is normal that this code is called for your JPEG image.