From 0fb3e9b50ef397fc66eec882ff9c86a298e9f91b Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Mon, 11 May 2026 12:51:27 -0500 Subject: [PATCH] Allow empty files in transit tier info --- .../CVComponentGenericAttachment.swift | 4 +- ...ckupArchiveMessageAttachmentArchiver.swift | 2 +- .../Jobs/IncomingContactSyncJobQueue.swift | 2 +- .../IncomingContactSyncJobRecord.swift | 2 +- .../Messages/Attachments/V2/Attachment.swift | 9 ++-- .../AttachmentManagerImpl.swift | 48 ++++++------------- .../Attachments/V2/AttachmentPointer.swift | 3 +- .../ReferencedAttachment.swift | 9 +--- .../V2/AttachmentTransitPointer.swift | 2 +- .../AttachmentContentValidator.swift | 2 +- .../AttachmentContentValidatorImpl.swift | 11 ++--- .../AttachmentContentValidatorMock.swift | 2 +- .../AttachmentDownloadManagerImpl.swift | 11 ++--- 13 files changed, 36 insertions(+), 71 deletions(-) diff --git a/Signal/ConversationView/Components/CVComponentGenericAttachment.swift b/Signal/ConversationView/Components/CVComponentGenericAttachment.swift index 3e36d9e665..13e1cc8729 100644 --- a/Signal/ConversationView/Components/CVComponentGenericAttachment.swift +++ b/Signal/ConversationView/Components/CVComponentGenericAttachment.swift @@ -179,8 +179,8 @@ public class CVComponentGenericAttachment: CVComponentBase, CVComponent { if let attachmentPointer = genericAttachment.attachmentPointer { var textComponents = [String]() - if let byteCount = attachmentPointer.unencryptedByteCount, byteCount > 0 { - textComponents.append(OWSFormat.localizedFileSizeString(from: UInt64(safeCast: byteCount))) + if attachmentPointer.unencryptedByteCount > 0 { + textComponents.append(OWSFormat.localizedFileSizeString(from: UInt64(safeCast: attachmentPointer.unencryptedByteCount))) } switch genericAttachment.attachment { diff --git a/SignalServiceKit/Backups/Archiving/Archivers/ChatItem/BackupArchiveMessageAttachmentArchiver.swift b/SignalServiceKit/Backups/Archiving/Archivers/ChatItem/BackupArchiveMessageAttachmentArchiver.swift index d558b25f65..f3994d1d9b 100644 --- a/SignalServiceKit/Backups/Archiving/Archivers/ChatItem/BackupArchiveMessageAttachmentArchiver.swift +++ b/SignalServiceKit/Backups/Archiving/Archivers/ChatItem/BackupArchiveMessageAttachmentArchiver.swift @@ -513,7 +513,7 @@ extension ReferencedAttachment { locatorInfo.integrityCheck = .plaintextHash(streamInfo.plaintextHash) } else if let transitTierInfoToExport { locatorInfo.key = attachment.encryptionKey - locatorInfo.size = transitTierInfoToExport.unencryptedByteCount ?? 0 + locatorInfo.size = transitTierInfoToExport.unencryptedByteCount // At the time of writing, TransitTierInfo.integrityCheck prefers // the encrypted digest even if both are present. (See comment in diff --git a/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift b/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift index d9bdf5c691..9a1c88e387 100644 --- a/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift +++ b/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift @@ -37,7 +37,7 @@ public class IncomingContactSyncJobQueue { cdnKey: String, attachmentKey: AttachmentKey, digest: Data, - plaintextLength: UInt32?, + plaintextLength: UInt32, isComplete: Bool, tx: DBWriteTransaction, ) { diff --git a/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift b/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift index 98ef4fd828..599c4e9959 100644 --- a/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift +++ b/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift @@ -62,7 +62,7 @@ public final class IncomingContactSyncJobRecord: JobRecord { cdnKey: String, attachmentKey: AttachmentKey, digest: Data, - plaintextLength: UInt32?, + plaintextLength: UInt32, isCompleteContactSync: Bool, failureCount: UInt = 0, status: Status = .ready, diff --git a/SignalServiceKit/Messages/Attachments/V2/Attachment.swift b/SignalServiceKit/Messages/Attachments/V2/Attachment.swift index 8104cfea24..144e696dc7 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Attachment.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Attachment.swift @@ -204,7 +204,7 @@ public class Attachment { /// Expected byte count after decrypting the resource off the transit tier (and removing padding). /// Provided by the sender of incoming attachments. - public let unencryptedByteCount: UInt32? + public let unencryptedByteCount: UInt32 /// Generated locally for outgoing attachments. /// For incoming attachments, taken off the service proto. If validation fails, the download is rejected. @@ -458,9 +458,7 @@ public class Attachment { cdnNumber: latestTransitTierInfo.cdnNumber, key: latestTransitTierInfo.encryptionKey, digest: digest, - // Okay to fall back to our local data length even if the original sender - // didn't include it; we now know it from the local file. - plaintextDataLength: latestTransitTierInfo.unencryptedByteCount ?? metadata.plaintextDataLength, + plaintextDataLength: latestTransitTierInfo.unencryptedByteCount, // Encrypted length is the same regardless of the key used. encryptedDataLength: metadata.encryptedDataLength, ), @@ -536,7 +534,8 @@ private extension Attachment.TransitTierInfo { self.uploadTimestamp = uploadTimestamp self.lastDownloadAttemptTimestamp = lastDownloadAttemptTimestamp self.encryptionKey = encryptionKey - self.unencryptedByteCount = unencryptedByteCount + // Old clients may have incorrectly persisted `nil` when `0` was provided. + self.unencryptedByteCount = unencryptedByteCount ?? 0 self.integrityCheck = integrityCheck if let incrementalMac, let incrementalMacChunkSize { self.incrementalMacInfo = .init(mac: incrementalMac, chunkSize: incrementalMacChunkSize) diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift index bff26260d9..7849b33586 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentManager/AttachmentManagerImpl.swift @@ -279,7 +279,7 @@ public class AttachmentManagerImpl: AttachmentManager { throw OWSAssertionError("Missing digest") } - return .init( + return Attachment.TransitTierInfo( cdnNumber: cdnNumber, cdnKey: cdnKey, uploadTimestamp: proto.uploadTimestamp, @@ -341,7 +341,6 @@ public class AttachmentManagerImpl: AttachmentManager { let contentType = Attachment.ContentType(mimeType: mimeType) var attachmentRecord: Attachment.Record - let sourceUnencryptedByteCount: UInt32? if proto.hasLocatorInfo, @@ -353,12 +352,6 @@ public class AttachmentManagerImpl: AttachmentManager { incrementalMacInfo: incrementalMacInfo, ) - if proto.locatorInfo.size > 0 { - sourceUnencryptedByteCount = proto.locatorInfo.size - } else { - sourceUnencryptedByteCount = nil - } - switch proto.locatorInfo.integrityCheck { case .plaintextHash(let plaintextHash): if plaintextHash.isEmpty { @@ -415,7 +408,6 @@ public class AttachmentManagerImpl: AttachmentManager { mimeType: mimeType, contentType: contentType, ) - sourceUnencryptedByteCount = nil } let referenceParams: AttachmentReference.ConstructionParams @@ -429,7 +421,7 @@ public class AttachmentManagerImpl: AttachmentManager { caption: proto.hasCaption ? proto.caption : nil, ), sourceFilename: sourceFilename, - sourceUnencryptedByteCount: sourceUnencryptedByteCount, + sourceUnencryptedByteCount: proto.locatorInfo.size, sourceMediaSizePixels: sourceMediaSizePixels, ) } @@ -441,24 +433,19 @@ public class AttachmentManagerImpl: AttachmentManager { tx: tx, ) - if let sourceUnencryptedByteCount { - let estimatedMediaTierSize: UInt64 - if - let size = Cryptography - .estimatedMediaTierCDNSize(unencryptedSize: UInt64(safeCast: sourceUnencryptedByteCount)) - { - estimatedMediaTierSize = size - } else { - Logger.warn("Failed to get estimated media tier size for attachment \(attachment.id)!") - estimatedMediaTierSize = UInt64(UInt32.max) - } - - attachmentByteCounter.addToByteCount( - attachmentID: attachment.id, - byteCount: estimatedMediaTierSize, - ) + let estimatedMediaTierSize: UInt64 + if let size = Cryptography.estimatedMediaTierCDNSize(unencryptedSize: UInt64(safeCast: proto.locatorInfo.size)) { + estimatedMediaTierSize = size + } else { + Logger.warn("Failed to get estimated media tier size for attachment \(attachment.id)!") + estimatedMediaTierSize = UInt64(UInt32.max) } + attachmentByteCounter.addToByteCount( + attachmentID: attachment.id, + byteCount: estimatedMediaTierSize, + ) + if let mediaName = attachmentRecord.mediaName { orphanedBackupAttachmentScheduler.didCreateOrUpdateAttachment( withMediaName: mediaName, @@ -504,13 +491,6 @@ public class AttachmentManagerImpl: AttachmentManager { return nil } - let unencryptedByteCount: UInt32? - if locatorInfo.size > 0 { - unencryptedByteCount = locatorInfo.size - } else { - unencryptedByteCount = nil - } - let uploadTimestampMs: UInt64 if locatorInfo.hasTransitTierUploadTimestamp, locatorInfo.transitTierUploadTimestamp > 0 { uploadTimestampMs = locatorInfo.transitTierUploadTimestamp @@ -545,7 +525,7 @@ public class AttachmentManagerImpl: AttachmentManager { cdnKey: locatorInfo.transitCdnKey, uploadTimestamp: uploadTimestampMs, encryptionKey: locatorInfo.key, - unencryptedByteCount: unencryptedByteCount, + unencryptedByteCount: locatorInfo.size, integrityCheck: integrityCheck, incrementalMacInfo: incrementalMacInfo, lastDownloadAttemptTimestamp: nil, diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentPointer.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentPointer.swift index 9f3e13bcf3..eeb21e70ac 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentPointer.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentPointer.swift @@ -51,13 +51,12 @@ public class AttachmentPointer { ].compacted().max() } - public var unencryptedByteCount: UInt32? { + public var unencryptedByteCount: UInt32 { switch source { case .mediaTier(let mediaTier): return mediaTier.unencryptedByteCount case .transitTier(let transitTier): return transitTier.unencryptedByteCount - ?? AttachmentBackupPointer(attachment: attachment)?.unencryptedByteCount } } diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentReference/ReferencedAttachment.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentReference/ReferencedAttachment.swift index 4b4197a0ea..da3a11296d 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentReference/ReferencedAttachment.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentReference/ReferencedAttachment.swift @@ -270,24 +270,19 @@ extension ReferencedAttachment { builder.setFlags(0) } - let unencryptedByteCount: UInt32? let pixelSize: CGSize? if let streamInfo = pointer.attachment.streamInfo { - unencryptedByteCount = streamInfo.unencryptedByteCount pixelSize = streamInfo.cachedMediaSizePixels } else { - unencryptedByteCount = reference.sourceUnencryptedByteCount pixelSize = reference.sourceMediaSizePixels } - - if let unencryptedByteCount { - builder.setSize(unencryptedByteCount) - } if let pixelSize { builder.setWidth(UInt32(pixelSize.width.rounded())) builder.setHeight(UInt32(pixelSize.height.rounded())) } + builder.setKey(pointer.info.encryptionKey) + builder.setSize(pointer.info.unencryptedByteCount) builder.setDigest(ciphertextDigest) builder.setUploadTimestamp(pointer.uploadTimestamp) diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentTransitPointer.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentTransitPointer.swift index 06ea1650b9..044d868927 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentTransitPointer.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentTransitPointer.swift @@ -19,7 +19,7 @@ public class AttachmentTransitPointer { public var cdnKey: String { info.cdnKey } public var uploadTimestamp: UInt64 { info.uploadTimestamp } public var lastDownloadAttemptTimestamp: UInt64? { info.lastDownloadAttemptTimestamp } - public var unencryptedByteCount: UInt32? { info.unencryptedByteCount } + public var unencryptedByteCount: UInt32 { info.unencryptedByteCount } private init( attachment: Attachment, diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift index d31dc4eea7..355db675d8 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift @@ -121,7 +121,7 @@ public protocol AttachmentContentValidator { func validateDownloadedContents( ofEncryptedFileAt fileUrl: URL, attachmentKey: AttachmentKey, - plaintextLength: UInt32?, + plaintextLength: UInt32, integrityCheck: AttachmentIntegrityCheck, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift index f8cb2a62b2..c4e1ea63d2 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift @@ -74,7 +74,7 @@ public class AttachmentContentValidatorImpl: AttachmentContentValidator { public func validateDownloadedContents( ofEncryptedFileAt fileUrl: URL, attachmentKey inputAttachmentKey: AttachmentKey, - plaintextLength: UInt32?, + plaintextLength: UInt32, integrityCheck: AttachmentIntegrityCheck, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, @@ -82,20 +82,15 @@ public class AttachmentContentValidatorImpl: AttachmentContentValidator { ) async throws -> PendingAttachment { // Very very first thing: validate the integrity check. // Throw if this fails. - var decryptedLength = 0 try Cryptography.decryptFile( at: fileUrl, metadata: DecryptionMetadata( key: inputAttachmentKey, integrityCheck: integrityCheck, - plaintextLength: plaintextLength.map(UInt64.init(safeCast:)), + plaintextLength: UInt64(safeCast: plaintextLength), ), - output: { data in - decryptedLength += data.count - }, + output: { _ in }, ) - let plaintextLength = plaintextLength ?? UInt32(decryptedLength) - let inputType = InputType.encryptedFile( fileUrl, inputAttachmentKey: inputAttachmentKey, diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift index 08e411f451..e7e788d404 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift @@ -32,7 +32,7 @@ open class AttachmentContentValidatorMock: AttachmentContentValidator { open func validateDownloadedContents( ofEncryptedFileAt fileUrl: URL, attachmentKey: AttachmentKey, - plaintextLength: UInt32?, + plaintextLength: UInt32, integrityCheck: AttachmentIntegrityCheck, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift index cad7b7d583..c2acc9e6eb 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift @@ -814,12 +814,9 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { plaintextLength: transitTierInfo.unencryptedByteCount, integrityCheck: transitTierInfo.integrityCheck, ) - downloadSizeSource = transitTierInfo.unencryptedByteCount.map({ - .estimatedSizeBytes(Cryptography.estimatedTransitTierCDNSize( - unencryptedSize: UInt64(safeCast: $0), - ) ?? { owsFail("can always produce estimate for 32-bit byte count") }()) - }) ?? .useHeadRequest - + downloadSizeSource = .estimatedSizeBytes(Cryptography.estimatedTransitTierCDNSize( + unencryptedSize: UInt64(safeCast: transitTierInfo.unencryptedByteCount), + ) ?? { owsFail("can always produce estimate for 32-bit byte count") }()) let attachmentLimits = IncomingAttachmentLimits.currentLimits(remoteConfig: remoteConfigProvider.currentConfig()) switch attachment.contentType { case .image: @@ -1997,7 +1994,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { case transitTier( mimeType: String, attachmentKey: AttachmentKey, - plaintextLength: UInt32?, + plaintextLength: UInt32, integrityCheck: AttachmentIntegrityCheck, )