From 188439eef7652d3557b75ef35ab6310a53d28dff Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Thu, 10 Apr 2025 09:56:25 +0100 Subject: [PATCH] Only use the appGroupTemporaryDirectory to access a file from the share extension. (#4002) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … and switch back to the plain `URL.temporaryDirectory` for everything else. * Fix documentation Co-authored-by: Stefan Ceriu --- .../Sources/Application/AppCoordinator.swift | 37 +++++++++++- .../Other/Extensions/FileManager.swift | 5 +- .../Other/Extensions/NSItemProvider.swift | 56 ++++++++++++------- ElementX/Sources/Other/Extensions/URL.swift | 5 +- .../ShareExtension/ShareExtensionModels.swift | 21 +++++++ .../ShareExtensionViewController.swift | 2 +- 6 files changed, 99 insertions(+), 27 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index 1274f5e8b..4db2553c1 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -125,6 +125,9 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg if let previousVersion = appSettings.lastVersionLaunched.flatMap(Version.init) { performMigrationsIfNecessary(from: previousVersion, to: currentVersion) + + // Manual clean to handle the potential case where the app crashes before moving a shared file. + cleanAppGroupTemporaryDirectory() } else { // The app has been deleted since the previous run. Reset everything. wipeUserData(includingSettings: true) @@ -252,12 +255,17 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg } else { handleAppRoute(.childEventOnRoomAlias(eventID: eventID, alias: alias)) } - case .share: + case .share(let payload): guard isExternalURL else { MXLog.error("Received unexpected internal share route") break } - handleAppRoute(route) + + do { + try handleAppRoute(.share(payload.withDefaultTemporaryDirectory())) + } catch { + MXLog.error("Failed moving payload out of the app group container: \(error)") + } default: break } @@ -408,6 +416,31 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg userSessionStore.reset() } + /// Manually cleans up any files in the app group's `tmp` directory. + /// + /// **Note:** If there is a single file we consider it to be an active share payload and ignore it. + private func cleanAppGroupTemporaryDirectory() { + let fileURLs: [URL] + do { + fileURLs = try FileManager.default.contentsOfDirectory(at: URL.appGroupTemporaryDirectory, includingPropertiesForKeys: nil, options: []) + } catch { + MXLog.warning("Failed to enumerate app group temporary directory: \(error)") + return + } + + guard fileURLs.count > 1 else { + return // If there is only a single item in here, there's likely a pending share payload that is yet to be processed. + } + + for url in fileURLs { + do { + try FileManager.default.removeItem(at: url) + } catch { + MXLog.warning("Failed to remove file from app group temporary directory: \(error)") + } + } + } + private func setupStateMachine() { stateMachine.addTransitionHandler { [weak self] context in guard let self else { return } diff --git a/ElementX/Sources/Other/Extensions/FileManager.swift b/ElementX/Sources/Other/Extensions/FileManager.swift index 849a02b24..92c62f20e 100644 --- a/ElementX/Sources/Other/Extensions/FileManager.swift +++ b/ElementX/Sources/Other/Extensions/FileManager.swift @@ -37,8 +37,9 @@ extension FileManager { } @discardableResult - func writeDataToTemporaryDirectory(data: Data, fileName: String) throws -> URL { - let newURL = URL.appGroupTemporaryDirectory.appendingPathComponent(fileName) + func writeDataToTemporaryDirectory(data: Data, fileName: String, withinAppGroupContainer: Bool = false) throws -> URL { + let baseURL: URL = withinAppGroupContainer ? .appGroupTemporaryDirectory : .temporaryDirectory + let newURL = baseURL.appendingPathComponent(fileName) try data.write(to: newURL) diff --git a/ElementX/Sources/Other/Extensions/NSItemProvider.swift b/ElementX/Sources/Other/Extensions/NSItemProvider.swift index 287c5ce29..4a7f07353 100644 --- a/ElementX/Sources/Other/Extensions/NSItemProvider.swift +++ b/ElementX/Sources/Other/Extensions/NSItemProvider.swift @@ -28,20 +28,23 @@ extension NSItemProvider { try? await loadItem(forTypeIdentifier: UTType.text.identifier) as? String } - func storeData() async -> URL? { + /// Stores the item's data from the provider within the temporary directory, returning the URL on success. + /// - Parameter withinAppGroupContainer: Whether the data needs to be shared between bundles. + /// If passing `true` you will need to manually clean up the file once you have the data in the receiving bundle. + func storeData(withinAppGroupContainer: Bool = false) async -> URL? { guard let contentType = preferredContentType else { MXLog.error("Invalid NSItemProvider: \(self)") return nil } if contentType.type.identifier == UTType.image.identifier { - return await generateURLForUIImage(contentType) + return await generateURLForUIImage(contentType, withinAppGroupContainer: withinAppGroupContainer) } else { - return await generateURLForGenericData(contentType) + return await generateURLForGenericData(contentType, withinAppGroupContainer: withinAppGroupContainer) } } - private func generateURLForUIImage(_ contentType: PreferredContentType) async -> URL? { + private func generateURLForUIImage(_ contentType: PreferredContentType, withinAppGroupContainer: Bool) async -> URL? { guard let uiImage = try? await loadItem(forTypeIdentifier: contentType.type.identifier) as? UIImage else { MXLog.error("Failed casting UIImage, invalid NSItemProvider: \(self)") return nil @@ -52,22 +55,25 @@ extension NSItemProvider { return nil } + let filename = if let suggestedName = suggestedName as NSString?, + // Suggestions are nice but their extension is `jpeg` + let filename = (suggestedName.deletingPathExtension as NSString).appendingPathExtension(contentType.fileExtension) { + filename + } else { + "\(UUID().uuidString).\(contentType.fileExtension)" + } + do { - if let suggestedName = suggestedName as? NSString, - // Suggestions are nice but their extension is `jpeg` - let filename = (suggestedName.deletingPathExtension as NSString).appendingPathExtension(contentType.fileExtension) { - return try FileManager.default.writeDataToTemporaryDirectory(data: pngData, fileName: filename) - } else { - let filename = "\(UUID().uuidString).\(contentType.fileExtension)" - return try FileManager.default.writeDataToTemporaryDirectory(data: pngData, fileName: filename) - } + return try FileManager.default.writeDataToTemporaryDirectory(data: pngData, + fileName: filename, + withinAppGroupContainer: withinAppGroupContainer) } catch { MXLog.error("Failed storing NSItemProvider data \(self) with error: \(error)") return nil } } - private func generateURLForGenericData(_ contentType: PreferredContentType) async -> URL? { + private func generateURLForGenericData(_ contentType: PreferredContentType, withinAppGroupContainer: Bool) async -> URL? { let providerDescription = description let shareData: Data? = await withCheckedContinuation { continuation in _ = loadDataRepresentation(for: contentType.type) { data, error in @@ -92,15 +98,19 @@ extension NSItemProvider { return nil } + let filename = if let suggestedName = suggestedName as NSString?, + suggestedName.hasPathExtension { + suggestedName as String + } else if let suggestedName { + "\(suggestedName).\(contentType.fileExtension)" + } else { + "\(UUID().uuidString).\(contentType.fileExtension)" + } + do { - if let filename = suggestedName { - let hasExtension = !(filename as NSString).pathExtension.isEmpty - let filename = hasExtension ? filename : "\(filename).\(contentType.fileExtension)" - return try FileManager.default.writeDataToTemporaryDirectory(data: shareData, fileName: filename) - } else { - let filename = "\(UUID().uuidString).\(contentType.fileExtension)" - return try FileManager.default.writeDataToTemporaryDirectory(data: shareData, fileName: filename) - } + return try FileManager.default.writeDataToTemporaryDirectory(data: shareData, + fileName: filename, + withinAppGroupContainer: withinAppGroupContainer) } catch { MXLog.error("Failed storing NSItemProvider data \(self) with error: \(error)") return nil @@ -164,3 +174,7 @@ extension NSItemProvider { return mimeType.hasPrefix("image/") || mimeType.hasPrefix("video/") || mimeType.hasPrefix("application/") } } + +private extension NSString { + var hasPathExtension: Bool { !pathExtension.isEmpty } +} diff --git a/ElementX/Sources/Other/Extensions/URL.swift b/ElementX/Sources/Other/Extensions/URL.swift index 11925e8a9..6ed52c0de 100644 --- a/ElementX/Sources/Other/Extensions/URL.swift +++ b/ElementX/Sources/Other/Extensions/URL.swift @@ -75,7 +75,10 @@ extension URL: @retroactive ExpressibleByStringLiteral { return url } - /// The app group temporary directory + /// The app group temporary directory (useful for transferring files between different bundles). + /// + /// **Note:** This `tmp` directory doesn't appear to behave as expected as it isn't being tidied up by the system. + /// Make sure to manually tidy up any files you place in here once you've transferred them from one bundle to another. static var appGroupTemporaryDirectory: URL { let url = appGroupContainerDirectory .appendingPathComponent("tmp", isDirectory: true) diff --git a/ElementX/Sources/ShareExtension/ShareExtensionModels.swift b/ElementX/Sources/ShareExtension/ShareExtensionModels.swift index baac484c0..e00a9f358 100644 --- a/ElementX/Sources/ShareExtension/ShareExtensionModels.swift +++ b/ElementX/Sources/ShareExtension/ShareExtensionModels.swift @@ -22,9 +22,30 @@ enum ShareExtensionPayload: Hashable, Codable { roomID } } + + /// Moves any files in the payload from our `appGroupTemporaryDirectory` to the + /// system's `temporaryDirectory` returning a modified payload with updated file URLs. + func withDefaultTemporaryDirectory() throws -> Self { + switch self { + case .mediaFile(let roomID, let mediaFile): + let path = mediaFile.url.path.replacing(URL.appGroupTemporaryDirectory.path, with: "").trimmingPrefix("/") + let newURL = URL.temporaryDirectory.appending(path: path) + + try? FileManager.default.removeItem(at: newURL) + try FileManager.default.moveItem(at: mediaFile.url, to: newURL) + + return .mediaFile(roomID: roomID, mediaFile: mediaFile.replacingURL(with: newURL)) + case .text: + return self + } + } } struct ShareExtensionMediaFile: Hashable, Codable { let url: URL let suggestedName: String? + + fileprivate func replacingURL(with newURL: URL) -> ShareExtensionMediaFile { + ShareExtensionMediaFile(url: newURL, suggestedName: suggestedName) + } } diff --git a/ShareExtension/Sources/ShareExtensionViewController.swift b/ShareExtension/Sources/ShareExtensionViewController.swift index f2eee72f2..24de9afc4 100644 --- a/ShareExtension/Sources/ShareExtensionViewController.swift +++ b/ShareExtension/Sources/ShareExtensionViewController.swift @@ -44,7 +44,7 @@ class ShareExtensionViewController: UIViewController { let roomID = (extensionContext?.intent as? INSendMessageIntent)?.conversationIdentifier - if let fileURL = await itemProvider.storeData() { + if let fileURL = await itemProvider.storeData(withinAppGroupContainer: true) { return .mediaFile(roomID: roomID, mediaFile: .init(url: fileURL, suggestedName: fileURL.lastPathComponent)) } else if let url = await itemProvider.loadTransferable(type: URL.self) { return .text(roomID: roomID, text: url.absoluteString)