Browse Source

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
DreamPiggy 1 year ago
parent
commit
d42cc279f4
3 changed files with 56 additions and 1 deletions
  1. 22 1
      SDWebImage/Core/SDDiskCache.m
  2. 21 0
      Tests/Tests/SDImageCacheTests.m
  3. 13 0
      Tests/Tests/SDImageCoderTests.m

+ 22 - 1
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;

+ 21 - 0
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"];

+ 13 - 0
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);