From 221043a99851acc29c5e295c7b908b38f6468553 Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Wed, 10 Jun 2026 13:51:02 -0500 Subject: [PATCH] Compute mediaName dynamically --- Signal.xcodeproj/project.pbxproj | 20 ++++++ .../Attachments/BackupListMediaManager.swift | 8 +-- .../BackupListMediaManagerTests.swift | 52 ++++++++++++---- .../Messages/Attachments/V2/Attachment.swift | 16 ++--- .../AttachmentManagerImpl.swift | 12 +--- .../V2/AttachmentStore/AttachmentStore.swift | 50 --------------- .../AttachmentStoreTests.swift | 22 ++----- .../AttachmentContentValidator.swift | 5 +- .../AttachmentDownloadManagerImpl.swift | 2 - .../Attachments/V2/Mocks/MockAttachment.swift | 31 +++++----- .../V2/Records/AttachmentRecord.swift | 22 +++---- .../Storage/Database/GRDBSchemaMigrator.swift | 26 +++++++- .../Upload/AttachmentUploadManager.swift | 2 - .../OrphanedBackupAttachmentTest.swift | 62 +++++++++++++++++++ 14 files changed, 186 insertions(+), 144 deletions(-) create mode 100644 SignalServiceKit/tests/Backups/Attachments/OrphanedBackupAttachmentTest.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index fc59488c01..5d5bb12550 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -856,6 +856,7 @@ 50D839512F916A3700EE009A /* MessageRequestDecliner.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50D839502F916A3700EE009A /* MessageRequestDecliner.swift */; }; 50D8796A2A16D2C20031345D /* MessageLoaderBatchTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50D879692A16D2C20031345D /* MessageLoaderBatchTest.swift */; }; 50D9CD8D2C52D78000273D6C /* StoryRecipientManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50D9CD8C2C52D78000273D6C /* StoryRecipientManager.swift */; }; + 50DAF7E02FD87BEC00BE7430 /* OrphanedBackupAttachmentTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50DAF7DF2FD87BEC00BE7430 /* OrphanedBackupAttachmentTest.swift */; }; 50DCCBFA2F1817280024D124 /* DisappearingMessagesConfigurationMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50DCCBF92F1817280024D124 /* DisappearingMessagesConfigurationMessage.swift */; }; 50DCCBFC2F181A790024D124 /* ProfileKeyMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50DCCBFB2F181A790024D124 /* ProfileKeyMessage.swift */; }; 50DCCBFE2F1820600024D124 /* OutgoingSenderKeyDistributionMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50DCCBFD2F1820600024D124 /* OutgoingSenderKeyDistributionMessage.swift */; }; @@ -5121,6 +5122,7 @@ 50D839502F916A3700EE009A /* MessageRequestDecliner.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageRequestDecliner.swift; sourceTree = ""; }; 50D879692A16D2C20031345D /* MessageLoaderBatchTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageLoaderBatchTest.swift; sourceTree = ""; }; 50D9CD8C2C52D78000273D6C /* StoryRecipientManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoryRecipientManager.swift; sourceTree = ""; }; + 50DAF7DF2FD87BEC00BE7430 /* OrphanedBackupAttachmentTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OrphanedBackupAttachmentTest.swift; sourceTree = ""; }; 50DCCBF92F1817280024D124 /* DisappearingMessagesConfigurationMessage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DisappearingMessagesConfigurationMessage.swift; sourceTree = ""; }; 50DCCBFB2F181A790024D124 /* ProfileKeyMessage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProfileKeyMessage.swift; sourceTree = ""; }; 50DCCBFD2F1820600024D124 /* OutgoingSenderKeyDistributionMessage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OutgoingSenderKeyDistributionMessage.swift; sourceTree = ""; }; @@ -9898,6 +9900,22 @@ path = Debugging; sourceTree = ""; }; + 50DAF7E12FD87BFD00BE7430 /* Backups */ = { + isa = PBXGroup; + children = ( + 50DAF7E22FD87C7000BE7430 /* Attachments */, + ); + path = Backups; + sourceTree = ""; + }; + 50DAF7E22FD87C7000BE7430 /* Attachments */ = { + isa = PBXGroup; + children = ( + 50DAF7DF2FD87BEC00BE7430 /* OrphanedBackupAttachmentTest.swift */, + ); + path = Attachments; + sourceTree = ""; + }; 50E0198E2CC2491A0063EA48 /* Concurrency */ = { isa = PBXGroup; children = ( @@ -14633,6 +14651,7 @@ F94261FF289B1B5400460798 /* Account */, D92EFDED2F69B9D00031D257 /* AttachmentBackfill */, 50ED28002F0EDAFB00E57C54 /* Attachments */, + 50DAF7E12FD87BFD00BE7430 /* Backups */, F945FE4B298481D800C835C7 /* Calls */, D985D86229B91C2B0087C90C /* ChangePhoneNumber */, 50E0198E2CC2491A0063EA48 /* Concurrency */, @@ -20171,6 +20190,7 @@ D979CC3A2AD3964E006AAC49 /* Numbers+Random.swift in Sources */, D95E149D2E3D22FD00B5B70B /* ObjectRetainerTest.swift in Sources */, 663D02DF2C069AB600350632 /* OrphanedAttachmentCleanerTest.swift in Sources */, + 50DAF7E02FD87BEC00BE7430 /* OrphanedBackupAttachmentTest.swift in Sources */, D9AA37A02A86E0910088EFFB /* OutgoingCallEventSyncMessageTest.swift in Sources */, D925C7BB2B7BEC0F00AC73B0 /* OutgoingCallLogEventSyncMessageTest.swift in Sources */, D9D3216A2A8AC9B0004FC110 /* OutgoingGroupCallUpdateMessageTest.swift in Sources */, diff --git a/SignalServiceKit/Backups/Attachments/BackupListMediaManager.swift b/SignalServiceKit/Backups/Attachments/BackupListMediaManager.swift index eeac09e2fb..75c2c5481a 100644 --- a/SignalServiceKit/Backups/Attachments/BackupListMediaManager.swift +++ b/SignalServiceKit/Backups/Attachments/BackupListMediaManager.swift @@ -394,7 +394,7 @@ class BackupListMediaManagerImpl: BackupListMediaManager { var query = Attachment.Record .order(Column(Attachment.Record.CodingKeys.sqliteId).asc) - .filter(Column(Attachment.Record.CodingKeys.mediaName) != nil) + .filter(Column(Attachment.Record.CodingKeys.plaintextHash) != nil) if let id = txContext.lastEnumeratedAttachmentId { query = query @@ -761,7 +761,6 @@ class BackupListMediaManagerImpl: BackupListMediaManager { attachment, listedMedia: matchedListedMedia, isThumbnail: isThumbnail, - fullsizeMediaName: fullsizeMediaName, uploadEraAtStartOfListMedia: uploadEraAtStartOfListMedia, currentBackupPlan: currentBackupPlan, remoteConfig: remoteConfig, @@ -950,7 +949,6 @@ class BackupListMediaManagerImpl: BackupListMediaManager { _ attachment: Attachment, listedMedia: ListedBackupMediaObject, isThumbnail: Bool, - fullsizeMediaName: String, uploadEraAtStartOfListMedia: String, currentBackupPlan: BackupPlan, remoteConfig: RemoteConfig, @@ -962,7 +960,6 @@ class BackupListMediaManagerImpl: BackupListMediaManager { from: listedMedia, isThumbnail: isThumbnail, uploadEraAtStartOfListMedia: uploadEraAtStartOfListMedia, - fullsizeMediaName: fullsizeMediaName, tx: tx, ) @@ -1015,7 +1012,6 @@ class BackupListMediaManagerImpl: BackupListMediaManager { from listedMedia: ListedBackupMediaObject, isThumbnail: Bool, uploadEraAtStartOfListMedia: String, - fullsizeMediaName: String, tx: DBWriteTransaction, ) -> Bool { if isThumbnail { @@ -1027,7 +1023,6 @@ class BackupListMediaManagerImpl: BackupListMediaManager { uploadEra: uploadEraAtStartOfListMedia, lastDownloadAttemptTimestamp: nil, ), - mediaName: fullsizeMediaName, tx: tx, ) return true @@ -1068,7 +1063,6 @@ class BackupListMediaManagerImpl: BackupListMediaManager { uploadEra: uploadEraAtStartOfListMedia, lastDownloadAttemptTimestamp: nil, ), - mediaName: fullsizeMediaName, tx: tx, ) return true diff --git a/SignalServiceKit/Backups/Attachments/BackupListMediaManagerTests.swift b/SignalServiceKit/Backups/Attachments/BackupListMediaManagerTests.swift index bb43a7a6ea..04db4d185e 100644 --- a/SignalServiceKit/Backups/Attachments/BackupListMediaManagerTests.swift +++ b/SignalServiceKit/Backups/Attachments/BackupListMediaManagerTests.swift @@ -91,12 +91,14 @@ public class BackupListMediaManagerTests { // Case 1: Attachment exists locally but not on CDN let localOnlyIds = await db.awaitableWrite { tx in return (0.. Attachment.IDType { + owsPrecondition(mediaTierInfo == nil || mediaTierInfo!.plaintextHash == plaintextHash) + let thread = TSThread(uniqueId: UUID().uuidString) try! thread.insert(tx.database) - var attachmentRecord = Attachment.Record.mockStream(mediaName: mediaName) + var attachmentRecord = Attachment.Record.mockStream( + encryptionKey: encryptionKey, + plaintextHash: plaintextHash, + ) try! attachmentRecord.insert(tx.database) if let mediaTierInfo { diff --git a/SignalServiceKit/Messages/Attachments/V2/Attachment.swift b/SignalServiceKit/Messages/Attachments/V2/Attachment.swift index d192f00002..adc88e7e20 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Attachment.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Attachment.swift @@ -58,7 +58,11 @@ public class Attachment { /// MediaName used for backups (but assigned even if backups disabled). /// Nonnull if downloaded OR if restored from a backup. - public var mediaName: String? + public var mediaName: String? { + return self.plaintextHash.map { + return Attachment.mediaName(plaintextHash: $0, encryptionKey: self.encryptionKey) + } + } /// If null, the resource has not been uploaded to the media tier. public var mediaTierInfo: MediaTierInfo? @@ -146,7 +150,7 @@ public class Attachment { init(pendingAttachment: PendingAttachment) { self.init( plaintextHash: pendingAttachment.plaintextHash, - mediaName: pendingAttachment.mediaName, + encryptionKey: pendingAttachment.encryptionKey, encryptedByteCount: pendingAttachment.encryptedByteCount, unencryptedByteCount: pendingAttachment.unencryptedByteCount, cachedMediaSizePixels: pendingAttachment.mediaPixelSize, @@ -161,7 +165,7 @@ public class Attachment { init( plaintextHash: Data, - mediaName: String, + encryptionKey: Data, encryptedByteCount: UInt32, unencryptedByteCount: UInt32, cachedMediaSizePixels: CGSize?, @@ -173,7 +177,7 @@ public class Attachment { localRelativeFilePath: String, ) { self.plaintextHash = plaintextHash - self.mediaName = mediaName + self.mediaName = Attachment.mediaName(plaintextHash: plaintextHash, encryptionKey: encryptionKey) self.encryptedByteCount = encryptedByteCount self.unencryptedByteCount = unencryptedByteCount self.cachedMediaSizePixels = cachedMediaSizePixels @@ -279,13 +283,11 @@ public class Attachment { self.encryptionKey = record.encryptionKey self.originalAttachmentIdForQuotedReply = record.originalAttachmentIdForQuotedReply self.plaintextHash = record.plaintextHash - self.mediaName = record.mediaName self.localRelativeFilePathThumbnail = record.localRelativeFilePathThumbnail self.lastFullscreenViewTimestamp = record.lastFullscreenViewTimestamp if let plaintextHash = record.plaintextHash, - let mediaName = record.mediaName, let encryptedByteCount = record.encryptedByteCount, let unencryptedByteCount = record.unencryptedByteCount, let ciphertextDigest = record.ciphertextDigest, @@ -293,7 +295,7 @@ public class Attachment { { self.streamInfo = StreamInfo( plaintextHash: plaintextHash, - mediaName: mediaName, + encryptionKey: self.encryptionKey, encryptedByteCount: encryptedByteCount, unencryptedByteCount: unencryptedByteCount, cachedMediaSizePixels: { diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift index 84ee0e24d5..c60d24eea4 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift @@ -463,8 +463,7 @@ public class AttachmentManagerImpl: AttachmentManager { // duplicate to the first attachment), but this does drop cdn info if, // for example, this copy had valid cdn info and the older one did not. // It is on the exporter to dedupe and merge as needed so this doesn't happen. - fallthrough - case .duplicateMediaName(let existingAttachmentId): + // We already have an attachment with the same mediaName (likely from this same // backup). Just point the reference at the existing attachment. attachmentStore.addReference( @@ -606,7 +605,6 @@ public class AttachmentManagerImpl: AttachmentManager { encryptionKey: pendingAttachment.encryptionKey, streamInfo: streamInfo, plaintextHash: pendingAttachment.plaintextHash, - mediaName: pendingAttachment.mediaName, ) let hasOrphanRecord = orphanedAttachmentStore.orphanAttachmentExists( @@ -729,11 +727,7 @@ public class AttachmentManagerImpl: AttachmentManager { switch error { case .duplicatePlaintextHash(let id): existingAttachmentId = id - case .duplicateMediaName(let id): - owsFailDebug("How did we match mediaName when using a random encryption key?") - existingAttachmentId = id } - // Already have an attachment with the same plaintext hash or media name! // Move all existing references to that copy, instead. // Doing so should delete the original attachment pointer. @@ -799,9 +793,7 @@ public class AttachmentManagerImpl: AttachmentManager { ) throws -> Attachment.IDType { let existingAttachmentId: Attachment.IDType switch error { - case - .duplicatePlaintextHash(let id), - .duplicateMediaName(let id): + case .duplicatePlaintextHash(let id): existingAttachmentId = id } diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStore.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStore.swift index dba2619726..97132b1e64 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStore.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStore.swift @@ -10,10 +10,6 @@ public enum AttachmentInsertError: Error { /// attachment a duplicate. Callers should instead create a new owner reference to /// the same existing attachment. case duplicatePlaintextHash(existingAttachmentId: Attachment.IDType) - /// An existing attachment was found with the same media name, making the new - /// attachment a duplicate. Callers should instead create a new owner reference to - /// the same existing attachment and possibly update it with any stream info. - case duplicateMediaName(existingAttachmentId: Attachment.IDType) } // MARK: - @@ -192,20 +188,6 @@ public struct AttachmentStore { } } - /// Fetch an existing Attachment record with the given mediaName. There will - /// be at most one. - public func fetchAttachmentRecord( - mediaName: String, - tx: DBReadTransaction, - ) -> Attachment.Record? { - let query = Attachment.Record - .filter(Column(Attachment.Record.CodingKeys.mediaName) == mediaName) - - return failIfThrows { - try query.fetchOne(tx.database) - } - } - // MARK: - /// Fetch an arbitrary referenced attachment for the provided owner. @@ -629,20 +611,6 @@ public struct AttachmentStore { throw AttachmentInsertError.duplicatePlaintextHash(existingAttachmentId: existingAttachmentId) } - // Find if there is already an attachment with the same media name. - if - let existingAttachmentId = fetchAttachmentRecord( - mediaName: Attachment.mediaName( - plaintextHash: streamInfo.plaintextHash, - encryptionKey: attachment.encryptionKey, - ), - tx: tx, - )?.sqliteId, - existingAttachmentId != attachment.id - { - throw AttachmentInsertError.duplicateMediaName(existingAttachmentId: existingAttachmentId) - } - // We count it as a "view" if the download was initiated by the user let lastFullscreenViewTimestamp: UInt64? switch priority { @@ -682,13 +650,11 @@ public struct AttachmentStore { attachment.streamInfo = streamInfo attachment.plaintextHash = streamInfo.plaintextHash attachment.latestTransitTierInfo = latestTransitTierInfo - attachment.mediaName = streamInfo.mediaName attachment.lastFullscreenViewTimestamp = lastFullscreenViewTimestamp ?? attachment.lastFullscreenViewTimestamp case .mediaTierFullsize: attachment.streamInfo = streamInfo attachment.plaintextHash = streamInfo.plaintextHash attachment.latestTransitTierInfo = latestTransitTierInfo - attachment.mediaName = streamInfo.mediaName if var mediaTierInfo = attachment.mediaTierInfo { // Wipe the last download attempt time; its now succeeded. mediaTierInfo.lastDownloadAttemptTimestamp = nil @@ -735,7 +701,6 @@ public struct AttachmentStore { attachment.latestTransitTierInfo = latestTransitTierInfo attachment.originalTransitTierInfo = originalTransitTierInfo attachment.plaintextHash = streamInfo.plaintextHash - attachment.mediaName = streamInfo.mediaName attachment.mediaTierInfo = mediaTierInfo attachment.thumbnailMediaTierInfo = thumbnailMediaTierInfo attachment.localRelativeFilePathThumbnail = nil @@ -824,11 +789,9 @@ public struct AttachmentStore { public func saveMediaTierInfo( attachment: Attachment, mediaTierInfo: Attachment.MediaTierInfo, - mediaName: String, tx: DBWriteTransaction, ) { attachment.mediaTierInfo = mediaTierInfo - attachment.mediaName = mediaName let record = Attachment.Record(attachment: attachment) failIfThrows { @@ -839,10 +802,8 @@ public struct AttachmentStore { func saveMediaTierThumbnailInfo( attachment: Attachment, thumbnailMediaTierInfo: Attachment.ThumbnailMediaTierInfo, - mediaName: String, tx: DBWriteTransaction, ) { - attachment.mediaName = mediaName attachment.thumbnailMediaTierInfo = thumbnailMediaTierInfo let record = Attachment.Record(attachment: attachment) @@ -1081,17 +1042,6 @@ public struct AttachmentStore { throw AttachmentInsertError.duplicatePlaintextHash(existingAttachmentId: existingAttachmentId) } - // Find if there is already an attachment with the same media name. - if - let mediaName = attachmentRecord.mediaName, - let existingAttachmentId = fetchAttachmentRecord( - mediaName: mediaName, - tx: tx, - )?.sqliteId - { - throw AttachmentInsertError.duplicateMediaName(existingAttachmentId: existingAttachmentId) - } - let attachment = failIfThrows { // Note that there are UNIQUE constraints on this table (e.g., // plaintext hash and mediaName). Importantly, those are checked diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift index 2fcc7a65b5..17ff60e850 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift @@ -144,30 +144,16 @@ class AttachmentStoreTests: XCTestCase { // MARK: - - func testInsertSameMediaName() { - let mediaName = Data(repeating: 27, count: 10).hexadecimalString - - switch testAttachmentInsertError( - attachmentParams1: Attachment.Record.mockStream(streamInfo: .mock(mediaName: mediaName)), - attachmentParams2: Attachment.Record.mockStream(streamInfo: .mock(mediaName: mediaName)), - ) { - case .duplicateMediaName: - break - case nil, .duplicatePlaintextHash: - XCTFail() - } - } - func testInsertSamePlaintextHash() throws { - let plaintextHash = UUID().data + let plaintextHash = Randomness.generateRandomBytes(32) switch testAttachmentInsertError( - attachmentParams1: Attachment.Record.mockStream(streamInfo: .mock(plaintextHash: plaintextHash)), - attachmentParams2: Attachment.Record.mockStream(streamInfo: .mock(plaintextHash: plaintextHash)), + attachmentParams1: Attachment.Record.mockStream(plaintextHash: plaintextHash), + attachmentParams2: Attachment.Record.mockStream(plaintextHash: plaintextHash), ) { case .duplicatePlaintextHash: break - case nil, .duplicateMediaName: + case nil: XCTFail() } } diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift index 355db675d8..a52017787c 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift @@ -47,10 +47,7 @@ public struct PendingAttachment { } var mediaName: String { - Attachment.mediaName( - plaintextHash: plaintextHash, - encryptionKey: encryptionKey, - ) + return Attachment.mediaName(plaintextHash: self.plaintextHash, encryptionKey: self.encryptionKey) } mutating func removeBorderlessRenderingFlagIfPresent() { diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift index 171d98ca38..3ba38074e1 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift @@ -2415,7 +2415,6 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { encryptionKey: pendingAttachment.encryptionKey, streamInfo: streamInfo, plaintextHash: pendingAttachment.plaintextHash, - mediaName: pendingAttachment.mediaName, ) let attachment = try self.attachmentStore.insert( @@ -2589,7 +2588,6 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { encryptionKey: pendingThumbnailAttachment.encryptionKey, streamInfo: streamInfo, plaintextHash: pendingThumbnailAttachment.plaintextHash, - mediaName: pendingThumbnailAttachment.mediaName, ) let newAttachment = try self.attachmentStore.insert( diff --git a/SignalServiceKit/Messages/Attachments/V2/Mocks/MockAttachment.swift b/SignalServiceKit/Messages/Attachments/V2/Mocks/MockAttachment.swift index ece64574e8..6493db0ca9 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Mocks/MockAttachment.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Mocks/MockAttachment.swift @@ -10,9 +10,9 @@ import Foundation // MARK: - Infos extension Attachment.StreamInfo { - public static func mock( + static func mock( plaintextHash: Data = Randomness.generateRandomBytes(32), - mediaName: String = UUID().uuidString, + encryptionKey: AttachmentKey = .generate(), encryptedByteCount: UInt32 = .random(in: 0..<95_000_000), unencryptedByteCount: UInt32 = .random(in: 0..<95_000_000), ciphertextDigest: Data = Randomness.generateRandomBytes(32), @@ -20,7 +20,7 @@ extension Attachment.StreamInfo { ) -> Attachment.StreamInfo { return Attachment.StreamInfo( plaintextHash: plaintextHash, - mediaName: mediaName, + encryptionKey: encryptionKey.combinedKey, encryptedByteCount: encryptedByteCount, unencryptedByteCount: unencryptedByteCount, cachedMediaSizePixels: nil, @@ -109,22 +109,23 @@ extension Attachment.Record { ) } - public static func mockStream( + static func mockStream( blurHash: String? = UUID().uuidString, mimeType: String = MimeType.imageJpeg.rawValue, - encryptionKey: Data = Randomness.generateRandomBytes(64), - plaintextHash: Data? = nil, - mediaName: String? = nil, - streamInfo: Attachment.StreamInfo = .mock(), + encryptionKey: AttachmentKey = .generate(), + plaintextHash: Data = Randomness.generateRandomBytes(32), + streamInfo: Attachment.StreamInfo? = nil, ) -> Attachment.Record { return .forInsertingStream( blurHash: blurHash, mimeType: mimeType, contentType: Attachment.ContentType(mimeType: mimeType), - encryptionKey: encryptionKey, - streamInfo: streamInfo, - plaintextHash: plaintextHash ?? streamInfo.plaintextHash, - mediaName: mediaName ?? streamInfo.mediaName, + encryptionKey: encryptionKey.combinedKey, + streamInfo: streamInfo ?? .mock( + plaintextHash: plaintextHash, + encryptionKey: encryptionKey, + ), + plaintextHash: plaintextHash, ) } } @@ -136,7 +137,6 @@ extension Attachment { mimeType: String = MimeType.applicationOctetStream.rawValue, encryptionKey: Data = Randomness.generateRandomBytes(64), plaintextHash: Data? = nil, - mediaName: String? = nil, streamInfo: Attachment.StreamInfo? = nil, transitTierInfo: Attachment.TransitTierInfo? = nil, mediaTierInfo: Attachment.MediaTierInfo? = nil, @@ -152,7 +152,6 @@ extension Attachment { contentType: Attachment.ContentType(mimeType: mimeType), encryptionKey: encryptionKey, plaintextHash: plaintextHash ?? streamInfo?.plaintextHash, - mediaName: mediaName ?? streamInfo?.mediaName, localRelativeFilePathThumbnail: localRelativeFilePathThumbnail, streamInfo: streamInfo, latestTransitTierInfo: transitTierInfo, @@ -169,10 +168,9 @@ extension Attachment { extension AttachmentStream { - public static func mock( + static func mock( blurHash: String? = nil, mimeType: String = MimeType.applicationOctetStream.rawValue, - mediaName: String? = nil, streamInfo: Attachment.StreamInfo = .mock(), transitTierInfo: Attachment.TransitTierInfo? = nil, mediaTierInfo: Attachment.MediaTierInfo? = nil, @@ -182,7 +180,6 @@ extension AttachmentStream { let attachment = Attachment.mock( blurHash: blurHash, mimeType: mimeType, - mediaName: mediaName, streamInfo: streamInfo, transitTierInfo: transitTierInfo, mediaTierInfo: mediaTierInfo, diff --git a/SignalServiceKit/Messages/Attachments/V2/Records/AttachmentRecord.swift b/SignalServiceKit/Messages/Attachments/V2/Records/AttachmentRecord.swift index 91424f03ff..f1616b7878 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Records/AttachmentRecord.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Records/AttachmentRecord.swift @@ -32,7 +32,7 @@ extension Attachment { let latestTransitCiphertextDigest: Data? @DBUInt64Optional var latestTransitLastDownloadAttemptTimestamp: UInt64? - let mediaName: String? + private let _mediaName: String? // deprecated, always NULL let mediaTierCdnNumber: UInt32? let mediaTierUnencryptedByteCount: UInt32? let mediaTierUploadEra: String? @@ -66,6 +66,12 @@ extension Attachment { let originalTransitTierIncrementalMac: Data? let originalTransitTierIncrementalMacChunkSize: UInt32? + var mediaName: String? { + return self.plaintextHash.map { + return Attachment.mediaName(plaintextHash: $0, encryptionKey: self.encryptionKey) + } + } + public var allFilesRelativePaths: [String] { return [ localRelativeFilePath, @@ -94,7 +100,7 @@ extension Attachment { case latestTransitUnencryptedByteCount = "transitUnencryptedByteCount" case latestTransitCiphertextDigest = "transitDigestSHA256Ciphertext" case latestTransitLastDownloadAttemptTimestamp = "lastTransitDownloadAttemptTimestamp" - case mediaName + case _mediaName = "mediaName" case mediaTierCdnNumber case mediaTierUnencryptedByteCount case mediaTierUploadEra @@ -143,7 +149,6 @@ extension Attachment { contentType: attachment.contentType, encryptionKey: attachment.encryptionKey, plaintextHash: attachment.plaintextHash, - mediaName: attachment.mediaName, localRelativeFilePathThumbnail: attachment.localRelativeFilePathThumbnail, streamInfo: attachment.streamInfo, latestTransitTierInfo: attachment.latestTransitTierInfo, @@ -162,7 +167,6 @@ extension Attachment { contentType: Attachment.ContentType, encryptionKey: Data, plaintextHash: Data?, - mediaName: String?, localRelativeFilePathThumbnail: String?, streamInfo: Attachment.StreamInfo?, latestTransitTierInfo: Attachment.TransitTierInfo?, @@ -209,7 +213,7 @@ extension Attachment { self.originalTransitTierIncrementalMac = originalTransitTierInfo?.incrementalMacInfo?.mac self.originalTransitTierIncrementalMacChunkSize = originalTransitTierInfo?.incrementalMacInfo?.chunkSize - self.mediaName = mediaName + self._mediaName = nil self.mediaTierCdnNumber = mediaTierInfo?.cdnNumber self.mediaTierUnencryptedByteCount = mediaTierInfo?.unencryptedByteCount self.mediaTierIncrementalMac = mediaTierInfo?.incrementalMacInfo?.mac @@ -248,7 +252,6 @@ extension Attachment { contentType: contentType, encryptionKey: encryptionKey, plaintextHash: nil, - mediaName: nil, localRelativeFilePathThumbnail: nil, streamInfo: nil, latestTransitTierInfo: latestTransitTierInfo, @@ -268,7 +271,6 @@ extension Attachment { encryptionKey: Data, streamInfo: Attachment.StreamInfo, plaintextHash: Data, - mediaName: String, ) -> Record { return Record( sqliteId: nil, @@ -277,7 +279,6 @@ extension Attachment { contentType: contentType, encryptionKey: encryptionKey, plaintextHash: plaintextHash, - mediaName: mediaName, localRelativeFilePathThumbnail: nil, streamInfo: streamInfo, latestTransitTierInfo: nil, @@ -306,9 +307,6 @@ extension Attachment { contentType: contentType, encryptionKey: encryptionKey, plaintextHash: plaintextHash, - mediaName: plaintextHash.map { - return Attachment.mediaName(plaintextHash: $0, encryptionKey: encryptionKey) - }, localRelativeFilePathThumbnail: nil, streamInfo: nil, latestTransitTierInfo: latestTransitTierInfo, @@ -335,7 +333,6 @@ extension Attachment { // encryption key we use is irrelevant. Just generate a new one. encryptionKey: AttachmentKey.generate().combinedKey, plaintextHash: nil, - mediaName: nil, localRelativeFilePathThumbnail: nil, streamInfo: nil, latestTransitTierInfo: nil, @@ -362,7 +359,6 @@ extension Attachment { contentType: thumbnailContentType, encryptionKey: thumbnailEncryptionKey, plaintextHash: nil, - mediaName: nil, localRelativeFilePathThumbnail: nil, streamInfo: nil, latestTransitTierInfo: thumbnailTransitTierInfo, diff --git a/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift b/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift index 46d9a8655b..3df0ddb67d 100644 --- a/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift +++ b/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift @@ -334,6 +334,7 @@ public class GRDBSchemaMigrator { case purgeMyStoryDeletedAtTimestamp case addRecoverablePlaceholderExpirationIndex case backfillRecoverablePlaceholderErrorType + case clearAttachmentMediaName // NOTE: Every time we add a migration id, consider // incrementing grdbSchemaVersionLatest. @@ -457,7 +458,7 @@ public class GRDBSchemaMigrator { } public static let grdbSchemaVersionDefault: UInt = 0 - public static let grdbSchemaVersionLatest: UInt = 144 + public static let grdbSchemaVersionLatest: UInt = 145 private class DatabaseMigratorWrapper { // Run with immediate (or disabled) foreign key checks so that pre-existing @@ -5220,6 +5221,29 @@ public class GRDBSchemaMigrator { return .success(()) } + migrator.registerMigration(.clearAttachmentMediaName) { tx in + try tx.database.execute(sql: "DROP TRIGGER IF EXISTS __Attachment_ad_backup_fullsize") + try tx.database.execute(sql: "DROP TRIGGER IF EXISTS __Attachment_ad_backup_thumbnail") + try tx.database.execute(sql: "UPDATE Attachment SET mediaName = NULL") + try tx.database.execute(sql: """ + CREATE TRIGGER "__Attachment_ad_backup_fullsize" AFTER DELETE ON "Attachment" + WHEN (OLD.mediaTierCdnNumber IS NOT NULL AND OLD.sha256ContentHash IS NOT NULL) + BEGIN + INSERT INTO OrphanedBackupAttachment (cdnNumber, mediaName, mediaId, type) + VALUES (OLD.mediaTierCdnNumber, LOWER(HEX(OLD.sha256ContentHash || OLD.encryptionKey)), NULL, 0); + END + """) + try tx.database.execute(sql: """ + CREATE TRIGGER "__Attachment_ad_backup_thumbnail" AFTER DELETE ON "Attachment" + WHEN (OLD.thumbnailCdnNumber IS NOT NULL AND OLD.sha256ContentHash IS NOT NULL) + BEGIN + INSERT INTO OrphanedBackupAttachment (cdnNumber, mediaName, mediaId, type) + VALUES (OLD.thumbnailCdnNumber, LOWER(HEX(OLD.sha256ContentHash || OLD.encryptionKey)), NULL, 1); + END; + """) + return .success(()) + } + // MARK: - Schema Migration Insertion Point } diff --git a/SignalServiceKit/Upload/AttachmentUploadManager.swift b/SignalServiceKit/Upload/AttachmentUploadManager.swift index cf7a4cb85f..f7785a77cf 100644 --- a/SignalServiceKit/Upload/AttachmentUploadManager.swift +++ b/SignalServiceKit/Upload/AttachmentUploadManager.swift @@ -439,7 +439,6 @@ public actor AttachmentUploadManagerImpl: AttachmentUploadManager { attachmentStore.saveMediaTierInfo( attachment: attachmentStream.attachment, mediaTierInfo: mediaTierInfo, - mediaName: attachmentStream.mediaName, tx: tx, ) @@ -553,7 +552,6 @@ public actor AttachmentUploadManagerImpl: AttachmentUploadManager { attachmentStore.saveMediaTierThumbnailInfo( attachment: attachmentStream.attachment, thumbnailMediaTierInfo: thumbnailInfo, - mediaName: attachmentStream.mediaName, tx: tx, ) diff --git a/SignalServiceKit/tests/Backups/Attachments/OrphanedBackupAttachmentTest.swift b/SignalServiceKit/tests/Backups/Attachments/OrphanedBackupAttachmentTest.swift new file mode 100644 index 0000000000..4d0659ad62 --- /dev/null +++ b/SignalServiceKit/tests/Backups/Attachments/OrphanedBackupAttachmentTest.swift @@ -0,0 +1,62 @@ +// +// Copyright 2026 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only +// + +import Foundation +import Testing +@testable import SignalServiceKit + +struct OrphanedBackupAttachmentTest { + @Test + func testTriggers() throws { + let db = InMemoryDB() + + let encryptionKey = AttachmentKey.generate() + let plaintextHash = Randomness.generateRandomBytes(32) + var record = Attachment.Record.forInsertingFromBackup( + blurHash: nil, + mimeType: "image/png", + contentType: .image, + encryptionKey: encryptionKey.combinedKey, + latestTransitTierInfo: nil, + plaintextHash: plaintextHash, + mediaTierInfo: Attachment.MediaTierInfo( + cdnNumber: 2, + unencryptedByteCount: 123, + plaintextHash: plaintextHash, + incrementalMacInfo: nil, + uploadEra: "initialUploadEra", + ), + thumbnailMediaTierInfo: Attachment.ThumbnailMediaTierInfo( + cdnNumber: 5, + uploadEra: "initialUploadEra", + ), + ) + + try db.write { tx in + try record.insert(tx.database) + try record.delete(tx.database) + } + + let orphanedAttachments = try db.read { tx in + return try OrphanedBackupAttachment.fetchAll(tx.database) + } + + struct OrphanedBackupAttachment2: Hashable { + var mediaName: String? + var cdnNumber: UInt32 + var type: Int? + } + + let actualValues = orphanedAttachments.map { + return OrphanedBackupAttachment2(mediaName: $0.mediaName, cdnNumber: $0.cdnNumber, type: $0.type?.rawValue) + } + let mediaName = (plaintextHash + encryptionKey.combinedKey).hexadecimalString + let expectedValues = [ + OrphanedBackupAttachment2(mediaName: mediaName, cdnNumber: 2, type: OrphanedBackupAttachment.SizeType.fullsize.rawValue), + OrphanedBackupAttachment2(mediaName: mediaName, cdnNumber: 5, type: OrphanedBackupAttachment.SizeType.thumbnail.rawValue), + ] + #expect(Set(actualValues) == Set(expectedValues)) + } +}