From 53fda7574cde1864f7126ad34a303abe4c5b04b4 Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Sun, 20 Sep 2026 23:59:05 +0200 Subject: [PATCH] Fix a crash when syncing watch entries whose video ID exceeds the CloudKit record name limit Extracted media-source video IDs are full file paths, so closing a long-named WebDAV video built a watch record name longer than 255 characters and CKRecord.ID threw a fatal CKException. Scoped record names now fall back to a SHA-256 digest of the ID when the verbatim form would exceed the limit. The sync engine's local lookups for materializing pending saves and applying remote deletions recognise the digest form and match it against all entities of that type instead of the indexed ID lookup. Short IDs keep their existing names, so records already in iCloud are unaffected. --- Yattee/Data/DataManager+Bookmarks.swift | 6 ++ Yattee/Data/DataManager+WatchHistory.swift | 6 ++ .../CloudKit/CloudKitSyncEngine.swift | 25 +++++-- Yattee/Services/CloudKit/RecordTypes.swift | 50 ++++++++++++-- YatteeTests/CloudKitSyncTests.swift | 66 +++++++++++++++++++ 5 files changed, 141 insertions(+), 12 deletions(-) diff --git a/Yattee/Data/DataManager+Bookmarks.swift b/Yattee/Data/DataManager+Bookmarks.swift index f955f60d..074c0a90 100644 --- a/Yattee/Data/DataManager+Bookmarks.swift +++ b/Yattee/Data/DataManager+Bookmarks.swift @@ -130,6 +130,12 @@ extension DataManager { return (try? modelContext.fetch(descriptor)) ?? [] } + /// Gets every bookmark, for lookups that cannot go through the + /// video-ID index (CloudKit record names that carry a digest of the ID). + func allBookmarks() -> [Bookmark] { + (try? modelContext.fetch(FetchDescriptor())) ?? [] + } + /// Deletes a single bookmark without queueing a CloudKit deletion. /// Used by CloudKitSyncEngine when applying remote deletions. func deleteBookmark(_ bookmark: Bookmark) { diff --git a/Yattee/Data/DataManager+WatchHistory.swift b/Yattee/Data/DataManager+WatchHistory.swift index 16233536..05659c3c 100644 --- a/Yattee/Data/DataManager+WatchHistory.swift +++ b/Yattee/Data/DataManager+WatchHistory.swift @@ -167,6 +167,12 @@ extension DataManager { return (try? modelContext.fetch(descriptor)) ?? [] } + /// Gets every watch entry, for lookups that cannot go through the + /// video-ID index (CloudKit record names that carry a digest of the ID). + func allWatchEntries() -> [WatchEntry] { + (try? modelContext.fetch(FetchDescriptor())) ?? [] + } + /// Deletes a single watch entry without queueing a CloudKit deletion. /// Used by CloudKitSyncEngine when applying remote deletions. func deleteWatchEntry(_ entry: WatchEntry) { diff --git a/Yattee/Services/CloudKit/CloudKitSyncEngine.swift b/Yattee/Services/CloudKit/CloudKitSyncEngine.swift index 5aec4cab..a4980f15 100644 --- a/Yattee/Services/CloudKit/CloudKitSyncEngine.swift +++ b/Yattee/Services/CloudKit/CloudKitSyncEngine.swift @@ -2092,38 +2092,51 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { /// Bare-ID lookups can return an entity from a different source scope when /// IDs collide across sources; these helpers match on the mapper-derived /// record name so the right entity is merged, materialized, or deleted. + /// Entities whose ID corresponds to a bare ID parsed from a record name. + /// A verbatim bare ID is an indexed lookup; a digest (an ID too long to + /// embed in the record name) has to be matched against every entity. + private func candidates( + for bareID: String, + exact: (String) -> [Entity], + all: () -> [Entity], + id: (Entity) -> String + ) -> [Entity] { + guard SyncableRecordType.isHashedBareID(bareID) else { return exact(bareID) } + return all().filter { SyncableRecordType.entityID(id($0), matchesBareID: bareID) } + } + private func localSubscription(matching recordName: String, channelID: String, in dataManager: DataManager) -> Subscription? { - dataManager.subscriptions(forChannelID: channelID).first { + candidates(for: channelID, exact: dataManager.subscriptions(forChannelID:), all: dataManager.subscriptions, id: \.channelID).first { recordMapper.toCKRecord(subscription: $0).recordID.recordName == recordName } } private func localWatchEntry(matching recordName: String, videoID: String, in dataManager: DataManager) -> WatchEntry? { - dataManager.watchEntries(forVideoID: videoID).first { + candidates(for: videoID, exact: dataManager.watchEntries(forVideoID:), all: dataManager.allWatchEntries, id: \.videoID).first { recordMapper.toCKRecord(watchEntry: $0).recordID.recordName == recordName } } private func localBookmark(matching recordName: String, videoID: String, in dataManager: DataManager) -> Bookmark? { - dataManager.bookmarks(forVideoID: videoID).first { + candidates(for: videoID, exact: dataManager.bookmarks(forVideoID:), all: dataManager.allBookmarks, id: \.videoID).first { recordMapper.toCKRecord(bookmark: $0).recordID.recordName == recordName } } private func localRecentChannel(matching recordName: String, channelID: String, in dataManager: DataManager) -> RecentChannel? { - dataManager.recentChannelEntries(forChannelID: channelID).first { + candidates(for: channelID, exact: dataManager.recentChannelEntries(forChannelID:), all: dataManager.allRecentChannels, id: \.channelID).first { recordMapper.toCKRecord(recentChannel: $0).recordID.recordName == recordName } } private func localRecentPlaylist(matching recordName: String, playlistID: String, in dataManager: DataManager) -> RecentPlaylist? { - dataManager.recentPlaylistEntries(forPlaylistID: playlistID).first { + candidates(for: playlistID, exact: dataManager.recentPlaylistEntries(forPlaylistID:), all: dataManager.allRecentPlaylists, id: \.playlistID).first { recordMapper.toCKRecord(recentPlaylist: $0).recordID.recordName == recordName } } private func localChannelNotificationSettings(matching recordName: String, channelID: String, in dataManager: DataManager) -> ChannelNotificationSettings? { - dataManager.allChannelNotificationSettings(forChannelID: channelID).first { + candidates(for: channelID, exact: dataManager.allChannelNotificationSettings(forChannelID:), all: dataManager.allChannelNotificationSettings, id: \.channelID).first { recordMapper.toCKRecord(channelNotificationSettings: $0).recordID.recordName == recordName } } diff --git a/Yattee/Services/CloudKit/RecordTypes.swift b/Yattee/Services/CloudKit/RecordTypes.swift index 1907ea9c..0757e7da 100644 --- a/Yattee/Services/CloudKit/RecordTypes.swift +++ b/Yattee/Services/CloudKit/RecordTypes.swift @@ -6,6 +6,7 @@ // import CloudKit +import CryptoKit import Foundation /// CloudKit record type names and constants. @@ -131,6 +132,43 @@ enum SyncableRecordType: Sendable { } } + /// CloudKit rejects record names longer than this with a fatal + /// `CKException` at `CKRecord.ID` construction time. + static let maxRecordNameLength = 255 + + /// Marks a bare-ID component that is a digest of the real ID rather than + /// the ID itself. Extracted-source video IDs can be full file paths, which + /// blow past the record name limit when embedded verbatim. + static let hashedIDPrefix = "sha256-" + + /// The bare-ID component used in place of `id` when the verbatim name + /// would be too long. Deterministic, so every device derives the same + /// record name for the same entity. + static func hashedBareID(for id: String) -> String { + let digest = SHA256.hash(data: Data(id.utf8)) + return hashedIDPrefix + digest.map { String(format: "%02x", $0) }.joined() + } + + /// Whether a bare ID parsed out of a record name is a digest, in which + /// case the real ID must be recovered by matching local entities. + static func isHashedBareID(_ bareID: String) -> Bool { + bareID.hasPrefix(hashedIDPrefix) && bareID.count == hashedIDPrefix.count + 64 + } + + /// Whether an entity ID corresponds to the bare-ID component parsed from + /// a record name, whichever form (verbatim or digest) that component took. + static func entityID(_ id: String, matchesBareID bareID: String) -> Bool { + isHashedBareID(bareID) ? hashedBareID(for: id) == bareID : id == bareID + } + + /// Builds a scoped record name, falling back to a digest of the ID when + /// the verbatim form would exceed CloudKit's limit. + private static func scopedRecordName(prefix: String, id: String, scope: SourceScope) -> String { + let verbatim = "\(prefix)\(id)\(scope.recordNameSuffix)" + guard verbatim.utf8.count > maxRecordNameLength else { return verbatim } + return "\(prefix)\(hashedBareID(for: id))\(scope.recordNameSuffix)" + } + /// Extracts the bare ID from a record name by stripping the scope suffix. static func extractBareID(from value: String) -> String { for prefix in ["@global:", "@federated:", "@extracted:"] { @@ -147,11 +185,11 @@ enum SyncableRecordType: Sendable { switch self { case .subscription(let channelID, let scope): - recordName = "sub-\(channelID)\(scope.recordNameSuffix)" + recordName = Self.scopedRecordName(prefix: "sub-", id: channelID, scope: scope) case .watchEntry(let videoID, let scope): - recordName = "watch-\(videoID)\(scope.recordNameSuffix)" + recordName = Self.scopedRecordName(prefix: "watch-", id: videoID, scope: scope) case .bookmark(let videoID, let scope): - recordName = "bookmark-\(videoID)\(scope.recordNameSuffix)" + recordName = Self.scopedRecordName(prefix: "bookmark-", id: videoID, scope: scope) case .localPlaylist(let id): recordName = "playlist-\(id.uuidString)" case .localPlaylistItem(let id): @@ -159,9 +197,9 @@ enum SyncableRecordType: Sendable { case .searchHistory(let id): recordName = "search-\(id.uuidString)" case .recentChannel(let channelID, let scope): - recordName = "recent-channel-\(channelID)\(scope.recordNameSuffix)" + recordName = Self.scopedRecordName(prefix: "recent-channel-", id: channelID, scope: scope) case .recentPlaylist(let playlistID, let scope): - recordName = "recent-playlist-\(playlistID)\(scope.recordNameSuffix)" + recordName = Self.scopedRecordName(prefix: "recent-playlist-", id: playlistID, scope: scope) case .appSettings: recordName = "settings-singleton" case .instance(let id): @@ -169,7 +207,7 @@ enum SyncableRecordType: Sendable { case .mediaSource(let id): recordName = "source-\(id.uuidString)" case .channelNotificationSettings(let channelID, let scope): - recordName = "channel-notif-\(channelID)\(scope.recordNameSuffix)" + recordName = Self.scopedRecordName(prefix: "channel-notif-", id: channelID, scope: scope) case .controlsPreset(let id): recordName = "controls-\(id.uuidString)" case .account(let id): diff --git a/YatteeTests/CloudKitSyncTests.swift b/YatteeTests/CloudKitSyncTests.swift index c2840df8..1eec1ad9 100644 --- a/YatteeTests/CloudKitSyncTests.swift +++ b/YatteeTests/CloudKitSyncTests.swift @@ -217,4 +217,70 @@ struct CloudKitSyncTests { #expect(resolved["notificationsEnabled"] as? Int64 == 1) } } + + // MARK: - Record Names + + @Suite("Record Names") + struct RecordNameTests { + + private static let zone = CKRecordZone(zoneName: RecordType.zoneName) + private static let extracted = SourceScope.from( + sourceRawValue: "extracted", globalProvider: nil, instanceURLString: nil, externalExtractor: "media_source_webdav" + ) + + /// A WebDAV media-source ID: the full path, far past CloudKit's limit. + private static let longVideoID = String(repeating: "Ted Lasso S04E05 Riches of Embarrassment 1080p ATVP WEB-DL/", count: 6) + "episode.mkv" + + @Test("Short IDs keep their verbatim record name") + func shortIDVerbatim() { + let name = SyncableRecordType.watchEntry(videoID: "abc123", scope: Self.extracted).recordID(in: Self.zone).recordName + #expect(name == "watch-abc123@extracted:media_source_webdav") + #expect(SyncableRecordType.isHashedBareID(SyncableRecordType.extractBareID(from: String(name.dropFirst(6)))) == false) + } + + @Test("Over-long IDs fall back to a digest that fits CloudKit's limit") + func longIDHashed() { + let verbatim = "watch-\(Self.longVideoID)\(Self.extracted.recordNameSuffix)" + #expect(verbatim.utf8.count > SyncableRecordType.maxRecordNameLength) + + let recordID = SyncableRecordType.watchEntry(videoID: Self.longVideoID, scope: Self.extracted).recordID(in: Self.zone) + let name = recordID.recordName + #expect(name.utf8.count <= SyncableRecordType.maxRecordNameLength) + #expect(name.hasPrefix("watch-sha256-")) + #expect(name.hasSuffix("@extracted:media_source_webdav")) + + let bareID = SyncableRecordType.extractBareID(from: String(name.dropFirst(6))) + #expect(SyncableRecordType.isHashedBareID(bareID)) + #expect(SyncableRecordType.entityID(Self.longVideoID, matchesBareID: bareID)) + #expect(SyncableRecordType.entityID(Self.longVideoID + "x", matchesBareID: bareID) == false) + } + + @Test("Digest names are stable and distinct per ID and scope") + func digestStable() { + let a = SyncableRecordType.bookmark(videoID: Self.longVideoID, scope: Self.extracted).recordID(in: Self.zone).recordName + let b = SyncableRecordType.bookmark(videoID: Self.longVideoID, scope: Self.extracted).recordID(in: Self.zone).recordName + let other = SyncableRecordType.bookmark(videoID: Self.longVideoID + "2", scope: Self.extracted).recordID(in: Self.zone).recordName + #expect(a == b) + #expect(a != other) + } + + @Test("The mapper round-trips a long-ID watch entry without throwing") + @MainActor + func mapperLongID() { + let entry = WatchEntry( + videoID: Self.longVideoID, + sourceRawValue: "extracted", + title: "Video", + authorName: "Channel", + authorID: "ch1", + duration: 10, + watchedSeconds: 5, + isLive: false + ) + entry.externalExtractor = "media_source_webdav" + let record = CloudKitSyncTests.makeMapper().toCKRecord(watchEntry: entry) + #expect(record.recordID.recordName.hasPrefix("watch-sha256-")) + #expect(record["videoID"] as? String == Self.longVideoID) + } + } }