From d42cc279f47c6af1a5ed1d0f4824220606a1954b Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 22 Aug 2024 16:40:05 +0800 Subject: [PATCH] Fix the issue that some URL which has percent-encoding with \0 will cause nil cache path Use the more robust way to calculate cache path, still keep the exists behavior --- SDWebImage/Core/SDDiskCache.m | 23 ++++++++++++++++++++++- Tests/Tests/SDImageCacheTests.m | 21 +++++++++++++++++++++ Tests/Tests/SDImageCoderTests.m | 13 +++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/SDWebImage/Core/SDDiskCache.m b/SDWebImage/Core/SDDiskCache.m index 82eb0d6b..88ccb455 100644 --- a/SDWebImage/Core/SDDiskCache.m +++ b/SDWebImage/Core/SDDiskCache.m @@ -335,6 +335,17 @@ static NSString * const SDDiskCacheExtendedAttributeName = @"com.hackemist.SDDis #pragma mark - Hash +static inline NSString *SDSanitizeFileNameString(NSString * _Nullable fileName) { + if ([fileName length] == 0) { + return fileName; + } + // note: `:` is the only invalid char on Apple file system + // but `/` or `\` is valid + // \0 is also special case (which cause Foundation API treat the C string as EOF) + NSCharacterSet* illegalFileNameCharacters = [NSCharacterSet characterSetWithCharactersInString:@"\0:"]; + return [[fileName componentsSeparatedByCharactersInSet:illegalFileNameCharacters] componentsJoinedByString:@""]; +} + #define SD_MAX_FILE_EXTENSION_LENGTH (NAME_MAX - CC_MD5_DIGEST_LENGTH * 2 - 1) #pragma clang diagnostic push @@ -346,8 +357,18 @@ static inline NSString * _Nonnull SDDiskCacheFileNameForKey(NSString * _Nullable } unsigned char r[CC_MD5_DIGEST_LENGTH]; CC_MD5(str, (CC_LONG)strlen(str), r); + NSString *ext; + // 1. Use URL path extname if valid NSURL *keyURL = [NSURL URLWithString:key]; - NSString *ext = keyURL ? keyURL.pathExtension : key.pathExtension; + if (keyURL) { + ext = keyURL.pathExtension; + } + // 2. Use file extname if valid + if (!ext) { + ext = key.pathExtension; + } + // 3. Check if extname valid on file system + ext = SDSanitizeFileNameString(ext); // File system has file name length limit, we need to check if ext is too long, we don't add it to the filename if (ext.length > SD_MAX_FILE_EXTENSION_LENGTH) { ext = nil; diff --git a/Tests/Tests/SDImageCacheTests.m b/Tests/Tests/SDImageCacheTests.m index ce8da568..2c4b62e2 100644 --- a/Tests/Tests/SDImageCacheTests.m +++ b/Tests/Tests/SDImageCacheTests.m @@ -714,6 +714,27 @@ static NSString *kTestImageKeyPNG = @"TestImageKey.png"; } */ +- (void)test49DiskCacheKeyForInvalidURL { + NSURL *url1 = [NSURL URLWithString:@"http://e.hiphotos.baidu.com/image/pic/item/a1ec08fa513d2697e542494057fbb2fb4316d81e.jpg00%00"]; + expect([url1.pathExtension hasSuffix:@"\0"]).beTruthy(); // Test Foundation API behavior here + expect(url1).notTo.beNil(); + NSString *key1 = [SDWebImageManager.sharedManager cacheKeyForURL:url1]; + expect(key1).notTo.beNil(); + NSString *path1 = [SDImageCache.sharedImageCache.diskCache cachePathForKey:key1]; + NSString *ext1 = path1.pathExtension; + expect(ext1).equal(@"jpg00"); + + NSURL *url2 = [NSURL URLWithString:@"https://maps.googleapis.com/maps/api/staticmap?center=48.8566,2.3522&format=png&maptype=roadmap&scale=2&size=375x200&zoom=15"]; + expect(url2.pathExtension.length).equal(0); // Test Foundation API behavior here + expect(url2).notTo.beNil(); + NSString *key2 = [SDWebImageManager.sharedManager cacheKeyForURL:url2]; + expect(key2).notTo.beNil(); + NSString *path2 = [SDImageCache.sharedImageCache.diskCache cachePathForKey:key2]; + expect(path2).notTo.beNil(); + NSString *ext2 = path2.pathExtension; + expect(ext2).equal(@""); +} + #pragma mark - SDImageCache & SDImageCachesManager - (void)test49SDImageCacheQueryOp { XCTestExpectation *expectation = [self expectationWithDescription:@"SDImageCache query op works"]; diff --git a/Tests/Tests/SDImageCoderTests.m b/Tests/Tests/SDImageCoderTests.m index 26ad0f43..6fe24aff 100644 --- a/Tests/Tests/SDImageCoderTests.m +++ b/Tests/Tests/SDImageCoderTests.m @@ -566,6 +566,19 @@ expect(orientation).equal(UIImageOrientationDown); #endif + // Check bitmap color equal, between our usage of ImageIO decoder and Apple system inernal + // So, this means, even Apple has bugs, we have bugs too :) + UIColor *testColor1 = [image sd_colorAtPoint:CGPointMake(1, 1)]; + UIColor *testColor2 = [systemImage sd_colorAtPoint:CGPointMake(1, 1)]; + CGFloat r1, g1, b1, a1; + CGFloat r2, g2, b2, a2; + [testColor1 getRed:&r1 green:&g1 blue:&b1 alpha:&a1]; + [testColor2 getRed:&r2 green:&g2 blue:&b2 alpha:&a2]; + expect(r1).beCloseToWithin(r2, 0.01); + expect(g1).beCloseToWithin(g2, 0.01); + expect(b1).beCloseToWithin(b2, 0.01); + expect(a1).beCloseToWithin(a2, 0.01); + // Manual test again for Apple's API CGImageSourceRef source = CGImageSourceCreateWithData((__bridge CFDataRef)data, nil); NSDictionary *properties = (__bridge_transfer NSDictionary *)CGImageSourceCopyPropertiesAtIndex(source, 0, nil);