diff --git a/Signal/Attachments/SignalAttachmentCloner.swift b/Signal/Attachments/SignalAttachmentCloner.swift index a36b7d0a99..aef464b049 100644 --- a/Signal/Attachments/SignalAttachmentCloner.swift +++ b/Signal/Attachments/SignalAttachmentCloner.swift @@ -17,7 +17,7 @@ enum SignalAttachmentCloner { filename: attachment.reference.sourceFilename ) - let decryptedDataSource = DataSourcePath(fileUrl: decryptedCopyUrl, shouldDeleteOnDeallocation: true) + let decryptedDataSource = DataSourcePath(fileUrl: decryptedCopyUrl, ownership: .owned) decryptedDataSource.sourceFilename = attachment.reference.sourceFilename let signalAttachment: SignalAttachment diff --git a/Signal/ConversationView/ConversationViewController+ConversationInputToolbarDelegate.swift b/Signal/ConversationView/ConversationViewController+ConversationInputToolbarDelegate.swift index 002d3d0426..caedfd3839 100644 --- a/Signal/ConversationView/ConversationViewController+ConversationInputToolbarDelegate.swift +++ b/Signal/ConversationView/ConversationViewController+ConversationInputToolbarDelegate.swift @@ -806,7 +806,7 @@ extension ConversationViewController: UIDocumentPickerDelegate { return } - let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeallocation: false) + let dataSource = DataSourcePath(fileUrl: url, ownership: .owned) dataSource.sourceFilename = filename let contentTypeIdentifier = (resourceValues?.contentType ?? .data).identifier diff --git a/Signal/ConversationView/VoiceMessage/VoiceMessageSendableDraft.swift b/Signal/ConversationView/VoiceMessage/VoiceMessageSendableDraft.swift index d8e26942d1..bc2a178793 100644 --- a/Signal/ConversationView/VoiceMessage/VoiceMessageSendableDraft.swift +++ b/Signal/ConversationView/VoiceMessage/VoiceMessageSendableDraft.swift @@ -27,7 +27,7 @@ extension VoiceMessageSendableDraft { func prepareAttachment() throws -> PreviewableAttachment { let attachmentUrl = try prepareForSending() - let dataSource = DataSourcePath(fileUrl: attachmentUrl, shouldDeleteOnDeallocation: true) + let dataSource = DataSourcePath(fileUrl: attachmentUrl, ownership: .owned) dataSource.sourceFilename = userVisibleFilename(currentDate: Date()) return PreviewableAttachment(rawValue: try SignalAttachment.voiceMessageAttachment(dataSource: dataSource, dataUTI: UTType.mpeg4Audio.identifier)) diff --git a/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift b/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift index 403e386b5c..57476a5693 100644 --- a/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift +++ b/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift @@ -510,7 +510,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect let consumableFilePath = OWSFileSystem.temporaryFilePath(fileExtension: assetFileExtension) try FileManager.default.copyItem(atPath: assetFilePath, toPath: consumableFilePath) - let dataSource = DataSourcePath(filePath: consumableFilePath, shouldDeleteOnDeallocation: false) + let dataSource = DataSourcePath(filePath: consumableFilePath, ownership: .owned) let attachment = try SignalAttachment.attachment(dataSource: dataSource, dataUTI: assetTypeIdentifier) attachment.isLoopingVideo = attachment.isVideo diff --git a/Signal/src/ViewControllers/Photos/CameraCaptureSession.swift b/Signal/src/ViewControllers/Photos/CameraCaptureSession.swift index b9e00a2013..72d575a785 100644 --- a/Signal/src/ViewControllers/Photos/CameraCaptureSession.swift +++ b/Signal/src/ViewControllers/Photos/CameraCaptureSession.swift @@ -831,7 +831,7 @@ class CameraCaptureSession: NSObject { private func handleVideoRecording(at outputUrl: URL) throws { AssertIsOnMainThread() - let dataSource = DataSourcePath(fileUrl: outputUrl, shouldDeleteOnDeallocation: true) + let dataSource = DataSourcePath(fileUrl: outputUrl, ownership: .owned) let attachment = try SignalAttachment.videoAttachment( dataSource: dataSource, dataUTI: UTType.mpeg4Movie.identifier, diff --git a/Signal/test/attachments/SignalAttachmentTest.swift b/Signal/test/attachments/SignalAttachmentTest.swift index ea0fe160df..98e9a1e251 100644 --- a/Signal/test/attachments/SignalAttachmentTest.swift +++ b/Signal/test/attachments/SignalAttachmentTest.swift @@ -14,7 +14,7 @@ class SignalAttachmentTest: SignalBaseTest { func testMetadataStrippingDoesNotChangeOrientation(url: URL) throws { let size = (try? DataImageSource.forPath(url.path).imageMetadata()?.pixelSize) ?? .zero - let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeallocation: false) + let dataSource = DataSourcePath(fileUrl: url, ownership: .borrowed) let attachment = try SignalAttachment.imageAttachment( dataSource: dataSource, dataUTI: UTType.jpeg.identifier @@ -52,7 +52,7 @@ class SignalAttachmentTest: SignalBaseTest { func testRemoveMetadataFromPng() throws { let testBundle = Bundle(for: Self.self) let url = testBundle.url(forResource: "test-png-with-metadata", withExtension: "png")! - let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeallocation: false) + let dataSource = DataSourcePath(fileUrl: url, ownership: .borrowed) XCTAssertEqual( try pngChunkTypes(data: dataSource.readData()), ["IHDR", "PLTE", "sRGB", "tIME", "tEXt", "IDAT", "IEND"], diff --git a/SignalServiceKit/Attachments/SendableAttachment.swift b/SignalServiceKit/Attachments/SendableAttachment.swift index 5c89922a2d..93de5df0ae 100644 --- a/SignalServiceKit/Attachments/SendableAttachment.swift +++ b/SignalServiceKit/Attachments/SendableAttachment.swift @@ -134,7 +134,7 @@ public struct SendableAttachment { } let segments = try segmentFileUrls.map { url in - let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeallocation: true) + let dataSource = DataSourcePath(fileUrl: url, ownership: .owned) // [15M] TODO: This doesn't transfer all SignalAttachment fields. let attachment = try SignalAttachment.videoAttachment(dataSource: dataSource, dataUTI: self.dataUTI) return Self(nonImagePreviewableAttachment: PreviewableAttachment(rawValue: attachment)) diff --git a/SignalServiceKit/Attachments/SignalAttachment+Sending.swift b/SignalServiceKit/Attachments/SignalAttachment+Sending.swift index fe6af91a11..716b11604c 100644 --- a/SignalServiceKit/Attachments/SignalAttachment+Sending.swift +++ b/SignalServiceKit/Attachments/SignalAttachment+Sending.swift @@ -20,7 +20,6 @@ extension SendableAttachment { public func forSending(attachmentContentValidator: any AttachmentContentValidator) async throws -> ForSending { let dataSource = try await attachmentContentValidator.validateContents( sendableAttachment: self, - shouldConsume: true, shouldUseDefaultFilename: true, ) return ForSending( diff --git a/SignalServiceKit/Attachments/SignalAttachment.swift b/SignalServiceKit/Attachments/SignalAttachment.swift index 7565bc0033..6738661699 100644 --- a/SignalServiceKit/Attachments/SignalAttachment.swift +++ b/SignalServiceKit/Attachments/SignalAttachment.swift @@ -848,7 +848,7 @@ public class SignalAttachment: CustomDebugStringConvertible { throw .couldNotConvertImage } - let outputDataSource = DataSourcePath(fileUrl: tempFileUrl, shouldDeleteOnDeallocation: false) + let outputDataSource = DataSourcePath(fileUrl: tempFileUrl, ownership: .owned) let outputFileSize: UInt64 do { outputFileSize = try outputDataSource.readLength() @@ -1085,7 +1085,7 @@ public class SignalAttachment: CustomDebugStringConvertible { mp4Filename = nil } - let dataSource = DataSourcePath(fileUrl: exportURL, shouldDeleteOnDeallocation: true) + let dataSource = DataSourcePath(fileUrl: exportURL, ownership: .owned) dataSource.sourceFilename = mp4Filename let endTime = MonotonicDate() diff --git a/SignalServiceKit/Contacts/OWSSyncManager.swift b/SignalServiceKit/Contacts/OWSSyncManager.swift index 6a767fd333..f75625f7d7 100644 --- a/SignalServiceKit/Contacts/OWSSyncManager.swift +++ b/SignalServiceKit/Contacts/OWSSyncManager.swift @@ -506,7 +506,7 @@ extension OWSSyncManager: SyncManagerProtocol, SyncManagerProtocolSwift { return } - let dataSource = DataSourcePath(fileUrl: result.syncFileUrl, shouldDeleteOnDeallocation: true) + let dataSource = DataSourcePath(fileUrl: result.syncFileUrl, ownership: .owned) let uploadResult = try await DependenciesBridge.shared.attachmentUploadManager.uploadTransientAttachment(dataSource: dataSource) let message = SSKEnvironment.shared.databaseStorageRef.read { tx in diff --git a/SignalServiceKit/Devices/LinkAndSyncManager.swift b/SignalServiceKit/Devices/LinkAndSyncManager.swift index c074663d12..c0ec277a07 100644 --- a/SignalServiceKit/Devices/LinkAndSyncManager.swift +++ b/SignalServiceKit/Devices/LinkAndSyncManager.swift @@ -484,7 +484,7 @@ public class LinkAndSyncManagerImpl: LinkAndSyncManager { ) async throws(PrimaryLinkNSyncError) -> Upload.Result { do { return try await attachmentUploadManager.uploadLinkNSyncAttachment( - dataSource: DataSourcePath(fileUrl: metadata.fileUrl, shouldDeleteOnDeallocation: true), + dataSource: DataSourcePath(fileUrl: metadata.fileUrl, ownership: .owned), progress: progress ) } catch { diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift index 6c981717a9..c3424d9101 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift @@ -66,12 +66,8 @@ public protocol AttachmentContentValidator { /// Returns a PendingAttachment with validated contents, ready to be inserted. /// Note the content type may be `invalid`; we can still create an Attachment from these. /// Errors are thrown if data reading/parsing fails. - /// - /// - Parameter shouldConsume: If true, the source file will be deleted and the DataSource - /// consumed after validation is complete; otherwise the source file will be left as-is. func validateContents( dataSource: DataSourcePath, - shouldConsume: Bool, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, sourceFilename: String? @@ -189,12 +185,10 @@ extension AttachmentContentValidator { public func validateContents( sendableAttachment: SendableAttachment, - shouldConsume: Bool, shouldUseDefaultFilename: Bool, ) async throws -> AttachmentDataSource { return try await validateContents( dataSource: sendableAttachment.dataSource, - shouldConsume: shouldConsume, mimeType: sendableAttachment.mimeType, renderingFlag: sendableAttachment.renderingFlag, sourceFilename: sendableAttachment.sourceFilename?.rawValue ?? (shouldUseDefaultFilename ? sendableAttachment.defaultFilename : nil), @@ -203,14 +197,12 @@ extension AttachmentContentValidator { public func validateContents( dataSource: DataSourcePath, - shouldConsume: Bool, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, sourceFilename: String? ) async throws -> AttachmentDataSource { return await .from(pendingAttachment: try self.validateContents( dataSource: dataSource, - shouldConsume: shouldConsume, mimeType: mimeType, renderingFlag: renderingFlag, sourceFilename: sourceFilename diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift index c663eef344..419bb49a41 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift @@ -28,7 +28,6 @@ public class AttachmentContentValidatorImpl: AttachmentContentValidator { public func validateContents( dataSource: DataSourcePath, - shouldConsume: Bool, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, sourceFilename: String? @@ -46,11 +45,7 @@ public class AttachmentContentValidatorImpl: AttachmentContentValidator { sourceFilename: sourceFilename )] ).values.first! - - if shouldConsume { - try dataSource.consumeAndDelete() - } - + try dataSource.consumeAndDeleteIfNecessary() return pendingAttachment } diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift index 1cccf9194a..cda4619319 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift @@ -13,7 +13,6 @@ open class AttachmentContentValidatorMock: AttachmentContentValidator { open func validateContents( dataSource: DataSourcePath, - shouldConsume: Bool, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, sourceFilename: String? diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift index aa378a4c5f..046b541852 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift @@ -1928,8 +1928,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { } return try await attachmentValidator.validateContents( - dataSource: DataSourcePath(fileUrl: stickerDataUrl, shouldDeleteOnDeallocation: false), - shouldConsume: false, + dataSource: DataSourcePath(fileUrl: stickerDataUrl, ownership: .borrowed), mimeType: mimeType.rawValue, renderingFlag: .borderless, sourceFilename: nil diff --git a/SignalServiceKit/Messages/Stories/SystemStoryManager.swift b/SignalServiceKit/Messages/Stories/SystemStoryManager.swift index 4a94efd353..4a1772762b 100644 --- a/SignalServiceKit/Messages/Stories/SystemStoryManager.swift +++ b/SignalServiceKit/Messages/Stories/SystemStoryManager.swift @@ -52,7 +52,6 @@ public class OnboardingStoryManagerStoryMessageFactory { ) async throws -> AttachmentDataSource { return try await DependenciesBridge.shared.attachmentContentValidator.validateContents( dataSource: dataSource, - shouldConsume: true, mimeType: mimeType, renderingFlag: .default, sourceFilename: nil @@ -548,7 +547,7 @@ public class SystemStoryManager: SystemStoryManagerProtocol { } let dataSource = DataSourcePath( fileUrl: resultUrl, - shouldDeleteOnDeallocation: CurrentAppContext().isRunningTests.negated + ownership: CurrentAppContext().isRunningTests ? .borrowed : .owned, ) return try await storyMessageFactory.validateAttachmentContents( dataSource: dataSource, diff --git a/SignalServiceKit/Util/DataSourcePath.swift b/SignalServiceKit/Util/DataSourcePath.swift index 9812806101..990081247d 100644 --- a/SignalServiceKit/Util/DataSourcePath.swift +++ b/SignalServiceKit/Util/DataSourcePath.swift @@ -6,21 +6,30 @@ import Foundation public class DataSourcePath { - public init(fileUrl: URL, shouldDeleteOnDeallocation: Bool) { - owsPrecondition(fileUrl.isFileURL) - self.fileUrl = fileUrl - self.shouldDeleteOnDeallocation = shouldDeleteOnDeallocation + public enum Ownership { + /// The `DataSourcePath` owns this URL and may consume it. + case owned + + /// The `DataSourcePath` is borrowing a reference to this file and must not + /// touch it. + case borrowed } - public convenience init(filePath: String, shouldDeleteOnDeallocation: Bool) { + public init(fileUrl: URL, ownership: Ownership) { + owsPrecondition(fileUrl.isFileURL) + self.fileUrl = fileUrl + self.ownership = ownership + } + + public convenience init(filePath: String, ownership: Ownership) { let fileUrl = URL(fileURLWithPath: filePath) - self.init(fileUrl: fileUrl, shouldDeleteOnDeallocation: shouldDeleteOnDeallocation) + self.init(fileUrl: fileUrl, ownership: ownership) } public convenience init(writingTempFileData: Data, fileExtension: String) throws { let fileUrl = OWSFileSystem.temporaryFileUrl(fileExtension: fileExtension, isAvailableWhileDeviceLocked: true) try writingTempFileData.write(to: fileUrl, options: .completeFileProtectionUntilFirstUserAuthentication) - self.init(fileUrl: fileUrl, shouldDeleteOnDeallocation: true) + self.init(fileUrl: fileUrl, ownership: .owned) } public convenience init(writingSyncMessageData: Data) throws { @@ -28,10 +37,9 @@ public class DataSourcePath { } deinit { - if shouldDeleteOnDeallocation && !isConsumed.get() { - // In the ObjC code this would fire into a dispatch queue + if ownership == .owned && !isConsumed.get() { do { - try FileManager.default.removeItem(at: fileUrl) + try OWSFileSystem.deleteFileIfExists(url: fileUrl) } catch { owsFailDebug("DataSourcePath could not delete file: \(fileUrl), \(error)") } @@ -39,7 +47,7 @@ public class DataSourcePath { } public let fileUrl: URL - private let shouldDeleteOnDeallocation: Bool + private let ownership: Ownership private let isConsumed = AtomicBool(false, lock: .init()) private var _sourceFilename: String? @@ -63,9 +71,11 @@ public class DataSourcePath { return UInt64(try fileUrl.resourceValues(forKeys: [.fileSizeKey]).fileSize!) } - public func consumeAndDelete() throws { + public func consumeAndDeleteIfNecessary() throws { owsAssertDebug(isConsumed.tryToSetFlag()) - try OWSFileSystem.deleteFileIfExists(url: fileUrl) + if ownership == .owned { + try OWSFileSystem.deleteFileIfExists(url: fileUrl) + } } public func imageSource() throws -> any OWSImageSource { diff --git a/SignalUI/AttachmentApproval/AttachmentApprovalViewController.swift b/SignalUI/AttachmentApproval/AttachmentApprovalViewController.swift index 01bc8dda64..2e8a326ad2 100644 --- a/SignalUI/AttachmentApproval/AttachmentApprovalViewController.swift +++ b/SignalUI/AttachmentApproval/AttachmentApprovalViewController.swift @@ -766,7 +766,7 @@ public final class AttachmentApprovalViewController: UIPageViewController, UIPag guard let dataUTI = MimeTypeUtil.utiTypeForFileExtension(fileExtension) else { throw OWSAssertionError("Missing dataUTI.") } - let dataSource = DataSourcePath(fileUrl: fileUrl, shouldDeleteOnDeallocation: true) + let dataSource = DataSourcePath(fileUrl: fileUrl, ownership: .owned) // Rewrite the filename's extension to reflect the output file format. var filename: String? = attachmentApprovalItem.attachment.rawValue.dataSource.sourceFilename?.filterFilename() if let sourceFilename = attachmentApprovalItem.attachment.rawValue.dataSource.sourceFilename?.filterFilename() { diff --git a/SignalUI/AttachmentMultisend/AttachmentMultisend.swift b/SignalUI/AttachmentMultisend/AttachmentMultisend.swift index aa454ebd45..73f9648ccc 100644 --- a/SignalUI/AttachmentMultisend/AttachmentMultisend.swift +++ b/SignalUI/AttachmentMultisend/AttachmentMultisend.swift @@ -159,7 +159,6 @@ public class AttachmentMultisend { for attachment in sendableAttachments { let dataSource: AttachmentDataSource = try await deps.attachmentValidator.validateContents( sendableAttachment: attachment, - shouldConsume: true, shouldUseDefaultFilename: false, ) try results.append(.init( @@ -182,7 +181,6 @@ public class AttachmentMultisend { // doesn't segment. originalDataSource = try await deps.attachmentValidator.validateContents( sendableAttachment: segmentingResult.original, - shouldConsume: true, shouldUseDefaultFilename: false, ) } else { @@ -197,7 +195,6 @@ public class AttachmentMultisend { for segment in segments { let dataSource: AttachmentDataSource = try await deps.attachmentValidator.validateContents( sendableAttachment: segment, - shouldConsume: true, shouldUseDefaultFilename: false, ) segmentedDataSources.append(dataSource) diff --git a/SignalUI/Attachments/TypedItemProvider.swift b/SignalUI/Attachments/TypedItemProvider.swift index de9b34a494..c444f1938f 100644 --- a/SignalUI/Attachments/TypedItemProvider.swift +++ b/SignalUI/Attachments/TypedItemProvider.swift @@ -379,7 +379,7 @@ public struct TypedItemProvider { let copiedUrl = OWSFileSystem.temporaryFileUrl(fileExtension: fileUrl.pathExtension) try FileManager.default.copyItem(at: fileUrl, to: copiedUrl) - let dataSource = DataSourcePath(fileUrl: copiedUrl, shouldDeleteOnDeallocation: true) + let dataSource = DataSourcePath(fileUrl: copiedUrl, ownership: .owned) dataSource.sourceFilename = fileUrl.lastPathComponent let dataUTI = MimeTypeUtil.utiTypeForFileExtension(fileUrl.pathExtension) ?? defaultTypeIdentifier