From 82752741542629824992d77dc55392f668a19766 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Tue, 2 Apr 2019 12:06:27 +0800 Subject: [PATCH 1/4] Fix the blacking error domain should filter specify SDWebImageErrorDomain codes which is indeed un-recoverable --- SDWebImage/SDWebImageManager.m | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/SDWebImage/SDWebImageManager.m b/SDWebImage/SDWebImageManager.m index 4d72ef0d..409b07ab 100644 --- a/SDWebImage/SDWebImageManager.m +++ b/SDWebImage/SDWebImageManager.m @@ -178,7 +178,7 @@ static id _defaultImageLoader; // Query cache process - (void)callCacheProcessForOperation:(nonnull SDWebImageCombinedOperation *)operation - url:(nullable NSURL *)url + url:(nonnull NSURL *)url options:(SDWebImageOptions)options context:(nullable SDWebImageContext *)context progress:(nullable SDImageLoaderProgressBlock)progressBlock @@ -206,7 +206,7 @@ static id _defaultImageLoader; // Download process - (void)callDownloadProcessForOperation:(nonnull SDWebImageCombinedOperation *)operation - url:(nullable NSURL *)url + url:(nonnull NSURL *)url options:(SDWebImageOptions)options context:(SDWebImageContext *)context cachedImage:(nullable UIImage *)cachedImage @@ -280,7 +280,7 @@ static id _defaultImageLoader; // Store cache process - (void)callStoreCacheProcessForOperation:(nonnull SDWebImageCombinedOperation *)operation - url:(nullable NSURL *)url + url:(nonnull NSURL *)url options:(SDWebImageOptions)options context:(SDWebImageContext *)context downloadedImage:(nullable UIImage *)downloadedImage @@ -375,7 +375,10 @@ static id _defaultImageLoader; shouldBlockFailedURL = [self.delegate imageManager:self shouldBlockFailedURL:url withError:error]; } else { // Filter the error domain and check error codes - if ([error.domain isEqualToString:NSURLErrorDomain]) { + if ([error.domain isEqualToString:SDWebImageErrorDomain]) { + shouldBlockFailedURL = ( error.code == SDWebImageErrorInvalidURL + || error.code == SDWebImageErrorBadImageData); + } else if ([error.domain isEqualToString:NSURLErrorDomain]) { shouldBlockFailedURL = ( error.code != NSURLErrorNotConnectedToInternet && error.code != NSURLErrorCancelled && error.code != NSURLErrorTimedOut From fa426f9b054d1b970d0d4c0bd094ca2c7179896a Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Tue, 2 Apr 2019 14:59:21 +0800 Subject: [PATCH 2/4] Update to use the delegate mode, let the actual image loader to decide whether to mark failed or not. --- SDWebImage/SDImageLoader.h | 14 ++++++++++++++ SDWebImage/SDImageLoadersManager.m | 13 +++++++++++++ SDWebImage/SDWebImageDownloader.m | 21 +++++++++++++++++++++ SDWebImage/SDWebImageManager.m | 15 ++------------- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/SDWebImage/SDImageLoader.h b/SDWebImage/SDImageLoader.h index a69f7c53..f4de58fa 100644 --- a/SDWebImage/SDImageLoader.h +++ b/SDWebImage/SDImageLoader.h @@ -85,4 +85,18 @@ FOUNDATION_EXPORT UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NS progress:(nullable SDImageLoaderProgressBlock)progressBlock completed:(nullable SDImageLoaderCompletedBlock)completedBlock; + +@optional +/** + Whether the error from image loader should be marked indded un-recoverable or not. + If this return YES, failed URL which does not using `SDWebImageRetryFailed` will be blocked into black list. Else not. + If does not implements this method, assume always return NO. + + @param url The URL represent the image. Note this may not be a HTTP URL + @param error The URL's loading error, from previous `requestImageWithURL:options:context:progress:completed:` completedBlock's error. + @return Whether to block this url or not. Return YES to mark this URL as failed. + */ +- (BOOL)shouldBlockFailedURLWithURL:(nonnull NSURL *)url + error:(nonnull NSError *)error; + @end diff --git a/SDWebImage/SDImageLoadersManager.m b/SDWebImage/SDImageLoadersManager.m index 91e0d941..dd99c31c 100644 --- a/SDWebImage/SDImageLoadersManager.m +++ b/SDWebImage/SDImageLoadersManager.m @@ -100,4 +100,17 @@ return nil; } +- (BOOL)shouldBlockFailedURLWithURL:(NSURL *)url error:(NSError *)error { + NSArray> *loaders = self.loaders; + for (id loader in loaders.reverseObjectEnumerator) { + if (![loader respondsToSelector:@selector(shouldBlockFailedURLWithURL:error:)]) { + break; + } + if ([loader canRequestImageForURL:url]) { + return [loader shouldBlockFailedURLWithURL:url error:error]; + } + } + return NO; +} + @end diff --git a/SDWebImage/SDWebImageDownloader.m b/SDWebImage/SDWebImageDownloader.m index 6ae608f9..da244659 100644 --- a/SDWebImage/SDWebImageDownloader.m +++ b/SDWebImage/SDWebImageDownloader.m @@ -546,4 +546,25 @@ didReceiveResponse:(NSURLResponse *)response return [self downloadImageWithURL:url options:downloaderOptions context:context progress:progressBlock completed:completedBlock]; } +- (BOOL)shouldBlockFailedURLWithURL:(NSURL *)url error:(NSError *)error { + BOOL shouldBlockFailedURL; + // Filter the error domain and check error codes + if ([error.domain isEqualToString:SDWebImageErrorDomain]) { + shouldBlockFailedURL = ( error.code == SDWebImageErrorInvalidURL + || error.code == SDWebImageErrorBadImageData); + } else if ([error.domain isEqualToString:NSURLErrorDomain]) { + shouldBlockFailedURL = ( error.code != NSURLErrorNotConnectedToInternet + && error.code != NSURLErrorCancelled + && error.code != NSURLErrorTimedOut + && error.code != NSURLErrorInternationalRoamingOff + && error.code != NSURLErrorDataNotAllowed + && error.code != NSURLErrorCannotFindHost + && error.code != NSURLErrorCannotConnectToHost + && error.code != NSURLErrorNetworkConnectionLost); + } else { + shouldBlockFailedURL = NO; + } + return shouldBlockFailedURL; +} + @end diff --git a/SDWebImage/SDWebImageManager.m b/SDWebImage/SDWebImageManager.m index 409b07ab..2ffe6c61 100644 --- a/SDWebImage/SDWebImageManager.m +++ b/SDWebImage/SDWebImageManager.m @@ -374,19 +374,8 @@ static id _defaultImageLoader; if ([self.delegate respondsToSelector:@selector(imageManager:shouldBlockFailedURL:withError:)]) { shouldBlockFailedURL = [self.delegate imageManager:self shouldBlockFailedURL:url withError:error]; } else { - // Filter the error domain and check error codes - if ([error.domain isEqualToString:SDWebImageErrorDomain]) { - shouldBlockFailedURL = ( error.code == SDWebImageErrorInvalidURL - || error.code == SDWebImageErrorBadImageData); - } else if ([error.domain isEqualToString:NSURLErrorDomain]) { - shouldBlockFailedURL = ( error.code != NSURLErrorNotConnectedToInternet - && error.code != NSURLErrorCancelled - && error.code != NSURLErrorTimedOut - && error.code != NSURLErrorInternationalRoamingOff - && error.code != NSURLErrorDataNotAllowed - && error.code != NSURLErrorCannotFindHost - && error.code != NSURLErrorCannotConnectToHost - && error.code != NSURLErrorNetworkConnectionLost); + if ([self.imageLoader respondsToSelector:@selector(shouldBlockFailedURLWithURL:error:)]) { + shouldBlockFailedURL = [self.imageLoader shouldBlockFailedURLWithURL:url error:error]; } else { shouldBlockFailedURL = NO; } From 691873117a21143da877d7ad48d350b2362571a8 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Tue, 2 Apr 2019 16:32:15 +0800 Subject: [PATCH 3/4] Fix the logic to check loaders. Find the correct loader first and then check if optional protocol method is implemented --- SDWebImage/SDImageLoadersManager.m | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/SDWebImage/SDImageLoadersManager.m b/SDWebImage/SDImageLoadersManager.m index dd99c31c..680b9a7a 100644 --- a/SDWebImage/SDImageLoadersManager.m +++ b/SDWebImage/SDImageLoadersManager.m @@ -103,11 +103,12 @@ - (BOOL)shouldBlockFailedURLWithURL:(NSURL *)url error:(NSError *)error { NSArray> *loaders = self.loaders; for (id loader in loaders.reverseObjectEnumerator) { - if (![loader respondsToSelector:@selector(shouldBlockFailedURLWithURL:error:)]) { - break; - } if ([loader canRequestImageForURL:url]) { - return [loader shouldBlockFailedURLWithURL:url error:error]; + if ([loader respondsToSelector:@selector(shouldBlockFailedURLWithURL:error:)]) { + return [loader shouldBlockFailedURLWithURL:url error:error]; + } else { + return NO; + } } } return NO; From 2640301e82e4a0ee03d4f5d0846a3d0b02979697 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Tue, 2 Apr 2019 17:56:12 +0800 Subject: [PATCH 4/4] Change the optional method into required, force the custom loader author to provide the error check --- SDWebImage/SDImageLoader.h | 2 -- SDWebImage/SDImageLoadersManager.m | 6 +----- SDWebImage/SDWebImageManager.m | 6 +----- Tests/Tests/SDWebImageTestLoader.m | 4 ++++ 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/SDWebImage/SDImageLoader.h b/SDWebImage/SDImageLoader.h index f4de58fa..136ac9e4 100644 --- a/SDWebImage/SDImageLoader.h +++ b/SDWebImage/SDImageLoader.h @@ -86,11 +86,9 @@ FOUNDATION_EXPORT UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NS completed:(nullable SDImageLoaderCompletedBlock)completedBlock; -@optional /** Whether the error from image loader should be marked indded un-recoverable or not. If this return YES, failed URL which does not using `SDWebImageRetryFailed` will be blocked into black list. Else not. - If does not implements this method, assume always return NO. @param url The URL represent the image. Note this may not be a HTTP URL @param error The URL's loading error, from previous `requestImageWithURL:options:context:progress:completed:` completedBlock's error. diff --git a/SDWebImage/SDImageLoadersManager.m b/SDWebImage/SDImageLoadersManager.m index 680b9a7a..002ef95a 100644 --- a/SDWebImage/SDImageLoadersManager.m +++ b/SDWebImage/SDImageLoadersManager.m @@ -104,11 +104,7 @@ NSArray> *loaders = self.loaders; for (id loader in loaders.reverseObjectEnumerator) { if ([loader canRequestImageForURL:url]) { - if ([loader respondsToSelector:@selector(shouldBlockFailedURLWithURL:error:)]) { - return [loader shouldBlockFailedURLWithURL:url error:error]; - } else { - return NO; - } + return [loader shouldBlockFailedURLWithURL:url error:error]; } } return NO; diff --git a/SDWebImage/SDWebImageManager.m b/SDWebImage/SDWebImageManager.m index 2ffe6c61..666148dd 100644 --- a/SDWebImage/SDWebImageManager.m +++ b/SDWebImage/SDWebImageManager.m @@ -374,11 +374,7 @@ static id _defaultImageLoader; if ([self.delegate respondsToSelector:@selector(imageManager:shouldBlockFailedURL:withError:)]) { shouldBlockFailedURL = [self.delegate imageManager:self shouldBlockFailedURL:url withError:error]; } else { - if ([self.imageLoader respondsToSelector:@selector(shouldBlockFailedURLWithURL:error:)]) { - shouldBlockFailedURL = [self.imageLoader shouldBlockFailedURLWithURL:url error:error]; - } else { - shouldBlockFailedURL = NO; - } + shouldBlockFailedURL = [self.imageLoader shouldBlockFailedURLWithURL:url error:error]; } return shouldBlockFailedURL; diff --git a/Tests/Tests/SDWebImageTestLoader.m b/Tests/Tests/SDWebImageTestLoader.m index 32635b2d..22978edb 100644 --- a/Tests/Tests/SDWebImageTestLoader.m +++ b/Tests/Tests/SDWebImageTestLoader.m @@ -50,4 +50,8 @@ return task; } +- (BOOL)shouldBlockFailedURLWithURL:(NSURL *)url error:(NSError *)error { + return NO; +} + @end