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

add check to image create functions #2780

Conversation

Levdbas
Copy link
Member

@Levdbas Levdbas commented Jul 25, 2023

Related:

Issue

The imagecreatefrom functions should return false or an instance of GDimage. In the related issue null is returned, which is strange. But I think it is save to add an additional check to see whether we should postpone the image operation when the result of the function is null or false.

Solution

Check if $input has a value before continuing.

Impact

Better error handling.

Usage Changes

None

Considerations

There is a bit of overlap in these two classes. We might want to take a closer look at them in the future to make them more DRY.

Testing

no.

@coveralls
Copy link

coveralls commented Jul 25, 2023

Coverage Status

coverage: 88.658% (-0.02%) from 88.676% when pulling cb9243f on 2771-bug-towebp-causes-uncaught-typeerror-imageistruecolor-argument-1-$image-must-be-of-type-gdimage-null-given-error into 129b9c6 on 2.x.

@gchtr
Copy link
Member

gchtr commented Jul 25, 2023

Thanks for taking this on!

I’m not sure about the best approach to handle this. Error handling in general is still an open issue for the future: #2210. Until we didn’t define how we should handle errors, I think we should silently return without adding a message to the error log.

@gchtr gchtr added the 2.0 label Jul 27, 2023
…geistruecolor-argument-1-$image-must-be-of-type-gdimage-null-given-error

# Conflicts:
#	src/Image/Operation/ToJpg.php
#	src/Image/Operation/ToWebp.php
Levdbas and others added 2 commits July 28, 2023 14:54
Copy link
Member

@gchtr gchtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you all for thinking through the different solutions to this.

I wouldn’t drop support for PHP 7.4 for the 2.x release to provide an easier upgrade path.

But for the next major version, we can definitely drop it.

@gchtr gchtr merged commit f56f062 into 2.x Jul 31, 2023
51 of 52 checks passed
@gchtr gchtr deleted the 2771-bug-towebp-causes-uncaught-typeerror-imageistruecolor-argument-1-$image-must-be-of-type-gdimage-null-given-error branch July 31, 2023 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants