Fix #20913: Check image resource before attempting to preserve alpha #28485
Conversation
Signed-off-by: Simon Spannagel <[email protected]>
| imagealphablending($this->resource, true); | ||
| imagesavealpha($this->resource, true); | ||
| } else { | ||
| $this->logger->debug('OC_Image->loadFromFile, GIF image not valid: ' . $imagePath, ['app' => 'core']); |
There was a problem hiding this comment.
Maybe an info log makes more sense here and below?
There was a problem hiding this comment.
I would say we don't log anything ;) What can an administrator (the one able to view the log) do when a user uploads a broken gif?
There was a problem hiding this comment.
The admin could notify the affected user that a specific gif/png is broken and which one it is...
(E.g. might also happen due to file corruption)
There was a problem hiding this comment.
So I gather a debug log is exactly the middle between "nothing" and info? :)
I guess having a debug message makes debugging things easier, e.g. when a user asks why no previews are available for some of his images, but it's not an info message because, as @kesselb says, it's not something that calls for action.
There was a problem hiding this comment.
I agree with both; these log are useless (and sometimes less-techy admins think it's a "bad" error...), and a debug can be a compromise.
|
Thanks for merging @juliushaertl ! This could probably be backported also to 21 and 22? |
|
Yes, backport makes sense 👍 |
|
/backport to stable22 |
|
/backport to stable21 |
This fixes #20913 by first checking if an image resource has been properly created before attempting to set alpha/transparency. This is implemented for both GIF and PNG images.