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

Fix crash for method cacheKeyForURL: when url is nil #1585

Closed

Conversation

NSKevin
Copy link

@NSKevin NSKevin commented Jun 6, 2016

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide
  • I have read the Documentation
  • I have searched for a similar pull request in the project and found none
  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
  • I have added the required tests to prove the fix/feature I am adding
  • I have updated the documentation (if necesarry)
  • I have run the tests and they pass
  • I have run the lint and it passes (pod lib lint)

This merge request fixes / reffers to the following issues: ...

Pull Request Description

In method cacheKeyForURL, when the url was nil, the app crashed. So I added a safe check on it.

@NSKevin NSKevin mentioned this pull request Jun 6, 2016
3 tasks
bpoplauschi added a commit that referenced this pull request Jun 6, 2016
@bpoplauschi
Copy link
Member

@NSKevin thanks for this. I adapted a bit your fix. Since we have places where we use that key as a dictionary key, I preferred to return an empty string rather than nil when the url was nil. 1bf62d4

@bpoplauschi bpoplauschi closed this Jun 6, 2016
@bpoplauschi bpoplauschi added this to the 3.8.0 milestone Jun 6, 2016
bpoplauschi added a commit that referenced this pull request Jun 7, 2016
…ilename (key)" fb0cdb6 1bf62d4 #1584 - fixes #1433 #1553 #1583 #1585

This is a deal breaker for people. The solution for those issues (i.e. very long urls) is to set the SDWebImageManager cacheKeyFilter block and do their own calculations there.
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

2 participants