-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: reject loadImage when src is null or invalid #2518
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
fix: reject loadImage when src is null or invalid #2518
Conversation
|
I'm also encountering this issue. I hope it can be merged. |
cf6b80f to
ecd20bd
Compare
index.js
Outdated
| function loadImage (src) { | ||
| return new Promise((resolve, reject) => { | ||
| if (!src) return reject(new Error("Invalid image source")); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be placed in a final else branch on line 66 of lib/image.js to fix the whole class of bugs. It isn't just null/undefined that will cause onload to not be called. Then you could mark this as fixes #2525 as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @chearon ,
Thanks for the review.
Would you prefer having the check in both index.js and image.js or only in the image.js ?
ecd20bd to
6186db2
Compare
|
Hi @chearon , Ian |
6186db2 to
3f92b48
Compare
chearon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks. I just added the issue # to the changelog.
Description:
- To handle non-string/non-Buffer sources while fetching image.
- Add test cases for '', null, and undefined inputs to loadImage()
Test Result:
```
npm run test
...
✔ rejects when loadImage is called with null
✔ rejects when loadImage is called with undefined
✔ rejects when loadImage is called with empty string
291 passing (302ms)
6 pending
```
3f92b48 to
3fe2c56
Compare
|
...and fixed conflicts |
fix: reject loadImage when src is null or invalid
Description:
Test Result:
Thanks for contributing!
Fix #2304