From b62d6900daff76cd46d88ae0123efc17870485de Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 11 May 2018 11:26:04 -0400 Subject: [PATCH 1/3] Fix crash converting images in SAE. --- .../attachments/SignalAttachment.swift | 72 +++++++++++++++---- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index 32da0bbb9..060143dca 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -17,6 +17,7 @@ enum SignalAttachmentError: Error { case couldNotConvertToMpeg4 case couldNotRemoveMetadata case invalidFileFormat + case couldNotResizeImage } extension String { @@ -56,6 +57,8 @@ extension SignalAttachmentError: LocalizedError { return NSLocalizedString("ATTACHMENT_ERROR_COULD_NOT_CONVERT_TO_MP4", comment: "Attachment error message for video attachments which could not be converted to MP4") case .couldNotRemoveMetadata: return NSLocalizedString("ATTACHMENT_ERROR_COULD_NOT_REMOVE_METADATA", comment: "Attachment error message for image attachments in which metadata could not be removed") + case .couldNotResizeImage: + return NSLocalizedString("ATTACHMENT_ERROR_COULD_NOT_RESIZE_IMAGE", comment: "Attachment error message for image attachments which could not be resized") } } } @@ -700,7 +703,11 @@ public class SignalAttachment: NSObject { var dstImage: UIImage! = image if image.size.width > maxSize || image.size.height > maxSize { - dstImage = imageScaled(image, toMaxSize: maxSize) + guard let resizedImage = imageScaled(image, toMaxSize: maxSize) else { + attachment.error = .couldNotResizeImage + return attachment + } + dstImage = resizedImage } guard let jpgImageData = UIImageJPEGRepresentation(dstImage, jpegCompressionQuality(imageUploadQuality: imageUploadQuality)) else { @@ -746,20 +753,61 @@ public class SignalAttachment: NSObject { } } - private class func imageScaled(_ image: UIImage, toMaxSize size: CGFloat) -> UIImage { + // NOTE: For unknown reasons, resizing images with UIGraphicsBeginImageContext() + // crashes reliably in the share extension after screen lock's auth UI has been presented. + // Resizing using a CGContext seems to work fine. + private class func imageScaled(_ uiImage: UIImage, toMaxSize size: CGFloat) -> UIImage? { + guard let cgImage = uiImage.cgImage else { + owsFail("\(logTag) UIImage missing cgImage.") + return nil + } + + // It's essential that we work consistently in "CG" coordinates (which are + // and pixels and don't reflect orientation), not "UI" coordinates (which + // are in points and do reflect orientation). + let srcWidth = CGFloat(cgImage.width) + let srcHeight = CGFloat(cgImage.height) var scaleFactor: CGFloat - let aspectRatio: CGFloat = image.size.height / image.size.width + let aspectRatio: CGFloat = srcWidth / srcHeight if aspectRatio > 1 { - scaleFactor = size / image.size.width + scaleFactor = size / srcWidth } else { - scaleFactor = size / image.size.height - } - let newSize = CGSize(width: CGFloat(image.size.width * scaleFactor), height: CGFloat(image.size.height * scaleFactor)) - UIGraphicsBeginImageContext(newSize) - image.draw(in: CGRect(x: CGFloat(0), y: CGFloat(0), width: CGFloat(newSize.width), height: CGFloat(newSize.height))) - let updatedImage: UIImage? = UIGraphicsGetImageFromCurrentImageContext() - UIGraphicsEndImageContext() - return updatedImage! + scaleFactor = size / srcHeight + } + let newSize = CGSize(width: round(srcWidth * scaleFactor), + height: round(srcHeight * scaleFactor)) + + let bitsPerComponent = cgImage.bitsPerComponent + let bytesPerRow = cgImage.bytesPerRow + guard let colorSpace = cgImage.colorSpace else { + owsFail("\(logTag) cgImage missing colorSpace.") + return nil + } + let bitmapInfo = cgImage.bitmapInfo + + guard let context = CGContext.init(data: nil, + width: Int(newSize.width), + height: Int(newSize.height), + bitsPerComponent: bitsPerComponent, + bytesPerRow: bytesPerRow, + space: colorSpace, + bitmapInfo: bitmapInfo.rawValue) else { + owsFail("\(logTag) could not create CGContext.") + return nil + } + context.interpolationQuality = .high + + var drawRect = CGRect.zero + drawRect.size = newSize + context.draw(cgImage, in: drawRect) + + guard let newCGImage = context.makeImage() else { + owsFail("\(logTag) could not create new CGImage.") + return nil + } + return UIImage(cgImage: newCGImage, + scale: uiImage.scale, + orientation: uiImage.imageOrientation) } private class func doesImageHaveAcceptableFileSize(dataSource: DataSource, imageQuality: TSImageQuality) -> Bool { From 56b91ddebb6a72eab9079e9e8ab8d29811f8e50a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 11 May 2018 11:28:05 -0400 Subject: [PATCH 2/3] Clean up ahead of PR. --- Signal/translations/en.lproj/Localizable.strings | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 077d194b2..257cba8e1 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -136,6 +136,9 @@ /* Attachment error message for image attachments in which metadata could not be removed */ "ATTACHMENT_ERROR_COULD_NOT_REMOVE_METADATA" = "Unable to remove metadata from image."; +/* Attachment error message for image attachments which could not be resized */ +"ATTACHMENT_ERROR_COULD_NOT_RESIZE_IMAGE" = "Could not resize image."; + /* Attachment error message for attachments whose data exceed file size limits */ "ATTACHMENT_ERROR_FILE_SIZE_TOO_LARGE" = "Attachment is too large."; From 73206c08ad95f37320b4d067501cd511c082a90c Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 11 May 2018 14:29:28 -0400 Subject: [PATCH 3/3] Respond to CR. --- .../attachments/SignalAttachment.swift | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index 060143dca..bfd45e298 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -756,26 +756,21 @@ public class SignalAttachment: NSObject { // NOTE: For unknown reasons, resizing images with UIGraphicsBeginImageContext() // crashes reliably in the share extension after screen lock's auth UI has been presented. // Resizing using a CGContext seems to work fine. - private class func imageScaled(_ uiImage: UIImage, toMaxSize size: CGFloat) -> UIImage? { + private class func imageScaled(_ uiImage: UIImage, toMaxSize maxSize: CGFloat) -> UIImage? { guard let cgImage = uiImage.cgImage else { owsFail("\(logTag) UIImage missing cgImage.") return nil } // It's essential that we work consistently in "CG" coordinates (which are - // and pixels and don't reflect orientation), not "UI" coordinates (which - // are in points and do reflect orientation). - let srcWidth = CGFloat(cgImage.width) - let srcHeight = CGFloat(cgImage.height) - var scaleFactor: CGFloat - let aspectRatio: CGFloat = srcWidth / srcHeight - if aspectRatio > 1 { - scaleFactor = size / srcWidth - } else { - scaleFactor = size / srcHeight - } - let newSize = CGSize(width: round(srcWidth * scaleFactor), - height: round(srcHeight * scaleFactor)) + // pixels and don't reflect orientation), not "UI" coordinates (which + // are points and do reflect orientation). + let scrSize = CGSize(width: cgImage.width, height: cgImage.height) + var maxSizeRect = CGRect.zero + maxSizeRect.size = CGSize(width: maxSize, height: maxSize) + let newSize = AVMakeRect(aspectRatio: scrSize, insideRect: maxSizeRect).size + assert(newSize.width <= maxSize) + assert(newSize.height <= maxSize) let bitsPerComponent = cgImage.bitsPerComponent let bytesPerRow = cgImage.bytesPerRow