From d9a2effff65bd391988da92cad9c2d70d13e2667 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 20 Mar 2018 10:25:37 -0400 Subject: [PATCH 1/4] CR: remove "k" from non constant // FREEBIE --- Signal/src/ViewControllers/DebugUI/DebugUIMessages.m | 3 ++- .../ViewControllers/MediaTileViewController.swift | 12 ++++++------ SignalServiceKit/src/Util/NSDate+OWS.h | 1 + 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index a41816cc2..1bb159efb 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -10,6 +10,7 @@ #import #import #import +#import #import #import #import @@ -1200,7 +1201,7 @@ NS_ASSUME_NONNULL_BEGIN for (NSUInteger i = 0; i < counter; i++) { // Random time within last n years. Helpful for filling out a media gallery over time. - double yearsMillis = 4.0 * 365.0 * 24.0 * 60.0 * 60.0 * 1000.0; + double yearsMillis = 4.0 * kYearsInMillis; uint64_t millisAgo = (uint64_t)(((double)arc4random() / ((double)0xffffffff)) * yearsMillis); uint64_t timestamp = [NSDate ows_millisecondTimeStamp] - millisAgo; diff --git a/Signal/src/ViewControllers/MediaTileViewController.swift b/Signal/src/ViewControllers/MediaTileViewController.swift index 27e75685a..d739cc2c6 100644 --- a/Signal/src/ViewControllers/MediaTileViewController.swift +++ b/Signal/src/ViewControllers/MediaTileViewController.swift @@ -155,7 +155,7 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe return 0 } - if sectionIdx == kLoadNewerSectionIdx { + if sectionIdx == loadNewerSectionIdx { // load more recent return 0 } @@ -187,7 +187,7 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe let title = NSLocalizedString("GALLERY_TILES_LOADING_OLDER_LABEL", comment: "Label indicating loading is in progress") sectionHeader.configure(title: title) return sectionHeader - case kLoadNewerSectionIdx: + case loadNewerSectionIdx: guard let sectionHeader = collectionView.dequeueReusableSupplementaryView(ofKind: kind, withReuseIdentifier: MediaGalleryLoadingHeader.reuseIdentifier, for: indexPath) as? MediaGalleryLoadingHeader else { owsFail("\(logTag) in \(#function) unable to build section header for kLoadOlderSectionIdx") @@ -223,8 +223,8 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe case kLoadOlderSectionIdx: owsFail("\(logTag) in \(#function) unexpected cell for kLoadOlderSectionIdx") return defaultCell - case kLoadNewerSectionIdx: - owsFail("\(logTag) in \(#function) unexpected cell for kLoadNewerSectionIdx") + case loadNewerSectionIdx: + owsFail("\(logTag) in \(#function) unexpected cell for loadNewerSectionIdx") return defaultCell default: guard let sectionDate = self.galleryDates[safe: indexPath.section - 1] else { @@ -269,7 +269,7 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe return CGSize.zero } return mediaGalleryDataSource.hasFetchedOldest ? CGSize.zero : CGSize(width: 0, height: 100) - case kLoadNewerSectionIdx: + case loadNewerSectionIdx: // Show "loading newer..." iff there is still more recent data to be fetched guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") @@ -311,7 +311,7 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe var isFetchingMoreData: Bool = false let kLoadOlderSectionIdx = 0 - var kLoadNewerSectionIdx: Int { + var loadNewerSectionIdx: Int { return galleryDates.count + 1 } diff --git a/SignalServiceKit/src/Util/NSDate+OWS.h b/SignalServiceKit/src/Util/NSDate+OWS.h index d11100340..338e9d205 100755 --- a/SignalServiceKit/src/Util/NSDate+OWS.h +++ b/SignalServiceKit/src/Util/NSDate+OWS.h @@ -17,6 +17,7 @@ extern const NSTimeInterval kMonthInterval; #define kDayInMs (kHourInMs * 24) #define kWeekInMs (kDayInMs * 7) #define kMonthInMs (kDayInMs * 30) +#define kYearsInMs (kDayInMs * 365) @interface NSDate (OWS) From 8e9eb6d2128568851ec2f74773a964a7aad6ff41 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 20 Mar 2018 10:25:48 -0400 Subject: [PATCH 2/4] CR: Use a less-likely-to-collide thumbnail name for legacy attachments If you had a legacy file foo.jpg and a file foo-thumbnail.jpg, we'd use the foo-thumbnail.jpg as the thumbnail for foo. This isn't a problem with modern attachments as each is in it's own directory. // FREEBIE --- SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 15b3a9f50..e963195dd 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -243,7 +243,7 @@ NS_ASSUME_NONNULL_BEGIN NSString *filename = filePath.lastPathComponent.stringByDeletingPathExtension; NSString *containingDir = filePath.stringByDeletingLastPathComponent; - NSString *newFilename = [filename stringByAppendingString:@"-thumbnail"]; + NSString *newFilename = [filename stringByAppendingString:@"-signal-ios-thumbnail"]; return [[containingDir stringByAppendingPathComponent:newFilename] stringByAppendingPathExtension:@"jpg"]; } From 32bf9d52a1a040ebef4ad94905baa620d34050ed Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 20 Mar 2018 10:42:00 -0400 Subject: [PATCH 3/4] CR: Delete thumbnail with directory // FREEBIE --- .../src/Messages/Attachments/TSAttachmentStream.m | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index e963195dd..26203f3f8 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -260,12 +260,22 @@ NS_ASSUME_NONNULL_BEGIN - (void)removeFileWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { + NSError *error; + + NSString *_Nullable thumbnailPath = self.thumbnailPath; + if (thumbnailPath) { + [[NSFileManager defaultManager] removeItemAtPath:thumbnailPath error:&error]; + + if (error) { + DDLogError(@"%@ remove thumbnail errored with: %@", self.logTag, error); + } + } + NSString *_Nullable filePath = self.filePath; if (!filePath) { OWSFail(@"%@ Missing path for attachment.", self.logTag); return; } - NSError *error; [[NSFileManager defaultManager] removeItemAtPath:filePath error:&error]; if (error) { @@ -345,7 +355,8 @@ NS_ASSUME_NONNULL_BEGIN } if (![[NSFileManager defaultManager] fileExistsAtPath:self.mediaURL.path]) { - OWSFail(@"%@ while generating thumbnail, source file doesn't exist: %@", self.logTag, self.mediaURL) return; + OWSFail(@"%@ while generating thumbnail, source file doesn't exist: %@", self.logTag, self.mediaURL); + return; } // TODO proper resolution? From 2465d6df007a360af1d1932e2d4eeec8374c6d1e Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 20 Mar 2018 10:42:17 -0400 Subject: [PATCH 4/4] CR: ensure image is safe before generating thumbnail // FREEBIE --- .../src/Messages/Attachments/TSAttachmentStream.m | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 26203f3f8..e0734b9aa 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -364,6 +364,11 @@ NS_ASSUME_NONNULL_BEGIN UIImage *_Nullable result; if (self.isImage || self.isAnimated) { + if (![NSData ows_isValidImageAtPath:self.filePath]) { + DDLogWarn(@"%@ skipping thumbnail generation for invalid image at path: %@", self.logTag, self.filePath); + return; + } + CGImageSourceRef imageSource = CGImageSourceCreateWithURL((__bridge CFURLRef)self.mediaURL, NULL); OWSAssert(imageSource != NULL) NSDictionary *imageOptions = @{ (NSString const *)kCGImageSourceCreateThumbnailFromImageIfAbsent : (NSNumber const *)kCFBooleanTrue,