Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip image creation when the calculated height or width of the original image is 0 #9475

Closed
2 tasks done
phamhung77 opened this issue Jun 3, 2024 · 9 comments
Closed
2 tasks done

Comments

@phamhung77
Copy link

phamhung77 commented Jun 3, 2024

Prerequisites

  • I have searched for duplicate or closed issues
  • I can recreate the issue with all plugins disabled

Describe the issue

When user attaches an image to signature, for example, the image is converted to an inline image, inserted directly to the mail. It's applied the same when an incoming email including an image, when user forwards that email, the image is attached as inline image. Then from the user GUI, a red alert comes up: Connection error. Roundcube error log records an PHP error related to imagecreatetruecolor() function.

Tested on both Chrome and Safari latest version. When it's happening, the email is still sent. But sometimes the body content is not included completely.

What browser(s) are you seeing the problem on?

Chrome

What version of PHP are you using?

v8.3

What version of Roundcube are you using?

v1.6.6

JavaScript errors

No response

PHP errors

PHP Fatal error: Uncaught ValueError: imagecreatetruecolor(): Argument #2 ($height) must be greater than 0 in /var/www/html/roundcubemail-1.6.6/program/lib/Roundcube/rcube_image.php:252
Stack trace:
#0 /var/www/html/roundcubemail-1.6.6/program/lib/Roundcube/rcube_image.php(252): imagecreatetruecolor()
#1 /var/www/html/roundcubemail-1.6.6/program/actions/mail/get.php(101): rcube_image->resize()
#2 /var/www/html/roundcubemail-1.6.6/program/include/rcmail.php(282): rcmail_action_mail_get->run()
#3 /var/www/html/roundcubemail-1.6.6/index.php(278): rcmail->action_handler()
#4 {main}
thrown in /var/www/html/roundcubemail-1.6.6/program/lib/Roundcube/rcube_image.php on line 252

@pabzm
Copy link
Member

pabzm commented Jun 4, 2024

Thank you for the report!

Can you please provide us with an example email, so that we can try to reproduce the problem?

@alecpl
Copy link
Member

alecpl commented Jun 5, 2024

Here the image is being resized (probably for a thumbnail). A sample would be nice, full message or the image alone. Looks like we at least need to skip image creation when the calculated height or width of the original image is 0.

ps. GD functions throw such errors since PHP 8.0, it was just a warning before that. So, we might need to handle such errors in other places too.

@phamhung77
Copy link
Author

Thank you for replying. The attached images have height or width > 0. However, after digging into the log, I found a conflict with Comodo WAF mod_security rule #212880. Disabling that rule, no more error.

Can you please check that and see why an inline attached image trigger that rule?
Thanks.

@MrSuddenJoy
Copy link

MrSuddenJoy commented Jun 5, 2024

Generally logic here should follow well-proven model of ansMail: thats is sth along:

if (($img:SizeOf(Height)) && ($img:SizeOf(Width)) ) {
// do something
} else {
// do sth else.....
}

At least this is what PHP PSRs requires.

@pabzm pabzm changed the title Error when email includes inline images Skip image creation when the calculated height or width of the original image is 0 Jun 6, 2024
@pabzm
Copy link
Member

pabzm commented Jun 6, 2024

We should make the code not run into that exception, whatever the reason(s) for it may be.

@MrSuddenJoy
Copy link

@pabzm what exception you talk about? and why not?

@pabzm
Copy link
Member

pabzm commented Jun 6, 2024

what exception you talk about? and why not?

The originally reported exception/error. Because we should have proper error handling, that maybe even tells the user what went wrong.

@MrSuddenJoy
Copy link

@pabzm Oh I dim now.......

Now, there I see 2 things:

  • use super shitty premade classes for this,
  • develop your own in-house solution for this.

@alecpl alecpl added this to the 1.6.8 milestone Jul 21, 2024
alecpl added a commit that referenced this issue Jul 21, 2024
…ttachment (#9475)

GD functions may throw ValueError in some cases since PHP 8.0.
We wrap them in try/catch blocks.
alecpl added a commit that referenced this issue Jul 21, 2024
…ttachment (#9475)

GD functions may throw ValueError in some cases since PHP 8.0.
We wrap them in try/catch blocks.
@alecpl
Copy link
Member

alecpl commented Jul 21, 2024

Fixed.

@alecpl alecpl closed this as completed Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants