Merge pull request #343 from mpretty-cyro/fix/file-sharing-bugs

Fixed an issue where sharing attachments could lose filename and extension
session-foundation · Dec 17, 2024 · a86ea3a · a86ea3a
2 parents d010aa7 + 13fabbb
commit a86ea3a
Show file tree

Hide file tree

Showing 12 changed files with 84 additions and 52 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...iesKit/Media Viewing & Editing/Attachment Approval/AttachmentApprovalViewController.swift b/...iesKit/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)
             }
         }