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 bottom and top image dimensions check to blend entry points #5

Closed
joao-conde opened this issue Nov 28, 2022 · 5 comments
Closed
Assignees
Labels
bug Something isn't working p-low Low priority issue

Comments

@joao-conde
Copy link
Contributor

@joamag

Description

PConvert works for images of the same dimensions (width and height). However, blending images with different dimensions should return an error and not segfault. This causes issues for clients of the API (whether in Python or in C) where the program just segfaults.

The blend_images function should make a simple verification to the bottom and top images dimensions and check if they are equal. If not, we can return with an exit code. Notice that it already returns an ERROR_T (just an int) but it is always the NORMAL one unless it segfaults.

When images do not have the same size, this pointer arithmetic is guaranteed to SEGFAULT:

Proposal

Add an images dimensions check to all blend entry points to avoid invalid pointer arithmetic.

@joao-conde
Copy link
Contributor Author

For reference, the way PConvert rust deals with this is: https://github.com/ripe-tech/pconvert-rust/blob/6f13b2c0e09762e8a609199b3212d112dabb030c/src/blending/mod.rs#L102

.zip will take both iterators and "fill" the shortest one with default values for the elements. Hence, if a smaller image is provided, this basically merges both by "aligning the shortest one to the top left" and using RGBA(0,0,0,0) for the filling.

@joamag joamag added bug Something isn't working p-low Low priority issue labels Nov 28, 2022
@joamag joamag self-assigned this Nov 28, 2022
joamag added a commit that referenced this issue Nov 28, 2022
@joamag
Copy link
Contributor

joamag commented Nov 28, 2022

@joao-conde Pre-validation has been added, but I'm still pending some incorrect image size tests to close this issue

@joamag
Copy link
Contributor

joamag commented Nov 28, 2022

Yup got it working. Closing this issue as fixed.

Screenshot 2022-11-28 at 23 55 27

@joamag joamag closed this as completed Nov 28, 2022
@joamag
Copy link
Contributor

joamag commented Nov 29, 2022

@joao-conde pconvert-python 0.4.5 already includes this fix

@joao-conde
Copy link
Contributor Author

Nice! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p-low Low priority issue
Projects
None yet
Development

No branches or pull requests

2 participants