From 13fabbb3059837959c6afa67f2366a9e489a58ea Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 18 Dec 2024 10:04:40 +1100 Subject: [PATCH] Fixed an issue where sharing attachments could lose filename and extension --- .../ConversationVC+Interaction.swift | 6 +-- .../GIFs/GifPickerViewController.swift | 2 +- .../PhotoCapture.swift | 2 +- .../PhotoLibrary.swift | 2 +- Session/Settings/HelpViewModel.swift | 16 +++--- .../Database/Models/Attachment.swift | 7 ++- .../Database/Models/LinkPreview.swift | 8 ++- .../Attachments/SignalAttachment.swift | 8 +-- .../ShareNavController.swift | 10 ++-- SessionUtilitiesKit/Media/DataSource.swift | 21 ++++++-- .../Media/UTType+Utilities.swift | 52 ++++++++++++------- .../AttachmentApprovalViewController.swift | 2 +- 12 files changed, 84 insertions(+), 52 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index 103158663..c09fed7dc 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -378,7 +378,7 @@ extension ConversationVC: } let fileName = urlResourceValues.name ?? "attachment".localized() - guard let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false) else { + guard let dataSource = DataSourcePath(fileUrl: url, sourceFilename: urlResourceValues.name, shouldDeleteOnDeinit: false) else { DispatchQueue.main.async { [weak self] in self?.viewModel.showToast(text: "attachmentsErrorLoad".localized()) } @@ -412,7 +412,7 @@ extension ConversationVC: func showAttachmentApprovalDialogAfterProcessingVideo(at url: URL, with fileName: String) { ModalActivityIndicatorViewController.present(fromViewController: self, canCancel: true, message: nil) { [weak self, dependencies = viewModel.dependencies] modalActivityIndicator in - let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false)! + let dataSource = DataSourcePath(fileUrl: url, sourceFilename: fileName, shouldDeleteOnDeinit: false)! dataSource.sourceFilename = fileName SignalAttachment @@ -2594,7 +2594,7 @@ extension ConversationVC: } // Get data - let dataSourceOrNil = DataSourcePath(fileUrl: audioRecorder.url, shouldDeleteOnDeinit: true) + let dataSourceOrNil = DataSourcePath(fileUrl: audioRecorder.url, sourceFilename: nil, shouldDeleteOnDeinit: true) self.audioRecorder = nil guard let dataSource = dataSourceOrNil else { return SNLog("Couldn't load recorded data.") } diff --git a/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift b/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift index 56fedbab5..c7c389ecd 100644 --- a/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift +++ b/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift @@ -380,7 +380,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect return } - let dataSource = DataSourcePath(filePath: asset.filePath, shouldDeleteOnDeinit: false) + let dataSource = DataSourcePath(filePath: asset.filePath, sourceFilename: URL(fileURLWithPath: asset.filePath).pathExtension, shouldDeleteOnDeinit: false) let attachment = SignalAttachment.attachment(dataSource: dataSource, type: rendition.type, imageQuality: .medium) self?.dismiss(animated: true) { diff --git a/Session/Media Viewing & Editing/PhotoCapture.swift b/Session/Media Viewing & Editing/PhotoCapture.swift index 33c0bc223..291ab6875 100644 --- a/Session/Media Viewing & Editing/PhotoCapture.swift +++ b/Session/Media Viewing & Editing/PhotoCapture.swift @@ -441,7 +441,7 @@ extension PhotoCapture: CaptureOutputDelegate { Log.debug("[PhotoCapture] Ignoring error, since capture succeeded.") } - let dataSource = DataSourcePath(fileUrl: outputFileURL, shouldDeleteOnDeinit: true) + let dataSource = DataSourcePath(fileUrl: outputFileURL, sourceFilename: nil, shouldDeleteOnDeinit: true) let attachment = SignalAttachment.attachment(dataSource: dataSource, type: .mpeg4Movie) delegate?.photoCapture(self, didFinishProcessingAttachment: attachment) } diff --git a/Session/Media Viewing & Editing/PhotoLibrary.swift b/Session/Media Viewing & Editing/PhotoLibrary.swift index e31f1c604..a6617060e 100644 --- a/Session/Media Viewing & Editing/PhotoLibrary.swift +++ b/Session/Media Viewing & Editing/PhotoLibrary.swift @@ -196,7 +196,7 @@ class PhotoCollectionContents { guard exportSession?.status == .completed, - let dataSource = DataSourcePath(fileUrl: exportURL, shouldDeleteOnDeinit: true) + let dataSource = DataSourcePath(fileUrl: exportURL, sourceFilename: nil, shouldDeleteOnDeinit: true) else { resolver(Result.failure(PhotoLibraryError.assertionError(description: "Failed to build data source for exported video URL"))) return diff --git a/Session/Settings/HelpViewModel.swift b/Session/Settings/HelpViewModel.swift index 2530fd329..f7ba3506d 100644 --- a/Session/Settings/HelpViewModel.swift +++ b/Session/Settings/HelpViewModel.swift @@ -169,13 +169,15 @@ class HelpViewModel: SessionTableViewModel, NavigatableStateHolder, ObservableTa cancelTitle: "Share", cancelStyle: .alert_text, onConfirm: { _ in UIPasteboard.general.string = latestLogFilePath }, - onCancel: { _ in - HelpViewModel.shareLogsInternal( - viewControllerToDismiss: viewControllerToDismiss, - targetView: targetView, - animated: animated, - onShareComplete: onShareComplete - ) + onCancel: { modal in + modal.dismiss(animated: true) { + HelpViewModel.shareLogsInternal( + viewControllerToDismiss: viewControllerToDismiss, + targetView: targetView, + animated: animated, + onShareComplete: onShareComplete + ) + } } ) ) diff --git a/SessionMessagingKit/Database/Models/Attachment.swift b/SessionMessagingKit/Database/Models/Attachment.swift index 6e4263274..ebc24ffd6 100644 --- a/SessionMessagingKit/Database/Models/Attachment.swift +++ b/SessionMessagingKit/Database/Models/Attachment.swift @@ -638,7 +638,10 @@ extension Attachment { // If the filename has not file extension, deduce one // from the MIME type. if targetFileExtension.isEmpty { - targetFileExtension = (UTType(sessionMimeType: mimeType)?.sessionFileExtension ?? UTType.fileExtensionDefault) + targetFileExtension = ( + UTType(sessionMimeType: mimeType)?.sessionFileExtension(sourceFilename: sourceFilename) ?? + UTType.fileExtensionDefault + ) } targetFileExtension = targetFileExtension.lowercased() @@ -657,7 +660,7 @@ extension Attachment { } let targetFileExtension: String = ( - UTType(sessionMimeType: mimeType)?.sessionFileExtension ?? + UTType(sessionMimeType: mimeType)?.sessionFileExtension(sourceFilename: sourceFilename) ?? UTType.fileExtensionDefault ).lowercased() diff --git a/SessionMessagingKit/Database/Models/LinkPreview.swift b/SessionMessagingKit/Database/Models/LinkPreview.swift index 05d57c2f3..9db5f25c9 100644 --- a/SessionMessagingKit/Database/Models/LinkPreview.swift +++ b/SessionMessagingKit/Database/Models/LinkPreview.swift @@ -132,12 +132,16 @@ public extension LinkPreview { static func generateAttachmentIfPossible(imageData: Data?, type: UTType) throws -> Attachment? { guard let imageData: Data = imageData, !imageData.isEmpty else { return nil } - guard let fileExtension: String = type.sessionFileExtension else { return nil } + guard let fileExtension: String = type.sessionFileExtension(sourceFilename: nil) else { return nil } guard let mimeType: String = type.preferredMIMEType else { return nil } let filePath = FileSystem.temporaryFilePath(fileExtension: fileExtension) try imageData.write(to: NSURL.fileURL(withPath: filePath), options: .atomicWrite) - let dataSource: DataSourcePath = DataSourcePath(filePath: filePath, shouldDeleteOnDeinit: true) + let dataSource: DataSourcePath = DataSourcePath( + filePath: filePath, + sourceFilename: nil, + shouldDeleteOnDeinit: true + ) return Attachment(contentType: mimeType, dataSource: dataSource) } diff --git a/SessionMessagingKit/Sending & Receiving/Attachments/SignalAttachment.swift b/SessionMessagingKit/Sending & Receiving/Attachments/SignalAttachment.swift index a7f7c3c39..ce8531edb 100644 --- a/SessionMessagingKit/Sending & Receiving/Attachments/SignalAttachment.swift +++ b/SessionMessagingKit/Sending & Receiving/Attachments/SignalAttachment.swift @@ -269,7 +269,7 @@ public class SignalAttachment: Equatable { // can be identified. public var mimeType: String { guard - let fileExtension: String = sourceFilename.map({ $0 as NSString })?.pathExtension, + let fileExtension: String = sourceFilename.map({ URL(fileURLWithPath: $0) })?.pathExtension, !fileExtension.isEmpty, let fileExtensionMimeType: String = UTType(sessionFileExtension: fileExtension)?.preferredMIMEType else { return (dataType.preferredMIMEType ?? UTType.mimeTypeDefault) } @@ -306,9 +306,9 @@ public class SignalAttachment: Equatable { // can be identified. public var fileExtension: String? { guard - let fileExtension: String = sourceFilename.map({ $0 as NSString })?.pathExtension, + let fileExtension: String = sourceFilename.map({ URL(fileURLWithPath: $0) })?.pathExtension, !fileExtension.isEmpty - else { return dataType.sessionFileExtension } + else { return dataType.sessionFileExtension(sourceFilename: sourceFilename) } return fileExtension.filteredFilename } @@ -803,7 +803,7 @@ public class SignalAttachment: Equatable { let baseFilename = dataSource.sourceFilename let mp4Filename = baseFilename?.filenameWithoutExtension.appendingFileExtension("mp4") - guard let dataSource = DataSourcePath(fileUrl: exportURL, shouldDeleteOnDeinit: true) else { + guard let dataSource = DataSourcePath(fileUrl: exportURL, sourceFilename: baseFilename, shouldDeleteOnDeinit: true) else { let attachment = SignalAttachment(dataSource: DataSourceValue.empty, dataType: type) attachment.error = .couldNotConvertToMpeg4 resolver(Result.success(attachment)) diff --git a/SessionShareExtension/ShareNavController.swift b/SessionShareExtension/ShareNavController.swift index f34d1ec15..964ca1743 100644 --- a/SessionShareExtension/ShareNavController.swift +++ b/SessionShareExtension/ShareNavController.swift @@ -274,16 +274,14 @@ final class ShareNavController: UINavigationController, ShareViewDelegate { // // NOTE: SharingThreadPickerViewController will try to unpack them // and send them as normal text messages if possible. - case (_, true): return DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false) + case (_, true): + return DataSourcePath(fileUrl: url, sourceFilename: customFileName, shouldDeleteOnDeinit: false) default: - guard let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false) else { + guard let dataSource = DataSourcePath(fileUrl: url, sourceFilename: customFileName, shouldDeleteOnDeinit: false) else { return nil } - // Fallback to the last part of the URL - dataSource.sourceFilename = (customFileName ?? url.lastPathComponent) - return dataSource } } @@ -429,7 +427,7 @@ final class ShareNavController: UINavigationController, ShareViewDelegate { switch value { case let data as Data: let customFileName = "Contact.vcf" // stringlint:ignore - let customFileExtension: String? = srcType.sessionFileExtension + let customFileExtension: String? = srcType.sessionFileExtension(sourceFilename: nil) guard let tempFilePath = try? FileSystem.write(data: data, toTemporaryFileWithExtension: customFileExtension) else { resolver( diff --git a/SessionUtilitiesKit/Media/DataSource.swift b/SessionUtilitiesKit/Media/DataSource.swift index 1cc3bf20d..e603b8434 100644 --- a/SessionUtilitiesKit/Media/DataSource.swift +++ b/SessionUtilitiesKit/Media/DataSource.swift @@ -97,7 +97,7 @@ public class DataSourceValue: DataSource { } public convenience init?(data: Data?, dataType: UTType) { - guard let fileExtension: String = dataType.sessionFileExtension else { return nil } + guard let fileExtension: String = dataType.sessionFileExtension(sourceFilename: nil) else { return nil } self.init(data: data, fileExtension: fileExtension) } @@ -210,15 +210,28 @@ public class DataSourcePath: DataSource { // MARK: - Initialization - public init(filePath: String, shouldDeleteOnDeinit: Bool) { + public init( + filePath: String, + sourceFilename: String?, + shouldDeleteOnDeinit: Bool + ) { self.filePath = filePath + self.sourceFilename = sourceFilename self.shouldDeleteOnDeinit = shouldDeleteOnDeinit } - public convenience init?(fileUrl: URL?, shouldDeleteOnDeinit: Bool) { + public convenience init?( + fileUrl: URL?, + sourceFilename: String?, + shouldDeleteOnDeinit: Bool + ) { guard let fileUrl: URL = fileUrl, fileUrl.isFileURL else { return nil } - self.init(filePath: fileUrl.path, shouldDeleteOnDeinit: shouldDeleteOnDeinit) + self.init( + filePath: fileUrl.path, + sourceFilename: (sourceFilename ?? fileUrl.lastPathComponent), + shouldDeleteOnDeinit: shouldDeleteOnDeinit + ) } deinit { diff --git a/SessionUtilitiesKit/Media/UTType+Utilities.swift b/SessionUtilitiesKit/Media/UTType+Utilities.swift index 64aef20b7..59353ead1 100644 --- a/SessionUtilitiesKit/Media/UTType+Utilities.swift +++ b/SessionUtilitiesKit/Media/UTType+Utilities.swift @@ -107,26 +107,6 @@ public extension UTType { var isMicrosoftDoc: Bool { UTType.supportedMicrosoftDocTypes.contains(self) } var isVisualMedia: Bool { isImage || isVideo || isAnimated } - var sessionFileExtension: String? { - // Special-case the "aac" filetype we use for voice messages (for legacy reasons) - // to use a .m4a file extension, not .aac, since AVAudioPlayer can't handle .aac - // properly. Doesn't affect file contents. - guard identifier != "public.aac-audio" else { return "m4a" } - - // Try to deduce the file extension by using a lookup table. - // - // This should be more accurate than deducing the file extension by - // converting to a UTI type. For example, .m4a files will have a - // UTI type of kUTTypeMPEG4Audio which incorrectly yields the file - // extension .mp4 instead of .m4a. - guard - let mimeType: String = preferredMIMEType, - let fileExtension: String = UTType.genericMimeTypesToExtensionTypes[mimeType] - else { return preferredFilenameExtension } - - return fileExtension - } - // MARK: - Initialization init?(sessionFileExtension: String) { @@ -188,6 +168,38 @@ public extension UTType { return (UTType(sessionMimeType: mimeType) ?? .invalid).isVisualMedia } + func sessionFileExtension(sourceFilename: String?) -> String? { + // First try to check if the file extension on `sourceFilename` is valid + // for the `preferredMIMEType` as we want to keep that if so (eg. use `.log` + // instead of `.txt`) + if + let sourceFileExtension: String = sourceFilename.map({ URL(fileURLWithPath: $0) })?.pathExtension, + let mimeType: String = preferredMIMEType, + let sourceExtensionMimeType: String = UTType.genericMimeTypesToExtensionTypes[sourceFileExtension], + UTType(mimeType: sourceExtensionMimeType)?.preferredMIMEType == mimeType + { + return sourceFileExtension + } + + // Special-case the "aac" filetype we use for voice messages (for legacy reasons) + // to use a .m4a file extension, not .aac, since AVAudioPlayer can't handle .aac + // properly. Doesn't affect file contents. + guard identifier != "public.aac-audio" else { return "m4a" } + + // Try to deduce the file extension by using a lookup table. + // + // This should be more accurate than deducing the file extension by + // converting to a UTI type. For example, .m4a files will have a + // UTI type of kUTTypeMPEG4Audio which incorrectly yields the file + // extension .mp4 instead of .m4a. + guard + let mimeType: String = preferredMIMEType, + let fileExtension: String = UTType.genericMimeTypesToExtensionTypes[mimeType] + else { return preferredFilenameExtension } + + return fileExtension + } + // MARK: - Lookup Table static func sessionMimeType(for fileExtension: String) -> String? { diff --git a/SignalUtilitiesKit/Media Viewing & Editing/Attachment Approval/AttachmentApprovalViewController.swift b/SignalUtilitiesKit/Media Viewing & Editing/Attachment Approval/AttachmentApprovalViewController.swift index 904a26bf9..63f40c158 100644 --- a/SignalUtilitiesKit/Media Viewing & Editing/Attachment Approval/AttachmentApprovalViewController.swift +++ b/SignalUtilitiesKit/Media Viewing & Editing/Attachment Approval/AttachmentApprovalViewController.swift @@ -533,7 +533,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC // Rewrite the filename's extension to reflect the output file format. var filename: String? = attachmentItem.attachment.sourceFilename if let sourceFilename = attachmentItem.attachment.sourceFilename { - if let fileExtension: String = dataType.sessionFileExtension { + if let fileExtension: String = dataType.sessionFileExtension(sourceFilename: sourceFilename) { filename = (sourceFilename as NSString).deletingPathExtension.appendingFileExtension(fileExtension) } }