From df7d56373d305bb9417825b4295d7a64608a7c15 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Fri, 7 May 2021 16:01:31 +0800 Subject: [PATCH 1/3] Added the feature to config the status code and content type checking logic, do not hard-coded 200-400 --- SDWebImage/Core/SDWebImageDownloader.m | 8 +++++ SDWebImage/Core/SDWebImageDownloaderConfig.h | 15 +++++++++ SDWebImage/Core/SDWebImageDownloaderConfig.m | 3 ++ .../Core/SDWebImageDownloaderOperation.h | 19 ++++++++++++ .../Core/SDWebImageDownloaderOperation.m | 31 +++++++++++++++---- SDWebImage/Core/SDWebImageError.h | 3 ++ SDWebImage/Core/SDWebImageError.m | 1 + 7 files changed, 74 insertions(+), 6 deletions(-) diff --git a/SDWebImage/Core/SDWebImageDownloader.m b/SDWebImage/Core/SDWebImageDownloader.m index 2eeb05e4..cd6b7966 100644 --- a/SDWebImage/Core/SDWebImageDownloader.m +++ b/SDWebImage/Core/SDWebImageDownloader.m @@ -349,6 +349,14 @@ static void * SDWebImageDownloaderContext = &SDWebImageDownloaderContext; operation.minimumProgressInterval = MIN(MAX(self.config.minimumProgressInterval, 0), 1); } + if ([operation respondsToSelector:@selector(setAcceptableStatusCodes:)]) { + operation.acceptableStatusCodes = self.config.acceptableStatusCodes; + } + + if ([operation respondsToSelector:@selector(setAcceptableContentTypes:)]) { + operation.acceptableContentTypes = self.config.acceptableContentTypes; + } + if (options & SDWebImageDownloaderHighPriority) { operation.queuePriority = NSOperationQueuePriorityHigh; } else if (options & SDWebImageDownloaderLowPriority) { diff --git a/SDWebImage/Core/SDWebImageDownloaderConfig.h b/SDWebImage/Core/SDWebImageDownloaderConfig.h index 5a8cf583..9d5e67bf 100644 --- a/SDWebImage/Core/SDWebImageDownloaderConfig.h +++ b/SDWebImage/Core/SDWebImageDownloaderConfig.h @@ -95,4 +95,19 @@ typedef NS_ENUM(NSInteger, SDWebImageDownloaderExecutionOrder) { */ @property (nonatomic, copy, nullable) NSString *password; +/** + * Set the acceptable HTTP Response status code. The status code which beyond the range will mark the download operation failed. + * For example, if we config [200, 400) but server response is 503, the download will fail with error code `SDWebImageErrorInvalidDownloadStatusCode`. + * Defaults to [200,400). Nil means no validation at all. + */ +@property (nonatomic, copy, nullable) NSIndexSet *acceptableStatusCodes; + +/** + * Set the acceptable HTTP Response content type. The content type beyond the set will mark the download operation failed. + * For example, if we config ["image/png"] but server response is "application/json", the download will fail with error code `SDWebImageErrorInvalidDownloadContentType`. + * Normally you don't need this for image format detection because we use image's data file signature magic bytes: https://en.wikipedia.org/wiki/List_of_file_signatures + * Defaults to nil. Nil means no validation at all. + */ +@property (nonatomic, copy, nullable) NSSet *acceptableContentTypes; + @end diff --git a/SDWebImage/Core/SDWebImageDownloaderConfig.m b/SDWebImage/Core/SDWebImageDownloaderConfig.m index 1fc93308..6120bd8a 100644 --- a/SDWebImage/Core/SDWebImageDownloaderConfig.m +++ b/SDWebImage/Core/SDWebImageDownloaderConfig.m @@ -26,6 +26,7 @@ static SDWebImageDownloaderConfig * _defaultDownloaderConfig; _maxConcurrentDownloads = 6; _downloadTimeout = 15.0; _executionOrder = SDWebImageDownloaderFIFOExecutionOrder; + _acceptableStatusCodes = [NSIndexSet indexSetWithIndexesInRange:NSMakeRange(200, 100)]; } return self; } @@ -41,6 +42,8 @@ static SDWebImageDownloaderConfig * _defaultDownloaderConfig; config.urlCredential = self.urlCredential; config.username = self.username; config.password = self.password; + config.acceptableStatusCodes = self.acceptableStatusCodes; + config.acceptableContentTypes = self.acceptableContentTypes; return config; } diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.h b/SDWebImage/Core/SDWebImageDownloaderOperation.h index 5fc19289..1cd2a50d 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.h +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.h @@ -37,8 +37,12 @@ @optional @property (strong, nonatomic, readonly, nullable) NSURLSessionTask *dataTask; @property (strong, nonatomic, readonly, nullable) NSURLSessionTaskMetrics *metrics API_AVAILABLE(macosx(10.12), ios(10.0), watchos(3.0), tvos(10.0)); + +// These operation-level config was inherited from downloader. See `SDWebImageDownloaderConfig` for documentation. @property (strong, nonatomic, nullable) NSURLCredential *credential; @property (assign, nonatomic) double minimumProgressInterval; +@property (copy, nonatomic, nullable) NSIndexSet *acceptableStatusCodes; +@property (copy, nonatomic, nullable) NSSet *acceptableContentTypes; @end @@ -85,6 +89,21 @@ */ @property (assign, nonatomic) double minimumProgressInterval; +/** + * Set the acceptable HTTP Response status code. The status code which beyond the range will mark the download operation failed. + * For example, if we config [200, 400) but server response is 503, the download will fail with error code `SDWebImageErrorInvalidDownloadStatusCode`. + * Defaults to [200,400). Nil means no validation at all. + */ +@property (copy, nonatomic, nullable) NSIndexSet *acceptableStatusCodes; + +/** + * Set the acceptable HTTP Response content type. The content type beyond the set will mark the download operation failed. + * For example, if we config ["image/png"] but server response is "application/json", the download will fail with error code `SDWebImageErrorInvalidDownloadContentType`. + * Normally you don't need this for image format detection because we use image's data file signature magic bytes: https://en.wikipedia.org/wiki/List_of_file_signatures + * Defaults to nil. Nil means no validation at all. + */ +@property (copy, nonatomic, nullable) NSSet *acceptableContentTypes; + /** * The options for the receiver. */ diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index 1de1e7e9..34a98adc 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -332,18 +332,37 @@ didReceiveResponse:(NSURLResponse *)response self.expectedSize = expected; self.response = response; - NSInteger statusCode = [response respondsToSelector:@selector(statusCode)] ? ((NSHTTPURLResponse *)response).statusCode : 200; - // Status code should between [200,400) - BOOL statusCodeValid = statusCode >= 200 && statusCode < 400; + // Check status code valid (defaults [200,400)) + NSInteger statusCode = [response isKindOfClass:NSHTTPURLResponse.class] ? ((NSHTTPURLResponse *)response).statusCode : 0; + BOOL statusCodeValid = YES; + if (valid && statusCode > 0 && self.acceptableStatusCodes) { + statusCodeValid = [self.acceptableStatusCodes containsIndex:statusCode]; + } if (!statusCodeValid) { valid = NO; - self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorInvalidDownloadStatusCode userInfo:@{NSLocalizedDescriptionKey : @"Download marked as failed because response status code is not in 200-400", SDWebImageErrorDownloadStatusCodeKey : @(statusCode)}]; + self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain + code:SDWebImageErrorInvalidDownloadStatusCode + userInfo:@{NSLocalizedDescriptionKey: [NSString stringWithFormat:@"Download marked as failed because of invalid response status code %ld", (long)statusCode], SDWebImageErrorDownloadStatusCodeKey: @(statusCode)}]; + } + // Check content type valid (defaults nil) + NSString *contentType = [response isKindOfClass:NSHTTPURLResponse.class] ? ((NSHTTPURLResponse *)response).MIMEType : nil; + BOOL contentTypeValid = YES; + if (valid && contentType.length > 0 && self.acceptableContentTypes) { + contentTypeValid = [self.acceptableContentTypes containsObject:contentType]; + } + if (!contentTypeValid) { + valid = NO; + self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain + code:SDWebImageErrorInvalidDownloadContentType + userInfo:@{NSLocalizedDescriptionKey: [NSString stringWithFormat:@"Download marked as failed because of invalid response content type %@", contentType], SDWebImageErrorDownloadContentTypeKey: contentType}]; } //'304 Not Modified' is an exceptional one //URLSession current behavior will return 200 status code when the server respond 304 and URLCache hit. But this is not a standard behavior and we just add a check - if (statusCode == 304 && !self.cachedData) { + if (valid && statusCode == 304 && !self.cachedData) { valid = NO; - self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorCacheNotModified userInfo:@{NSLocalizedDescriptionKey : @"Download response status code is 304 not modified and ignored"}]; + self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain + code:SDWebImageErrorCacheNotModified + userInfo:@{NSLocalizedDescriptionKey: @"Download response status code is 304 not modified and ignored"}]; } if (valid) { diff --git a/SDWebImage/Core/SDWebImageError.h b/SDWebImage/Core/SDWebImageError.h index 2b412010..acae0801 100644 --- a/SDWebImage/Core/SDWebImageError.h +++ b/SDWebImage/Core/SDWebImageError.h @@ -13,6 +13,8 @@ FOUNDATION_EXPORT NSErrorDomain const _Nonnull SDWebImageErrorDomain; /// The HTTP status code for invalid download response (NSNumber *) FOUNDATION_EXPORT NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadStatusCodeKey; +/// The HTTP MIME content type for invalid download response (NSString *) +FOUNDATION_EXPORT NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadContentTypeKey; /// SDWebImage error domain and codes typedef NS_ERROR_ENUM(SDWebImageErrorDomain, SDWebImageError) { @@ -24,4 +26,5 @@ typedef NS_ERROR_ENUM(SDWebImageErrorDomain, SDWebImageError) { SDWebImageErrorInvalidDownloadStatusCode = 2001, // The image download response a invalid status code. You can check the status code in error's userInfo under `SDWebImageErrorDownloadStatusCodeKey` SDWebImageErrorCancelled = 2002, // The image loading operation is cancelled before finished, during either async disk cache query, or waiting before actual network request. For actual network request error, check `NSURLErrorDomain` error domain and code. SDWebImageErrorInvalidDownloadResponse = 2003, // When using response modifier, the modified download response is nil and marked as failed. + SDWebImageErrorInvalidDownloadContentType = 2004, // The image download response a invalid content type. You can check the MIME content type in error's userInfo under `SDWebImageErrorDownloadContentTypeKey` }; diff --git a/SDWebImage/Core/SDWebImageError.m b/SDWebImage/Core/SDWebImageError.m index 6d174769..29a7039b 100644 --- a/SDWebImage/Core/SDWebImageError.m +++ b/SDWebImage/Core/SDWebImageError.m @@ -11,3 +11,4 @@ NSErrorDomain const _Nonnull SDWebImageErrorDomain = @"SDWebImageErrorDomain"; NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadStatusCodeKey = @"SDWebImageErrorDownloadStatusCodeKey"; +NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadContentTypeKey = @"SDWebImageErrorDownloadContentTypeKey"; From 43219b0739048cc7124cd54e6a501b930f02b91e Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Fri, 7 May 2021 16:26:52 +0800 Subject: [PATCH 2/3] Added test case `test29AcceptableStatusCodeAndContentType` --- Tests/Tests/SDImageCacheTests.m | 4 +-- Tests/Tests/SDWebImageDownloaderTests.m | 35 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/Tests/Tests/SDImageCacheTests.m b/Tests/Tests/SDImageCacheTests.m index 91f0d572..b0a8835d 100644 --- a/Tests/Tests/SDImageCacheTests.m +++ b/Tests/Tests/SDImageCacheTests.m @@ -628,8 +628,8 @@ static NSString *kTestImageKeyPNG = @"TestImageKey.png"; #pragma mark - SDImageCache & SDImageCachesManager - (void)test49SDImageCacheQueryOp { XCTestExpectation *expectation = [self expectationWithDescription:@"SDImageCache query op works"]; - NSData *data = [[SDImageCodersManager sharedManager] encodedDataWithImage:[self testJPEGImage] format:SDImageFormatJPEG options:nil]; - [[SDImageCache sharedImageCache] storeImageDataToDisk:data forKey:kTestImageKeyJPEG]; + NSData *imageData = [[SDImageCodersManager sharedManager] encodedDataWithImage:[self testJPEGImage] format:SDImageFormatJPEG options:nil]; + [[SDImageCache sharedImageCache] storeImageDataToDisk:imageData forKey:kTestImageKeyJPEG]; [[SDImageCachesManager sharedManager] queryImageForKey:kTestImageKeyJPEG options:0 context:@{SDWebImageContextStoreCacheType : @(SDImageCacheTypeDisk)} cacheType:SDImageCacheTypeAll completion:^(UIImage * _Nullable image, NSData * _Nullable data, SDImageCacheType cacheType) { expect(image).notTo.beNil(); diff --git a/Tests/Tests/SDWebImageDownloaderTests.m b/Tests/Tests/SDWebImageDownloaderTests.m index ba4ec5b2..b58a458f 100644 --- a/Tests/Tests/SDWebImageDownloaderTests.m +++ b/Tests/Tests/SDWebImageDownloaderTests.m @@ -733,6 +733,41 @@ }]; } +- (void)test29AcceptableStatusCodeAndContentType { + SDWebImageDownloaderConfig *config1 = [[SDWebImageDownloaderConfig alloc] init]; + config1.acceptableStatusCodes = [NSIndexSet indexSetWithIndex:1]; + SDWebImageDownloader *downloader1 = [[SDWebImageDownloader alloc] initWithConfig:config1]; + XCTestExpectation *expectation1 = [self expectationWithDescription:@"Acceptable status code should work"]; + + SDWebImageDownloaderConfig *config2 = [[SDWebImageDownloaderConfig alloc] init]; + config2.acceptableContentTypes = [NSSet setWithArray:@[@"application/json"]]; + SDWebImageDownloader *downloader2 = [[SDWebImageDownloader alloc] initWithConfig:config2]; + XCTestExpectation *expectation2 = [self expectationWithDescription:@"Acceptable content type should work"]; + + __block SDWebImageDownloadToken *token1; + token1 = [downloader1 downloadImageWithURL:[NSURL URLWithString:kTestJPEGURL] completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, BOOL finished) { + expect(error).notTo.beNil(); + expect(error.code).equal(SDWebImageErrorInvalidDownloadStatusCode); + NSInteger statusCode = ((NSHTTPURLResponse *)token1.response).statusCode; + expect(statusCode).equal(200); + [expectation1 fulfill]; + }]; + + __block SDWebImageDownloadToken *token2; + token2 = [downloader2 downloadImageWithURL:[NSURL URLWithString:kTestJPEGURL] completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, BOOL finished) { + expect(error).notTo.beNil(); + expect(error.code).equal(SDWebImageErrorInvalidDownloadContentType); + NSString *contentType = ((NSHTTPURLResponse *)token1.response).MIMEType; + expect(contentType).equal(@"image/jpeg"); + [expectation2 fulfill]; + }]; + + [self waitForExpectationsWithCommonTimeoutUsingHandler:^(NSError * _Nullable error) { + [downloader1 invalidateSessionAndCancel:YES]; + [downloader2 invalidateSessionAndCancel:YES]; + }]; +} + #pragma mark - SDWebImageLoader - (void)test30CustomImageLoaderWorks { XCTestExpectation *expectation = [self expectationWithDescription:@"Custom image not works"]; From f3d68c9cc26c8016a8b528a4f0e3c91f362c9c7d Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Fri, 7 May 2021 16:40:10 +0800 Subject: [PATCH 3/3] Added `SDWebImageErrorDownloadResponseKey` userInfo for better error report --- .../Core/SDWebImageDownloaderOperation.m | 21 ++++++++++++++----- SDWebImage/Core/SDWebImageError.h | 2 ++ SDWebImage/Core/SDWebImageError.m | 2 ++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index 34a98adc..a2395c5c 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -191,6 +191,7 @@ typedef NSMutableDictionary SDCallbacksDictionary; } if (cachedResponse) { self.cachedData = cachedResponse.data; + self.response = cachedResponse.response; } } @@ -323,7 +324,9 @@ didReceiveResponse:(NSURLResponse *)response response = [self.responseModifier modifiedResponseWithResponse:response]; if (!response) { valid = NO; - self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorInvalidDownloadResponse userInfo:@{NSLocalizedDescriptionKey : @"Download marked as failed because response is nil"}]; + self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain + code:SDWebImageErrorInvalidDownloadResponse + userInfo:@{NSLocalizedDescriptionKey : @"Download marked as failed because response is nil"}]; } } @@ -342,7 +345,9 @@ didReceiveResponse:(NSURLResponse *)response valid = NO; self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorInvalidDownloadStatusCode - userInfo:@{NSLocalizedDescriptionKey: [NSString stringWithFormat:@"Download marked as failed because of invalid response status code %ld", (long)statusCode], SDWebImageErrorDownloadStatusCodeKey: @(statusCode)}]; + userInfo:@{NSLocalizedDescriptionKey : [NSString stringWithFormat:@"Download marked as failed because of invalid response status code %ld", (long)statusCode], + SDWebImageErrorDownloadStatusCodeKey : @(statusCode), + SDWebImageErrorDownloadResponseKey : response}]; } // Check content type valid (defaults nil) NSString *contentType = [response isKindOfClass:NSHTTPURLResponse.class] ? ((NSHTTPURLResponse *)response).MIMEType : nil; @@ -354,7 +359,9 @@ didReceiveResponse:(NSURLResponse *)response valid = NO; self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorInvalidDownloadContentType - userInfo:@{NSLocalizedDescriptionKey: [NSString stringWithFormat:@"Download marked as failed because of invalid response content type %@", contentType], SDWebImageErrorDownloadContentTypeKey: contentType}]; + userInfo:@{NSLocalizedDescriptionKey : [NSString stringWithFormat:@"Download marked as failed because of invalid response content type %@", contentType], + SDWebImageErrorDownloadContentTypeKey : contentType, + SDWebImageErrorDownloadResponseKey : response}]; } //'304 Not Modified' is an exceptional one //URLSession current behavior will return 200 status code when the server respond 304 and URLCache hit. But this is not a standard behavior and we just add a check @@ -362,7 +369,8 @@ didReceiveResponse:(NSURLResponse *)response valid = NO; self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorCacheNotModified - userInfo:@{NSLocalizedDescriptionKey: @"Download response status code is 304 not modified and ignored"}]; + userInfo:@{NSLocalizedDescriptionKey: @"Download response status code is 304 not modified and ignored", + SDWebImageErrorDownloadResponseKey : response}]; } if (valid) { @@ -495,7 +503,10 @@ didReceiveResponse:(NSURLResponse *)response * then we should check if the cached data is equal to image data */ if (self.options & SDWebImageDownloaderIgnoreCachedResponse && [self.cachedData isEqualToData:imageData]) { - self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorCacheNotModified userInfo:@{NSLocalizedDescriptionKey : @"Downloaded image is not modified and ignored"}]; + self.responseError = [NSError errorWithDomain:SDWebImageErrorDomain + code:SDWebImageErrorCacheNotModified + userInfo:@{NSLocalizedDescriptionKey : @"Downloaded image is not modified and ignored", + SDWebImageErrorDownloadResponseKey : self.response}]; // call completion block with not modified error [self callCompletionBlocksWithError:self.responseError]; [self done]; diff --git a/SDWebImage/Core/SDWebImageError.h b/SDWebImage/Core/SDWebImageError.h index acae0801..bb91d0bd 100644 --- a/SDWebImage/Core/SDWebImageError.h +++ b/SDWebImage/Core/SDWebImageError.h @@ -11,6 +11,8 @@ FOUNDATION_EXPORT NSErrorDomain const _Nonnull SDWebImageErrorDomain; +/// The response instance for invalid download response (NSURLResponse *) +FOUNDATION_EXPORT NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadResponseKey; /// The HTTP status code for invalid download response (NSNumber *) FOUNDATION_EXPORT NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadStatusCodeKey; /// The HTTP MIME content type for invalid download response (NSString *) diff --git a/SDWebImage/Core/SDWebImageError.m b/SDWebImage/Core/SDWebImageError.m index 29a7039b..bd0d17ad 100644 --- a/SDWebImage/Core/SDWebImageError.m +++ b/SDWebImage/Core/SDWebImageError.m @@ -10,5 +10,7 @@ #import "SDWebImageError.h" NSErrorDomain const _Nonnull SDWebImageErrorDomain = @"SDWebImageErrorDomain"; + +NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadResponseKey = @"SDWebImageErrorDownloadResponseKey"; NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadStatusCodeKey = @"SDWebImageErrorDownloadStatusCodeKey"; NSErrorUserInfoKey const _Nonnull SDWebImageErrorDownloadContentTypeKey = @"SDWebImageErrorDownloadContentTypeKey";