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

Fixing sd_imageOrientationFromImageData call crashes if image is nil, re... #813

Closed
wants to merge 1 commit into from

Conversation

seviu
Copy link
Contributor

@seviu seviu commented Jul 16, 2014

We have seen that SDWebImage has added support for image orientation that our apps are crashing now on line 63 of UIImage+MultiFormat.m.

I do not know why, but on some circumstances it might be that imageData is nil. If that is the case, imageSource would be imageSource pointing to 0x00000000, and CFRelease'ing it throws an exception that crashes the whole app.

My suggested fix, is to check for nil on imageSource in sd_imageOrientationFromImageData, and return UIImageOrientationUp if there is no image (UIImage+Multiformat.h)

@rromanchuk
Copy link
Contributor

@seviu can confirm, this is a serious problem for production apps. Probably should be completely rolled back, it wasn't tested in any sort of production environment

c76aba6#diff-a2080a524a7f749c0dab0afb2d8b8566

@seviu
Copy link
Contributor Author

seviu commented Jul 17, 2014

I totally agree. I have tested it with bogus NSData and it does not crash. Only if the imageData is null, it fails. The pull request just does a check. Maybe it is safer that the code is rolled back, and this pull request then can be discarded.

@bpoplauschi
Copy link
Member

I have to agree, it's probably safer to just revert. I have asked the commit author to have a look, but we may revert anyway. We should probably release a 3.7.1 patch asap with this fix.

@bpoplauschi bpoplauschi added this to the 3.7.1 milestone Jul 18, 2014
@bpoplauschi
Copy link
Member

Replaced by #819 Thanks @seviu for investigating this, your fix is the same. I will release a 3.7.1 patch asap, hope this fixes the issues.

@bpoplauschi
Copy link
Member

3.7.1 just got out. Could you please check and see if this crash still reproduces or is fixed as it should be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants