From 7d904bce235f904c9ccf872050d7d3c73ba35deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 11:52:46 +0200 Subject: [PATCH 01/11] Add logs --- .../LCP/Content Protection/LCPDecryptor.swift | 30 +++++++++++++++---- .../Navigator/Audiobook/AudioNavigator.swift | 19 ++++++++++-- .../Audiobook/PublicationMediaLoader.swift | 28 +++++++++++++++-- TestApp/Sources/Library/LibraryService.swift | 6 ++-- 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/Sources/LCP/Content Protection/LCPDecryptor.swift b/Sources/LCP/Content Protection/LCPDecryptor.swift index 86f23cf5b4..282635e0fc 100644 --- a/Sources/LCP/Content Protection/LCPDecryptor.swift +++ b/Sources/LCP/Content Protection/LCPDecryptor.swift @@ -9,8 +9,15 @@ import ReadiumShared private let lcpScheme = "http://readium.org/2014/01/lcp" +/// Timestamp helper to correlate the [#579] diagnostic log events. +func timestamp579lcp() -> String { + String(format: "t=%.3fs", CFAbsoluteTimeGetCurrent() - loadingStartTime579) +} + +private let loadingStartTime579 = CFAbsoluteTimeGetCurrent() + /// Decrypts a resource protected with LCP. -final class LCPDecryptor: Sendable { +final class LCPDecryptor: Sendable, Loggable { enum Error: Swift.Error { case emptyDecryptedData case invalidCBCData @@ -43,9 +50,11 @@ final class LCPDecryptor: Sendable { } if encryption.isDeflated || !encryption.isCbcEncrypted { + log(.info, "[#579] LCPDecryptor: using FullLCPResource for \(href) (isDeflated=\(encryption.isDeflated), isCbcEncrypted=\(encryption.isCbcEncrypted)) — the WHOLE resource will be read and decrypted on first access") return FullLCPResource(resource, license: license, encryption: encryption).cached() } else { + log(.info, "[#579] LCPDecryptor: using CBCLCPResource for \(href) (random access supported)") // We use a buffered resource because when requesting a range from // an LCP resource, we always read a bit more to align the data with // the next AES block. This means that consecutive requests are not @@ -63,14 +72,15 @@ final class LCPDecryptor: Sendable { /// Can be used when it's impossible to map a read range (byte range /// request) to the encrypted resource, for example when the resource is /// deflated before encryption. - private final class FullLCPResource: Resource, Sendable { + private final class FullLCPResource: Resource, Sendable, Loggable { private let resource: TransformingResource private let originalLength: UInt64? init(_ resource: Resource, license: LCPLicense, encryption: ReadiumShared.Encryption) { originalLength = encryption.originalLength.map { UInt64($0) } self.resource = TransformingResource(resource, transform: { data in - await license.decryptFully(data: data, isDeflated: encryption.isDeflated) + Self.log(.info, "[#579] \(timestamp579lcp()) FullLCPResource: decrypting the WHOLE resource (\((try? data.get().count) ?? -1) bytes) in one shot") + return await license.decryptFully(data: data, isDeflated: encryption.isDeflated) }) } @@ -92,7 +102,7 @@ final class LCPDecryptor: Sendable { /// A LCP resource used to read content encrypted with the CBC algorithm. /// /// Supports random access for byte range requests, but the resource MUST NOT be deflated. - private final class CBCLCPResource: Resource, Sendable { + private final class CBCLCPResource: Resource, Sendable, Loggable { private let resource: Resource private let license: LCPLicense private let encryption: ReadiumShared.Encryption @@ -121,9 +131,14 @@ final class LCPDecryptor: Sendable { } @concurrent func stream(range: Range?, consume: @escaping @Sendable (Data) -> Void) async -> ReadResult { + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource.stream(range: \(range.map(String.init(describing:)) ?? "nil"))") + guard let range = range else { + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: reading the FULL encrypted resource in memory before decrypting…") + let readStart = CFAbsoluteTimeGetCurrent() return await license.decryptFully(data: resource.read(), isDeflated: encryption.isDeflated) .map { + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: full read+decrypt done in \(String(format: "%.3fs", CFAbsoluteTimeGetCurrent() - readStart)), delivering \($0.count) bytes in a SINGLE consume call") consume($0) return () } @@ -145,9 +160,13 @@ final class LCPDecryptor: Sendable { encryptedLength ) + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: buffering the WHOLE encrypted range \(encryptedStart ..< encryptedEndExclusive) (\(encryptedEndExclusive - encryptedStart) bytes) before decrypting…") + let readStart = CFAbsoluteTimeGetCurrent() + return await resource.read(range: encryptedStart ..< encryptedEndExclusive) .combine(plainTextSize()) - .flatMap { encryptedData, plainTextSize in + .flatMap { [self] encryptedData, plainTextSize in + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: range read done in \(String(format: "%.3fs", CFAbsoluteTimeGetCurrent() - readStart)), got \(encryptedData.count) bytes, decrypting in one shot") do { guard let plainTextSize = plainTextSize else { return .failure(.decoding(LCPDecryptor.Error.noPlainTextSize)) @@ -170,6 +189,7 @@ final class LCPDecryptor: Sendable { // include padding. let sliceEnd = sliceStart + rangeLength + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: delivering \(sliceEnd - sliceStart) decrypted bytes in a SINGLE consume call") consume(bytes[sliceStart ..< sliceEnd]) return .success(()) } catch { diff --git a/Sources/Navigator/Audiobook/AudioNavigator.swift b/Sources/Navigator/Audiobook/AudioNavigator.swift index 7f268bb7d8..43e98fbcb6 100644 --- a/Sources/Navigator/Audiobook/AudioNavigator.swift +++ b/Sources/Navigator/Audiobook/AudioNavigator.swift @@ -222,6 +222,7 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo await go(to: link) } } + log(.info, "[#579] \(timestamp579()) play(): calling playImmediately(atRate: \(settings.speed)) with automaticallyWaitsToMinimizeStalling=\(player.automaticallyWaitsToMinimizeStalling)") player.playImmediately(atRate: Float(settings.speed)) } } @@ -280,9 +281,10 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo preferredTimescale: 1000 ), queue: .main - ) { [weak self] _ in + ) { [weak self] time in MainActor.assumeIsolated { guard let self = self else { return } + self.log(.info, "[#579] \(timestamp579()) periodic tick: time=\(time.secondsOrZero), reportedState=\(self.state), rate=\(self.player.rate), bufferEmpty=\(self.player.currentItem?.isPlaybackBufferEmpty.description ?? "nil"), likelyToKeepUp=\(self.player.currentItem?.isPlaybackLikelyToKeepUp.description ?? "nil")") self.locationDidChange() } } @@ -306,7 +308,8 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo } } - timeControlStatusObserver = player.observe(\.timeControlStatus, options: [.new, .old]) { [weak self] _, _ in + timeControlStatusObserver = player.observe(\.timeControlStatus, options: [.new, .old]) { [weak self] player, _ in + self?.log(.info, "[#579] \(timestamp579()) timeControlStatus changed to \(player.timeControlStatus.debugLabel) (rate=\(player.rate), reasonForWaitingToPlay=\(player.reasonForWaitingToPlay?.rawValue ?? "nil"), bufferEmpty=\(player.currentItem?.isPlaybackBufferEmpty.description ?? "nil"), likelyToKeepUp=\(player.currentItem?.isPlaybackLikelyToKeepUp.description ?? "nil"))") Task { @MainActor [weak self] in self?.playbackDidChange() } @@ -580,6 +583,18 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo } } +extension AVPlayer.TimeControlStatus { + /// Debug helper for the [#579] diagnostic log events. + var debugLabel: String { + switch self { + case .paused: return "paused" + case .waitingToPlayAtSpecifiedRate: return "waitingToPlayAtSpecifiedRate" + case .playing: return "playing" + @unknown default: return "unknown(\(rawValue))" + } + } +} + private extension MediaPlaybackState { init(_ timeControlStatus: AVPlayer.TimeControlStatus) { switch timeControlStatus { diff --git a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift index c36d1a3208..4534c79aae 100644 --- a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift +++ b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift @@ -138,13 +138,16 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log using resource: Resource, link: Link ) { - tasks.add { + log(.info, "[#579] \(timestamp579()) contentInformationRequest for \(link.href)") + + tasks.add { [self] in infoRequest.isByteRangeAccessSupported = true infoRequest.contentType = link.mediaType?.uti switch await resource.length() { case let .success(length): infoRequest.contentLength = Int64(length) + log(.info, "[#579] \(timestamp579()) contentInformationRequest fulfilled: contentLength=\(length), contentType=\(infoRequest.contentType ?? "nil"), byteRangeAccessSupported=true") request.finishLoading() case let .failure(error): @@ -162,17 +165,28 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log range = UInt64(dataRequest.currentOffset) ..< (UInt64(dataRequest.currentOffset) + UInt64(dataRequest.requestedLength)) } - let task = Task { + log(.info, "[#579] \(timestamp579()) dataRequest for \(link.href): currentOffset=\(dataRequest.currentOffset), requestedLength=\(dataRequest.requestedLength), requestsAllDataToEndOfResource=\(dataRequest.requestsAllDataToEndOfResource) -> range=\(range.map(String.init(describing:)) ?? "nil (full resource)")") + + let task = Task { [self] in + var consumedBytes = 0 + var consumeCalls = 0 let result = await resource.stream( range: range, - consume: { dataRequest.respond(with: $0) } + consume: { + consumedBytes += $0.count + consumeCalls += 1 + log(.info, "[#579] \(timestamp579()) consume #\(consumeCalls): chunk=\($0.count) bytes, total=\(consumedBytes) bytes") + dataRequest.respond(with: $0) + } ) queue.async { [weak self] in switch result { case .success: + self?.log(.info, "[#579] \(timestamp579()) dataRequest finished: \(consumeCalls) consume call(s), \(consumedBytes) bytes total") request.finishLoading() case let .failure(error): + self?.log(.info, "[#579] \(timestamp579()) dataRequest failed after \(consumeCalls) consume call(s), \(consumedBytes) bytes: \(error)") request.finishLoading(with: error) } @@ -184,10 +198,18 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log } func resourceLoader(_ resourceLoader: AVAssetResourceLoader, didCancel loadingRequest: AVAssetResourceLoadingRequest) { + log(.info, "[#579] \(timestamp579()) didCancel loadingRequest: offset=\(loadingRequest.dataRequest?.currentOffset ?? -1)") finishRequest(loadingRequest) } } +/// Timestamp helper to correlate the [#579] diagnostic log events. +func timestamp579() -> String { + String(format: "t=%.3fs", CFAbsoluteTimeGetCurrent() - loadingStartTime579) +} + +private let loadingStartTime579 = CFAbsoluteTimeGetCurrent() + private let schemePrefix = "readium" extension URL { diff --git a/TestApp/Sources/Library/LibraryService.swift b/TestApp/Sources/Library/LibraryService.swift index 88ad0f6093..128542b2c3 100644 --- a/TestApp/Sources/Library/LibraryService.swift +++ b/TestApp/Sources/Library/LibraryService.swift @@ -108,9 +108,9 @@ import UIKit } var url = url - if let file = url.fileURL { - url = try await fulfillIfNeeded(file, progress: progress) - } +// if let file = url.fileURL { +// url = try await fulfillIfNeeded(file, progress: progress) +// } let (pub, format) = try await openPublication(at: url, allowUserInteraction: false) let title = pub.metadata.title ?? url.url.deletingPathExtension().lastPathComponent From d744d6b9e72a7b61bbc173f9126790053fe92f1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 12:02:06 +0200 Subject: [PATCH 02/11] Clamp out-of-range ZIP reads and cap BufferingResource read-ahead The Streamable contract requires out-of-range indexes to be clamped, but ZIPFoundationContainer let ReadiumZIPFoundation throw a rangeOutOfBounds error instead. BufferingResource also extended every request with a read-ahead that could exceed the resource length, over-requesting from resources that reject out-of-range ranges (e.g. HTTP servers replying with 416). Part of #579. Co-Authored-By: Claude Fable 5 --- .../Data/Resource/BufferingResource.swift | 10 +++- .../ZIPFoundationContainer.swift | 8 ++++ .../Resource/BufferingResourceTests.swift | 46 +++++++++++++++++++ .../Toolkit/ZIP/MinizipContainerTests.swift | 29 ++++++++++++ .../ZIP/ZIPFoundationContainerTests.swift | 29 ++++++++++++ 5 files changed, 120 insertions(+), 2 deletions(-) diff --git a/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift b/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift index 735d8ab54b..aa004063d9 100644 --- a/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift +++ b/Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift @@ -74,8 +74,14 @@ public actor BufferingResource: Resource, Loggable { return .success(()) } - // Read ahead from the request start to fill the buffer. - let readAheadEnd = requestedRange.lowerBound + UInt64(buffer.maxSize) + // Read ahead from the request start to fill the buffer, without + // requesting bytes past the end of the resource. Not all resources + // clamp out-of-range requests, e.g. HTTP servers reply with a + // 416 Range Not Satisfiable error. + var readAheadEnd = requestedRange.lowerBound + UInt64(buffer.maxSize) + if case let .success(.some(length)) = await estimatedLength() { + readAheadEnd = min(readAheadEnd, length) + } let readRange = requestedRange.lowerBound ..< max(requestedRange.upperBound, readAheadEnd) // Range that will actually need to be read from the original resource, diff --git a/Sources/Shared/Toolkit/ZIP/ZIPFoundation/ZIPFoundationContainer.swift b/Sources/Shared/Toolkit/ZIP/ZIPFoundation/ZIPFoundationContainer.swift index 7aa3e61397..f2e55c34a1 100644 --- a/Sources/Shared/Toolkit/ZIP/ZIPFoundation/ZIPFoundationContainer.swift +++ b/Sources/Shared/Toolkit/ZIP/ZIPFoundation/ZIPFoundationContainer.swift @@ -112,6 +112,14 @@ private actor ZIPFoundationResource: Resource, Loggable { return await archive().asyncFlatMap { archive in do { if let range = range { + // The `Streamable` contract requires out-of-range indexes + // to be clamped, while ZIPFoundation throws a + // `rangeOutOfBounds` error. + let length = entry.uncompressedSize + let range = min(range.lowerBound, length) ..< min(range.upperBound, length) + guard !range.isEmpty else { + return .success(()) + } try await archive.extractRange(range, of: entry) { data in try Task.checkCancellation() consume(data) diff --git a/Tests/SharedTests/Toolkit/Data/Resource/BufferingResourceTests.swift b/Tests/SharedTests/Toolkit/Data/Resource/BufferingResourceTests.swift index b2bcff92b2..a1be4802ff 100644 --- a/Tests/SharedTests/Toolkit/Data/Resource/BufferingResourceTests.swift +++ b/Tests/SharedTests/Toolkit/Data/Resource/BufferingResourceTests.swift @@ -100,6 +100,20 @@ class BufferingResourceTests: XCTestCase { } } + /// The read-ahead must not request bytes past the end of the resource, + /// as some resources reject out-of-range requests instead of clamping + /// them (e.g. HTTP servers reply with a 416 error). + func testReadNearEndDoesNotOverRequest() async throws { + let data = Data((0 ..< 2000).map { UInt8($0 % 256) }) + let sut = BufferingResource(resource: StrictResource(data: data), bufferSize: 1024) + + var result = try await sut.read(range: 1900 ..< 2000).get() + XCTAssertEqual(result, data[1900 ..< 2000]) + + result = try await sut.read(range: 1990 ..< 1995).get() + XCTAssertEqual(result, data[1990 ..< 1995]) + } + private let file = FileURL(url: TestPublications.url(for: "childrens-literature.epub"))! private lazy var data = try! Data(contentsOf: file.url) private lazy var resource = FileResource(file: file) @@ -120,3 +134,35 @@ class BufferingResourceTests: XCTestCase { } } } + +/// A `Resource` that fails when requesting a range past its end, instead of +/// clamping it. +private actor StrictResource: Resource { + private let data: Data + + init(data: Data) { + self.data = data + } + + let sourceURL: AbsoluteURL? = nil + + func estimatedLength() async -> ReadResult { + .success(UInt64(data.count)) + } + + func properties() async -> ReadResult { + .success(ResourceProperties()) + } + + func stream(range: Range?, consume: @escaping (Data) -> Void) async -> ReadResult { + guard let range = range else { + consume(data) + return .success(()) + } + guard range.upperBound <= UInt64(data.count) else { + return .failure(.decoding("Requested a range out of bounds")) + } + consume(data[Int(range.lowerBound) ..< Int(range.upperBound)]) + return .success(()) + } +} diff --git a/Tests/SharedTests/Toolkit/ZIP/MinizipContainerTests.swift b/Tests/SharedTests/Toolkit/ZIP/MinizipContainerTests.swift index 857e02fa73..8f08616fba 100644 --- a/Tests/SharedTests/Toolkit/ZIP/MinizipContainerTests.swift +++ b/Tests/SharedTests/Toolkit/ZIP/MinizipContainerTests.swift @@ -107,6 +107,35 @@ class MinizipContainerTests: XCTestCase { ) } + /// Out-of-range indexes must be clamped per the `Streamable` contract. + func testReadRangeOverlappingEndIsClamped() async throws { + let container = try await container(for: "test.zip") + + let stored = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file.txt"))]) + var data = try await stored.read(range: 14 ..< 40).get() + XCTAssertEqual( + String(data: data, encoding: .utf8), + " ZIP.\n" + ) + + let compressed = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file-compressed.txt"))]) + data = try await compressed.read(range: 29600 ..< 29700).get() + XCTAssertEqual(data.count, 9) + } + + /// Out-of-range indexes must be clamped per the `Streamable` contract. + func testReadRangePastEndReturnsEmptyData() async throws { + let container = try await container(for: "test.zip") + + let stored = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file.txt"))]) + var data = try await stored.read(range: 100 ..< 200).get() + XCTAssertEqual(data, Data()) + + let compressed = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file-compressed.txt"))]) + data = try await compressed.read(range: 40000 ..< 40100).get() + XCTAssertEqual(data, Data()) + } + func testRandomCompressedRead() async throws { for _ in 0 ..< 100 { let container = try await container(for: "test.zip") diff --git a/Tests/SharedTests/Toolkit/ZIP/ZIPFoundationContainerTests.swift b/Tests/SharedTests/Toolkit/ZIP/ZIPFoundationContainerTests.swift index f86e892428..fec4c49ffd 100644 --- a/Tests/SharedTests/Toolkit/ZIP/ZIPFoundationContainerTests.swift +++ b/Tests/SharedTests/Toolkit/ZIP/ZIPFoundationContainerTests.swift @@ -106,6 +106,35 @@ class ZIPFoundationContainerTests: XCTestCase { ) } + /// Out-of-range indexes must be clamped per the `Streamable` contract. + func testReadRangeOverlappingEndIsClamped() async throws { + let container = try await container(for: "test.zip") + + let stored = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file.txt"))]) + var data = try await stored.read(range: 14 ..< 40).get() + XCTAssertEqual( + String(data: data, encoding: .utf8), + " ZIP.\n" + ) + + let compressed = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file-compressed.txt"))]) + data = try await compressed.read(range: 29600 ..< 29700).get() + XCTAssertEqual(data.count, 9) + } + + /// Out-of-range indexes must be clamped per the `Streamable` contract. + func testReadRangePastEndReturnsEmptyData() async throws { + let container = try await container(for: "test.zip") + + let stored = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file.txt"))]) + var data = try await stored.read(range: 100 ..< 200).get() + XCTAssertEqual(data, Data()) + + let compressed = try XCTUnwrap(try container[XCTUnwrap(AnyURL(path: "A folder/Sub.folder%/file-compressed.txt"))]) + data = try await compressed.read(range: 40000 ..< 40100).get() + XCTAssertEqual(data, Data()) + } + func testRandomCompressedRead() async throws { for _ in 0 ..< 100 { let container = try await container(for: "test.zip") From a3b33bce0305dae73c98a0d3d9f8ec2183191a0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 12:10:57 +0200 Subject: [PATCH 03/11] Stream LCP CBC resources in decrypted chunks CBCLCPResource used to materialize the entire requested range in memory (or the whole resource for open-ended requests) before a single consume call. Since AVPlayer requests audio tracks with open-ended ranges, a streamed LCP audiobook was fully downloaded and decrypted before playback could start. The range is now decrypted and delivered in 256 KB plaintext chunks, checking for task cancellation between chunks so AVPlayer can actually stop an abandoned request. Part of #579. Co-Authored-By: Claude Fable 5 --- .../LCP/Content Protection/LCPDecryptor.swift | 154 ++++++++++++------ Tests/LCPTests/LCPDecryptionTests.swift | 30 ++++ 2 files changed, 134 insertions(+), 50 deletions(-) diff --git a/Sources/LCP/Content Protection/LCPDecryptor.swift b/Sources/LCP/Content Protection/LCPDecryptor.swift index 282635e0fc..6afe60f128 100644 --- a/Sources/LCP/Content Protection/LCPDecryptor.swift +++ b/Sources/LCP/Content Protection/LCPDecryptor.swift @@ -130,73 +130,127 @@ final class LCPDecryptor: Sendable, Loggable { await plainTextSize() } + /// Number of plaintext bytes decrypted and delivered per `consume` + /// call when streaming. + private static let chunkSize: UInt64 = 256 * 1024 + @concurrent func stream(range: Range?, consume: @escaping @Sendable (Data) -> Void) async -> ReadResult { log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource.stream(range: \(range.map(String.init(describing:)) ?? "nil"))") - guard let range = range else { - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: reading the FULL encrypted resource in memory before decrypting…") - let readStart = CFAbsoluteTimeGetCurrent() + var plainTextSize: UInt64? + switch await self.plainTextSize() { + case let .success(size): + plainTextSize = size + case let .failure(error): + guard range == nil else { + return .failure(error) + } + } + + guard let plainTextSize = plainTextSize else { + // Without the plaintext size, we can't compute the chunks to + // decrypt; fall back on reading and decrypting the whole + // resource in one shot. + guard range == nil else { + return failure(.noPlainTextSize) + } + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: no plaintext size, reading the FULL encrypted resource in memory before decrypting…") return await license.decryptFully(data: resource.read(), isDeflated: encryption.isDeflated) .map { - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: full read+decrypt done in \(String(format: "%.3fs", CFAbsoluteTimeGetCurrent() - readStart)), delivering \($0.count) bytes in a SINGLE consume call") consume($0) return () } } - return await resource.estimatedLength().asyncFlatMap { encryptedLength in + let requestedRange = range ?? 0 ..< plainTextSize + let clampedRange = min(requestedRange.lowerBound, plainTextSize) ..< min(requestedRange.upperBound, plainTextSize) + guard !clampedRange.isEmpty else { + return .success(()) + } + + return await resource.estimatedLength().asyncFlatMap { [self] encryptedLength in guard let encryptedLength = encryptedLength else { return .failure(.decoding(LCPDecryptor.Error.requiredEstimatedLength)) } - guard let rangeFirst = range.first, let rangeLast = range.last else { - return .failure(.decoding(LCPDecryptor.Error.invalidRange(range))) + + log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: streaming range \(clampedRange) in chunks of \(Self.chunkSize) bytes") + + // Decrypting in chunks lets the caller process the beginning + // of the resource without waiting for the whole range, which + // matters when streaming a large track from the network. + var chunkStart = clampedRange.lowerBound + while chunkStart < clampedRange.upperBound { + guard !Task.isCancelled else { + return .failure(.cancelled) + } + + let chunkEnd = min(chunkStart + Self.chunkSize, clampedRange.upperBound) + let result = await decrypt( + range: chunkStart ..< chunkEnd, + encryptedLength: encryptedLength, + plainTextSize: plainTextSize + ) + switch result { + case let .success(chunk): + consume(chunk) + chunkStart = chunkEnd + case let .failure(error): + return .failure(error) + } } - // Encrypted data is shifted by AESBlockSize, because of IV and because the - // previous block must be provided to perform XOR on intermediate blocks. - let encryptedStart = rangeFirst.floorMultiple(of: AESBlockSize) - let encryptedEndExclusive = min( - (rangeLast + 1).ceilMultiple(of: AESBlockSize) + AESBlockSize, - encryptedLength - ) - - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: buffering the WHOLE encrypted range \(encryptedStart ..< encryptedEndExclusive) (\(encryptedEndExclusive - encryptedStart) bytes) before decrypting…") - let readStart = CFAbsoluteTimeGetCurrent() - - return await resource.read(range: encryptedStart ..< encryptedEndExclusive) - .combine(plainTextSize()) - .flatMap { [self] encryptedData, plainTextSize in - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: range read done in \(String(format: "%.3fs", CFAbsoluteTimeGetCurrent() - readStart)), got \(encryptedData.count) bytes, decrypting in one shot") - do { - guard let plainTextSize = plainTextSize else { - return .failure(.decoding(LCPDecryptor.Error.noPlainTextSize)) - } - guard let bytes = try license.decipher(encryptedData) else { - return .failure(.decoding(LCPDecryptor.Error.emptyDecryptedData)) - } - - // Exclude the bytes added to match a multiple of AESBlockSize. - let sliceStart = (rangeFirst - encryptedStart) - - let isLastBlockRead = encryptedLength - encryptedEndExclusive <= AESBlockSize - let rangeLength = isLastBlockRead - // Use decrypted length to ensure `rangeLast` doesn't exceed decrypted length - 1. - ? min(rangeLast, plainTextSize - 1) - rangeFirst + 1 - // The last block won't be read, so there's no need to compute the length - : rangeLast - rangeFirst + 1 - - // Keep only enough bytes to fit the length-corrected request in order to never - // include padding. - let sliceEnd = sliceStart + rangeLength - - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: delivering \(sliceEnd - sliceStart) decrypted bytes in a SINGLE consume call") - consume(bytes[sliceStart ..< sliceEnd]) - return .success(()) - } catch { - return .failure(.decoding(error)) + return .success(()) + } + } + + /// Decrypts a single chunk of plaintext located at `range`. + private func decrypt( + range: Range, + encryptedLength: UInt64, + plainTextSize: UInt64 + ) async -> ReadResult { + guard let rangeFirst = range.first, let rangeLast = range.last else { + return failure(.invalidRange(range)) + } + + // Encrypted data is shifted by AESBlockSize, because of IV and because the + // previous block must be provided to perform XOR on intermediate blocks. + let encryptedStart = rangeFirst.floorMultiple(of: AESBlockSize) + let encryptedEndExclusive = min( + (rangeLast + 1).ceilMultiple(of: AESBlockSize) + AESBlockSize, + encryptedLength + ) + + return await resource.read(range: encryptedStart ..< encryptedEndExclusive) + .flatMap { [self] encryptedData in + do { + guard let bytes = try license.decipher(encryptedData) else { + return failure(.emptyDecryptedData) } + + // Exclude the bytes added to match a multiple of AESBlockSize. + let sliceStart = (rangeFirst - encryptedStart) + + let isLastBlockRead = encryptedLength - encryptedEndExclusive <= AESBlockSize + let rangeLength = isLastBlockRead + // Use decrypted length to ensure `rangeLast` doesn't exceed decrypted length - 1. + ? min(rangeLast, plainTextSize - 1) - rangeFirst + 1 + // The last block won't be read, so there's no need to compute the length + : rangeLast - rangeFirst + 1 + + // Keep only enough bytes to fit the length-corrected request in order to never + // include padding. + let sliceEnd = sliceStart + rangeLength + + return .success(bytes[sliceStart ..< sliceEnd]) + } catch { + return .failure(.decoding(error)) } - } + } + } + + private func failure(_ error: LCPDecryptor.Error) -> ReadResult { + .failure(.decoding(error)) } } } diff --git a/Tests/LCPTests/LCPDecryptionTests.swift b/Tests/LCPTests/LCPDecryptionTests.swift index 3abb935ed4..2fb6a7470b 100644 --- a/Tests/LCPTests/LCPDecryptionTests.swift +++ b/Tests/LCPTests/LCPDecryptionTests.swift @@ -70,6 +70,36 @@ struct LCPDecryptionTests { } } + /// A large range is decrypted and delivered in several chunks, so the + /// caller can process the beginning of the resource without waiting for + /// the whole range. + @Test func streamsLargeRangeInChunks() async throws { + var chunks: [Data] = [] + let result = await encryptedResource.stream(range: 0 ..< UInt64(clearData.count)) { chunk in + chunks.append(chunk) + } + try result.get() + + #expect(chunks.count > 1) + #expect(chunks.reduce(Data(), +) == clearData) + } + + /// Cancelling the task stops the decryption loop, instead of streaming + /// the remaining chunks. + @Test func cancellationStopsStreaming() async throws { + var chunks: [Data] = [] + let result = await encryptedResource.stream(range: 0 ..< UInt64(clearData.count)) { chunk in + chunks.append(chunk) + withUnsafeCurrentTask { $0?.cancel() } + } + + guard case .failure(.cancelled) = result else { + Issue.record("Expected a cancelled failure, got \(result)") + return + } + #expect(chunks.count == 1) + } + /// Reproduces the arithmetic overflow in /// `CBCLCPResource.stream(range:consume:)`. /// From 62c3b63f036d397cc8b9d929ebd2ae50facce0f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 12:21:31 +0200 Subject: [PATCH 04/11] Report honest playback state and surface loading errors in AudioNavigator With automaticallyWaitsToMinimizeStalling disabled, AVPlayer reports .playing even when stalled on an empty buffer, so AudioNavigator claimed the audiobook was playing while nothing was audible. The state now reports .loading in that situation, with a KVO observer on isPlaybackLikelyToKeepUp for snappier transitions. Failures of the AVPlayerItem and of the media loader used to be silently swallowed; they are now forwarded to NavigatorDelegate.navigator(_:didFailToLoadResourceAt:withError:). Part of #579. Co-Authored-By: Claude Fable 5 --- .../Navigator/Audiobook/AudioNavigator.swift | 57 ++++++++++++++++++- .../Audiobook/PublicationMediaLoader.swift | 15 +++++ 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/Sources/Navigator/Audiobook/AudioNavigator.swift b/Sources/Navigator/Audiobook/AudioNavigator.swift index 43e98fbcb6..60324e6377 100644 --- a/Sources/Navigator/Audiobook/AudioNavigator.swift +++ b/Sources/Navigator/Audiobook/AudioNavigator.swift @@ -156,7 +156,18 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo /// Returns whether the resource is currently playing or not. public var state: MediaPlaybackState { - MediaPlaybackState(player.timeControlStatus) + let state = MediaPlaybackState(player.timeControlStatus) + if + state == .playing, + let item = player.currentItem, + item.isPlaybackBufferEmpty, !item.isPlaybackLikelyToKeepUp + { + // As `automaticallyWaitsToMinimizeStalling` is disabled, the + // player reports `.playing` even when it is stalled on an empty + // buffer waiting for data. + return .loading + } + return state } /// Current playback info. @@ -262,10 +273,23 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo private var rateObserver: NSKeyValueObservation? private var timeControlStatusObserver: NSKeyValueObservation? private var currentItemObserver: NSKeyValueObservation? + private var itemStatusObserver: NSKeyValueObservation? + private var itemLikelyToKeepUpObserver: NSKeyValueObservation? private var timeObserverToken: TimeObserverToken? private var notificationTask: Task? - private lazy var mediaLoader = PublicationMediaLoader(publication: publication) + private lazy var mediaLoader: PublicationMediaLoader = { + let loader = PublicationMediaLoader(publication: publication) + loader.onLoadingError = { [weak self] href, error in + Task { @MainActor in + guard let self = self, let href = href.relativeURL else { + return + } + self.delegate?.navigator(self, didFailToLoadResourceAt: href, withError: error) + } + } + return loader + }() private lazy var player: AVPlayer = makePlayer() @@ -317,7 +341,9 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo currentItemObserver = player.observe(\.currentItem, options: [.new, .old]) { [weak self] _, _ in Task { @MainActor [weak self] in - self?.playbackDidChange() + guard let self = self else { return } + self.observe(currentItem: self.player.currentItem) + self.playbackDidChange() } } @@ -342,6 +368,31 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo return player } + private func observe(currentItem item: AVPlayerItem?) { + itemLikelyToKeepUpObserver = item?.observe(\.isPlaybackLikelyToKeepUp) { [weak self] _, _ in + self?.playbackDidChange() + } + + itemStatusObserver = item?.observe(\.status) { [weak self] item, _ in + guard let self = self, item.status == .failed else { + return + } + + let itemError = item.error + log(.error, "Failed to load the player item: \(String(describing: itemError))") + + let href = publication.readingOrder[resourceIndex].url().relativeURL + Task { @MainActor in + guard let href = href else { + return + } + let error: ReadError = itemError.flatMap { .wrap($0) } + ?? .decoding("The AVPlayerItem failed to load", cause: itemError) + self.delegate?.navigator(self, didFailToLoadResourceAt: href, withError: error) + } + } + } + private func shouldPlayNextResource() -> Bool { guard let delegate = delegate else { return true diff --git a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift index 4534c79aae..67cffbd86a 100644 --- a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift +++ b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift @@ -19,6 +19,10 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log private let publication: Publication + /// Called when a resource failed to be served to the player, e.g. to + /// forward the error to the `NavigatorDelegate`. + var onLoadingError: ((AnyURL, ReadError) -> Void)? + private let tasks = CancellableTasks() init(publication: Publication) { @@ -152,6 +156,7 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log case let .failure(error): log(.error, error) + report(error, forHREF: link.url()) request.finishLoading(with: error) } } @@ -187,6 +192,7 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log request.finishLoading() case let .failure(error): self?.log(.info, "[#579] \(timestamp579()) dataRequest failed after \(consumeCalls) consume call(s), \(consumedBytes) bytes: \(error)") + self?.report(error, forHREF: link.url()) request.finishLoading(with: error) } @@ -197,6 +203,15 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log registerRequest(request, task: task, for: link.url()) } + private func report(_ error: ReadError, forHREF href: AnyURL) { + // Cancellation is not an error worth reporting, it occurs whenever + // the player abandons a data request, e.g. when seeking. + if case .cancelled = error { + return + } + onLoadingError?(href, error) + } + func resourceLoader(_ resourceLoader: AVAssetResourceLoader, didCancel loadingRequest: AVAssetResourceLoadingRequest) { log(.info, "[#579] \(timestamp579()) didCancel loadingRequest: offset=\(loadingRequest.dataRequest?.currentOffset ?? -1)") finishRequest(loadingRequest) From 8951461765207d80bf9410fb819010af22e5f653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 12:22:09 +0200 Subject: [PATCH 05/11] Update CHANGELOG for #579 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6a2c2bed9..baa289fe3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,12 +90,18 @@ All notable changes to this project will be documented in this file. Take a look #### Shared * EPUB HREFs that are not percent-encoded but carry a fragment or query (e.g. `chapter one.xhtml#section`, with a space in the filename) now keep their `#fragment`/`?query` instead of encoding the separators into the path. This fixes table of contents and Media Overlays links failing to resolve and navigate in poorly-authored EPUBs. +* [#579](https://github.com/readium/swift-toolkit/issues/579) Reading a range past the end of a ZIP entry now returns the clamped bytes instead of failing, per the `Streamable` contract. `BufferingResource` also no longer extends its read-ahead past the end of the resource, which HTTP servers reject with a 416 error. #### Navigator * Fixed custom `EditingAction`s sometimes missing from the text-selection menu for double-tap (single word) selections (contributed by [@raphi011](https://github.com/readium/swift-toolkit/pull/822)). * Fixed memory leak in the `AudioNavigator`. * [#802](https://github.com/readium/swift-toolkit/issues/802) Fixed fonts declared with `fontFamilyDeclarations` never loading in the EPUB navigator. Font fetches were CORS-gated by WebKit (contributed by [@atani](https://github.com/readium/swift-toolkit/pull/845)). +* [#579](https://github.com/readium/swift-toolkit/issues/579) The `AudioNavigator` now reports the `.loading` state while the player is stalled on an empty buffer, and forwards resource loading errors to `NavigatorDelegate.navigator(_:didFailToLoadResourceAt:withError:)` instead of swallowing them. + +#### LCP + +* [#579](https://github.com/readium/swift-toolkit/issues/579) Streamed LCP audiobooks now start playing almost immediately. Resources encrypted with AES-CBC are decrypted and served in chunks, instead of being fully downloaded and decrypted upfront. ## [3.10.0] - 2026-06-24 From a231d3b0c2e3eee11b5003896a1a0e76b5b34bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 12:35:06 +0200 Subject: [PATCH 06/11] Remove #579 diagnostic logs The end-to-end verification on a streamed LCP audiobook confirmed the fixes, so the temporary instrumentation is no longer needed. Also restores the LCPL fulfillment in the TestApp, which was disabled to force streaming. Co-Authored-By: Claude Fable 5 --- .../LCP/Content Protection/LCPDecryptor.swift | 23 ++++-------------- .../Navigator/Audiobook/AudioNavigator.swift | 17 +------------ .../Audiobook/PublicationMediaLoader.swift | 24 +------------------ TestApp/Sources/Library/LibraryService.swift | 6 ++--- 4 files changed, 9 insertions(+), 61 deletions(-) diff --git a/Sources/LCP/Content Protection/LCPDecryptor.swift b/Sources/LCP/Content Protection/LCPDecryptor.swift index 6afe60f128..3b7617599e 100644 --- a/Sources/LCP/Content Protection/LCPDecryptor.swift +++ b/Sources/LCP/Content Protection/LCPDecryptor.swift @@ -9,15 +9,8 @@ import ReadiumShared private let lcpScheme = "http://readium.org/2014/01/lcp" -/// Timestamp helper to correlate the [#579] diagnostic log events. -func timestamp579lcp() -> String { - String(format: "t=%.3fs", CFAbsoluteTimeGetCurrent() - loadingStartTime579) -} - -private let loadingStartTime579 = CFAbsoluteTimeGetCurrent() - /// Decrypts a resource protected with LCP. -final class LCPDecryptor: Sendable, Loggable { +final class LCPDecryptor: Sendable { enum Error: Swift.Error { case emptyDecryptedData case invalidCBCData @@ -50,11 +43,9 @@ final class LCPDecryptor: Sendable, Loggable { } if encryption.isDeflated || !encryption.isCbcEncrypted { - log(.info, "[#579] LCPDecryptor: using FullLCPResource for \(href) (isDeflated=\(encryption.isDeflated), isCbcEncrypted=\(encryption.isCbcEncrypted)) — the WHOLE resource will be read and decrypted on first access") return FullLCPResource(resource, license: license, encryption: encryption).cached() } else { - log(.info, "[#579] LCPDecryptor: using CBCLCPResource for \(href) (random access supported)") // We use a buffered resource because when requesting a range from // an LCP resource, we always read a bit more to align the data with // the next AES block. This means that consecutive requests are not @@ -72,15 +63,14 @@ final class LCPDecryptor: Sendable, Loggable { /// Can be used when it's impossible to map a read range (byte range /// request) to the encrypted resource, for example when the resource is /// deflated before encryption. - private final class FullLCPResource: Resource, Sendable, Loggable { + private final class FullLCPResource: Resource, Sendable { private let resource: TransformingResource private let originalLength: UInt64? init(_ resource: Resource, license: LCPLicense, encryption: ReadiumShared.Encryption) { originalLength = encryption.originalLength.map { UInt64($0) } self.resource = TransformingResource(resource, transform: { data in - Self.log(.info, "[#579] \(timestamp579lcp()) FullLCPResource: decrypting the WHOLE resource (\((try? data.get().count) ?? -1) bytes) in one shot") - return await license.decryptFully(data: data, isDeflated: encryption.isDeflated) + await license.decryptFully(data: data, isDeflated: encryption.isDeflated) }) } @@ -102,7 +92,7 @@ final class LCPDecryptor: Sendable, Loggable { /// A LCP resource used to read content encrypted with the CBC algorithm. /// /// Supports random access for byte range requests, but the resource MUST NOT be deflated. - private final class CBCLCPResource: Resource, Sendable, Loggable { + private final class CBCLCPResource: Resource, Sendable { private let resource: Resource private let license: LCPLicense private let encryption: ReadiumShared.Encryption @@ -135,8 +125,6 @@ final class LCPDecryptor: Sendable, Loggable { private static let chunkSize: UInt64 = 256 * 1024 @concurrent func stream(range: Range?, consume: @escaping @Sendable (Data) -> Void) async -> ReadResult { - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource.stream(range: \(range.map(String.init(describing:)) ?? "nil"))") - var plainTextSize: UInt64? switch await self.plainTextSize() { case let .success(size): @@ -154,7 +142,6 @@ final class LCPDecryptor: Sendable, Loggable { guard range == nil else { return failure(.noPlainTextSize) } - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: no plaintext size, reading the FULL encrypted resource in memory before decrypting…") return await license.decryptFully(data: resource.read(), isDeflated: encryption.isDeflated) .map { consume($0) @@ -173,8 +160,6 @@ final class LCPDecryptor: Sendable, Loggable { return .failure(.decoding(LCPDecryptor.Error.requiredEstimatedLength)) } - log(.info, "[#579] \(timestamp579lcp()) CBCLCPResource: streaming range \(clampedRange) in chunks of \(Self.chunkSize) bytes") - // Decrypting in chunks lets the caller process the beginning // of the resource without waiting for the whole range, which // matters when streaming a large track from the network. diff --git a/Sources/Navigator/Audiobook/AudioNavigator.swift b/Sources/Navigator/Audiobook/AudioNavigator.swift index 60324e6377..90522c63de 100644 --- a/Sources/Navigator/Audiobook/AudioNavigator.swift +++ b/Sources/Navigator/Audiobook/AudioNavigator.swift @@ -233,7 +233,6 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo await go(to: link) } } - log(.info, "[#579] \(timestamp579()) play(): calling playImmediately(atRate: \(settings.speed)) with automaticallyWaitsToMinimizeStalling=\(player.automaticallyWaitsToMinimizeStalling)") player.playImmediately(atRate: Float(settings.speed)) } } @@ -308,7 +307,6 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo ) { [weak self] time in MainActor.assumeIsolated { guard let self = self else { return } - self.log(.info, "[#579] \(timestamp579()) periodic tick: time=\(time.secondsOrZero), reportedState=\(self.state), rate=\(self.player.rate), bufferEmpty=\(self.player.currentItem?.isPlaybackBufferEmpty.description ?? "nil"), likelyToKeepUp=\(self.player.currentItem?.isPlaybackLikelyToKeepUp.description ?? "nil")") self.locationDidChange() } } @@ -332,8 +330,7 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo } } - timeControlStatusObserver = player.observe(\.timeControlStatus, options: [.new, .old]) { [weak self] player, _ in - self?.log(.info, "[#579] \(timestamp579()) timeControlStatus changed to \(player.timeControlStatus.debugLabel) (rate=\(player.rate), reasonForWaitingToPlay=\(player.reasonForWaitingToPlay?.rawValue ?? "nil"), bufferEmpty=\(player.currentItem?.isPlaybackBufferEmpty.description ?? "nil"), likelyToKeepUp=\(player.currentItem?.isPlaybackLikelyToKeepUp.description ?? "nil"))") + timeControlStatusObserver = player.observe(\.timeControlStatus, options: [.new, .old]) { [weak self] _, _ in Task { @MainActor [weak self] in self?.playbackDidChange() } @@ -634,18 +631,6 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo } } -extension AVPlayer.TimeControlStatus { - /// Debug helper for the [#579] diagnostic log events. - var debugLabel: String { - switch self { - case .paused: return "paused" - case .waitingToPlayAtSpecifiedRate: return "waitingToPlayAtSpecifiedRate" - case .playing: return "playing" - @unknown default: return "unknown(\(rawValue))" - } - } -} - private extension MediaPlaybackState { init(_ timeControlStatus: AVPlayer.TimeControlStatus) { switch timeControlStatus { diff --git a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift index 67cffbd86a..175470f5c3 100644 --- a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift +++ b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift @@ -142,8 +142,6 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log using resource: Resource, link: Link ) { - log(.info, "[#579] \(timestamp579()) contentInformationRequest for \(link.href)") - tasks.add { [self] in infoRequest.isByteRangeAccessSupported = true infoRequest.contentType = link.mediaType?.uti @@ -151,7 +149,6 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log switch await resource.length() { case let .success(length): infoRequest.contentLength = Int64(length) - log(.info, "[#579] \(timestamp579()) contentInformationRequest fulfilled: contentLength=\(length), contentType=\(infoRequest.contentType ?? "nil"), byteRangeAccessSupported=true") request.finishLoading() case let .failure(error): @@ -170,28 +167,17 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log range = UInt64(dataRequest.currentOffset) ..< (UInt64(dataRequest.currentOffset) + UInt64(dataRequest.requestedLength)) } - log(.info, "[#579] \(timestamp579()) dataRequest for \(link.href): currentOffset=\(dataRequest.currentOffset), requestedLength=\(dataRequest.requestedLength), requestsAllDataToEndOfResource=\(dataRequest.requestsAllDataToEndOfResource) -> range=\(range.map(String.init(describing:)) ?? "nil (full resource)")") - let task = Task { [self] in - var consumedBytes = 0 - var consumeCalls = 0 let result = await resource.stream( range: range, - consume: { - consumedBytes += $0.count - consumeCalls += 1 - log(.info, "[#579] \(timestamp579()) consume #\(consumeCalls): chunk=\($0.count) bytes, total=\(consumedBytes) bytes") - dataRequest.respond(with: $0) - } + consume: { dataRequest.respond(with: $0) } ) queue.async { [weak self] in switch result { case .success: - self?.log(.info, "[#579] \(timestamp579()) dataRequest finished: \(consumeCalls) consume call(s), \(consumedBytes) bytes total") request.finishLoading() case let .failure(error): - self?.log(.info, "[#579] \(timestamp579()) dataRequest failed after \(consumeCalls) consume call(s), \(consumedBytes) bytes: \(error)") self?.report(error, forHREF: link.url()) request.finishLoading(with: error) } @@ -213,18 +199,10 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log } func resourceLoader(_ resourceLoader: AVAssetResourceLoader, didCancel loadingRequest: AVAssetResourceLoadingRequest) { - log(.info, "[#579] \(timestamp579()) didCancel loadingRequest: offset=\(loadingRequest.dataRequest?.currentOffset ?? -1)") finishRequest(loadingRequest) } } -/// Timestamp helper to correlate the [#579] diagnostic log events. -func timestamp579() -> String { - String(format: "t=%.3fs", CFAbsoluteTimeGetCurrent() - loadingStartTime579) -} - -private let loadingStartTime579 = CFAbsoluteTimeGetCurrent() - private let schemePrefix = "readium" extension URL { diff --git a/TestApp/Sources/Library/LibraryService.swift b/TestApp/Sources/Library/LibraryService.swift index 128542b2c3..88ad0f6093 100644 --- a/TestApp/Sources/Library/LibraryService.swift +++ b/TestApp/Sources/Library/LibraryService.swift @@ -108,9 +108,9 @@ import UIKit } var url = url -// if let file = url.fileURL { -// url = try await fulfillIfNeeded(file, progress: progress) -// } + if let file = url.fileURL { + url = try await fulfillIfNeeded(file, progress: progress) + } let (pub, format) = try await openPublication(at: url, allowUserInteraction: false) let title = pub.metadata.title ?? url.url.deletingPathExtension().lastPathComponent From c1d57d841db90c1b25b4020864e4b7208566a34a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 13:19:10 +0200 Subject: [PATCH 07/11] Keep media resources cached across loading requests PublicationMediaLoader evicted the cached resource as soon as its last loading request finished. But AVPlayer routinely abandons a data request to reissue a new one for the same entry, and rebuilding the resource threw away the buffered data and the cached plaintext size, re-downloading the beginning of the track. The resources of other entries are still evicted, e.g. when switching tracks. Part of #579. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 +- .../Navigator/Audiobook/PublicationMediaLoader.swift | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index baa289fe3c..f4ca61894a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,7 +97,7 @@ All notable changes to this project will be documented in this file. Take a look * Fixed custom `EditingAction`s sometimes missing from the text-selection menu for double-tap (single word) selections (contributed by [@raphi011](https://github.com/readium/swift-toolkit/pull/822)). * Fixed memory leak in the `AudioNavigator`. * [#802](https://github.com/readium/swift-toolkit/issues/802) Fixed fonts declared with `fontFamilyDeclarations` never loading in the EPUB navigator. Font fetches were CORS-gated by WebKit (contributed by [@atani](https://github.com/readium/swift-toolkit/pull/845)). -* [#579](https://github.com/readium/swift-toolkit/issues/579) The `AudioNavigator` now reports the `.loading` state while the player is stalled on an empty buffer, and forwards resource loading errors to `NavigatorDelegate.navigator(_:didFailToLoadResourceAt:withError:)` instead of swallowing them. +* [#579](https://github.com/readium/swift-toolkit/issues/579) The `AudioNavigator` now reports the `.loading` state while the player is stalled on an empty buffer, and forwards resource loading errors to `NavigatorDelegate.navigator(_:didFailToLoadResourceAt:withError:)` instead of swallowing them. It also keeps the current resource cached across loading requests, instead of re-downloading the beginning of a track whenever the player reissues a request. #### LCP diff --git a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift index 175470f5c3..7dbf6bec3d 100644 --- a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift +++ b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift @@ -67,6 +67,13 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log else { return nil } + + // Only the resources of other entries are evicted, as the player + // routinely abandons its requests to issue new ones for the same + // entry. Dropping the current resource would throw away its buffered + // data and force re-downloading the beginning of the entry. + resources = resources.filter { requests[$0.key] != nil } + resources[href] = (link, resource) return (link, resource) } @@ -103,8 +110,9 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log let req = reqs.remove(at: index) req.task.cancel() + // The resource is intentionally kept in `resources`, to reuse its + // buffered data with the next loading requests for the same entry. if reqs.isEmpty { - resources.removeValue(forKey: href) requests.removeValue(forKey: href) } else { requests[href] = reqs From 5828fb1623ad49d3e11eb559f6e1f7d58794cb8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 10 Jul 2026 14:10:08 +0200 Subject: [PATCH 08/11] Do not report cancelled HTTP reads as loading errors When the player abandoned a data request while streaming, the cancelled HTTP read surfaced through the ZIP layer as .decoding(ReadError.access(.http(.cancelled))) and was forwarded to NavigatorDelegate.didFailToLoadResourceAt, even though nothing failed. ReadError.wrap() now passes through errors that are already ReadErrors instead of re-wrapping them in .decoding, and PublicationMediaLoader filters cancellations with the new ReadError.isCancellation helper, which also recognizes cancelled HTTP requests and CancellationErrors nested in .decoding. Part of #579. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 1 + .../Audiobook/PublicationMediaLoader.swift | 2 +- Sources/Shared/Toolkit/Data/ReadError.swift | 18 ++++++++++ .../Toolkit/Data/ReadErrorTests.swift | 33 +++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index f4ca61894a..f7da37201b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ All notable changes to this project will be documented in this file. Take a look * EPUB HREFs that are not percent-encoded but carry a fragment or query (e.g. `chapter one.xhtml#section`, with a space in the filename) now keep their `#fragment`/`?query` instead of encoding the separators into the path. This fixes table of contents and Media Overlays links failing to resolve and navigate in poorly-authored EPUBs. * [#579](https://github.com/readium/swift-toolkit/issues/579) Reading a range past the end of a ZIP entry now returns the clamped bytes instead of failing, per the `Streamable` contract. `BufferingResource` also no longer extends its read-ahead past the end of the resource, which HTTP servers reject with a 416 error. +* `ReadError.wrap()` now passes through errors that are already `ReadError`s, instead of obscuring them in a `.decoding` case. A new `ReadError.isCancellation` helper identifies errors caused by a cancelled task or HTTP request. #### Navigator diff --git a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift index 7dbf6bec3d..46bab3833c 100644 --- a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift +++ b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift @@ -200,7 +200,7 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log private func report(_ error: ReadError, forHREF href: AnyURL) { // Cancellation is not an error worth reporting, it occurs whenever // the player abandons a data request, e.g. when seeking. - if case .cancelled = error { + guard !error.isCancellation else { return } onLoadingError?(href, error) diff --git a/Sources/Shared/Toolkit/Data/ReadError.swift b/Sources/Shared/Toolkit/Data/ReadError.swift index 58cfa64379..54ebca91e1 100644 --- a/Sources/Shared/Toolkit/Data/ReadError.swift +++ b/Sources/Shared/Toolkit/Data/ReadError.swift @@ -43,6 +43,8 @@ public enum ReadError: Error, Sendable { /// Returns `nil` if the error cannot be mapped to a known `ReadError`. public static func wrap(_ error: Error) -> ReadError? { switch error { + case let error as ReadError: + return error case is CancellationError: return .cancelled case let error as CocoaError: @@ -130,6 +132,22 @@ public enum ReadError: Error, Sendable { } } +public extension ReadError { + /// Indicates whether the error is caused by a cancelled task or HTTP + /// request, instead of a genuine failure. + var isCancellation: Bool { + switch self { + case .cancelled, .access(.http(.cancelled)): + return true + case let .decoding(error): + return (error as? ReadError)?.isCancellation + ?? (error is CancellationError) + default: + return false + } + } +} + public enum AccessError: Error, Sendable { /// An error occurred while accessing content over HTTP. case http(HTTPError) diff --git a/Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift b/Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift new file mode 100644 index 0000000000..365c5f8e4e --- /dev/null +++ b/Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift @@ -0,0 +1,33 @@ +// +// Copyright 2026 Readium Foundation. All rights reserved. +// Use of this source code is governed by the BSD-style license +// available in the top-level LICENSE file of the project. +// + +@testable import ReadiumShared +import XCTest + +class ReadErrorTests: XCTestCase { + /// A `ReadError` is passed through as-is, instead of being wrapped in + /// another `ReadError` losing its semantics. + func testWrapPassesThroughReadError() { + let error: Error = ReadError.access(.http(.cancelled)) + XCTAssertEqual(ReadError.wrap(error), .access(.http(.cancelled))) + } + + func testWrapCancellationError() { + XCTAssertEqual(ReadError.wrap(CancellationError()), .cancelled) + } + + func testIsCancellation() { + XCTAssertTrue(ReadError.cancelled.isCancellation) + XCTAssertTrue(ReadError.access(.http(.cancelled)).isCancellation) + XCTAssertTrue(ReadError.decoding(ReadError.cancelled).isCancellation) + XCTAssertTrue(ReadError.decoding(ReadError.access(.http(.cancelled))).isCancellation) + XCTAssertTrue(ReadError.decoding(CancellationError()).isCancellation) + + XCTAssertFalse(ReadError.decoding("Invalid data").isCancellation) + XCTAssertFalse(ReadError.access(.http(.rangeNotSupported)).isCancellation) + XCTAssertFalse(ReadError.access(.fileSystem(.fileNotFound(nil))).isCancellation) + } +} From 4931f136e428956b0fd0f083dc53e58367672250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 11 Sep 2026 15:21:01 +0200 Subject: [PATCH 09/11] Fixes --- .../Navigator/Audiobook/AudioNavigator.swift | 24 ++++++++++--------- Tests/LCPTests/LCPDecryptionTests.swift | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Sources/Navigator/Audiobook/AudioNavigator.swift b/Sources/Navigator/Audiobook/AudioNavigator.swift index 90522c63de..17e654f7ce 100644 --- a/Sources/Navigator/Audiobook/AudioNavigator.swift +++ b/Sources/Navigator/Audiobook/AudioNavigator.swift @@ -304,7 +304,7 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo preferredTimescale: 1000 ), queue: .main - ) { [weak self] time in + ) { [weak self] _ in MainActor.assumeIsolated { guard let self = self else { return } self.locationDidChange() @@ -336,11 +336,11 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo } } - currentItemObserver = player.observe(\.currentItem, options: [.new, .old]) { [weak self] _, _ in + currentItemObserver = player.observe(\.currentItem, options: [.new, .old]) { [weak self] player, _ in + let item = player.currentItem Task { @MainActor [weak self] in - guard let self = self else { return } - self.observe(currentItem: self.player.currentItem) - self.playbackDidChange() + self?.observe(currentItem: item) + self?.playbackDidChange() } } @@ -367,20 +367,22 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo private func observe(currentItem item: AVPlayerItem?) { itemLikelyToKeepUpObserver = item?.observe(\.isPlaybackLikelyToKeepUp) { [weak self] _, _ in - self?.playbackDidChange() + Task { @MainActor [weak self] in + self?.playbackDidChange() + } } itemStatusObserver = item?.observe(\.status) { [weak self] item, _ in - guard let self = self, item.status == .failed else { + guard item.status == .failed else { return } let itemError = item.error - log(.error, "Failed to load the player item: \(String(describing: itemError))") - let href = publication.readingOrder[resourceIndex].url().relativeURL - Task { @MainActor in - guard let href = href else { + Task { @MainActor [weak self] in + guard let self = self else { return } + log(.error, "Failed to load the player item: \(String(describing: itemError))") + guard let href = self.publication.readingOrder[self.resourceIndex].url().relativeURL else { return } let error: ReadError = itemError.flatMap { .wrap($0) } diff --git a/Tests/LCPTests/LCPDecryptionTests.swift b/Tests/LCPTests/LCPDecryptionTests.swift index 2fb6a7470b..975fa923d6 100644 --- a/Tests/LCPTests/LCPDecryptionTests.swift +++ b/Tests/LCPTests/LCPDecryptionTests.swift @@ -86,7 +86,7 @@ struct LCPDecryptionTests { /// Cancelling the task stops the decryption loop, instead of streaming /// the remaining chunks. - @Test func cancellationStopsStreaming() async throws { + @Test func cancellationStopsStreaming() async { var chunks: [Data] = [] let result = await encryptedResource.stream(range: 0 ..< UInt64(clearData.count)) { chunk in chunks.append(chunk) From c53ce5e3e4aab2bf832b2c3110d976e3b3bbd831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 11 Sep 2026 16:36:43 +0200 Subject: [PATCH 10/11] Fixes --- CHANGELOG.md | 8 +----- .../Navigator/Audiobook/AudioNavigator.swift | 13 ++++++++-- Tests/LCPTests/Capture.swift | 19 ++++++++++++++ Tests/LCPTests/LCPDecryptionTests.swift | 25 +++++++++++-------- 4 files changed, 46 insertions(+), 19 deletions(-) create mode 100644 Tests/LCPTests/Capture.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index f7da37201b..335ae618b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ All notable changes to this project will be documented in this file. Take a look #### LCP * The CRL used to validate LCP licenses is now checked to be a genuine X.509 CRL before being cached. Networks with a captive portal (e.g. on a plane) could return their login page with a `200 OK` status, which was then cached for seven days and prevented opening LCP publications. An invalid CRL cached by a previous version is now ignored instead of waiting for its expiration. +* [#579](https://github.com/readium/swift-toolkit/issues/579) Streamed LCP audiobooks now start playing almost immediately. Resources encrypted with AES-CBC are decrypted and served in chunks, instead of being fully downloaded and decrypted upfront. ## [4.0.0-alpha.1] - 2026-08-14 @@ -90,19 +91,12 @@ All notable changes to this project will be documented in this file. Take a look #### Shared * EPUB HREFs that are not percent-encoded but carry a fragment or query (e.g. `chapter one.xhtml#section`, with a space in the filename) now keep their `#fragment`/`?query` instead of encoding the separators into the path. This fixes table of contents and Media Overlays links failing to resolve and navigate in poorly-authored EPUBs. -* [#579](https://github.com/readium/swift-toolkit/issues/579) Reading a range past the end of a ZIP entry now returns the clamped bytes instead of failing, per the `Streamable` contract. `BufferingResource` also no longer extends its read-ahead past the end of the resource, which HTTP servers reject with a 416 error. -* `ReadError.wrap()` now passes through errors that are already `ReadError`s, instead of obscuring them in a `.decoding` case. A new `ReadError.isCancellation` helper identifies errors caused by a cancelled task or HTTP request. #### Navigator * Fixed custom `EditingAction`s sometimes missing from the text-selection menu for double-tap (single word) selections (contributed by [@raphi011](https://github.com/readium/swift-toolkit/pull/822)). * Fixed memory leak in the `AudioNavigator`. * [#802](https://github.com/readium/swift-toolkit/issues/802) Fixed fonts declared with `fontFamilyDeclarations` never loading in the EPUB navigator. Font fetches were CORS-gated by WebKit (contributed by [@atani](https://github.com/readium/swift-toolkit/pull/845)). -* [#579](https://github.com/readium/swift-toolkit/issues/579) The `AudioNavigator` now reports the `.loading` state while the player is stalled on an empty buffer, and forwards resource loading errors to `NavigatorDelegate.navigator(_:didFailToLoadResourceAt:withError:)` instead of swallowing them. It also keeps the current resource cached across loading requests, instead of re-downloading the beginning of a track whenever the player reissues a request. - -#### LCP - -* [#579](https://github.com/readium/swift-toolkit/issues/579) Streamed LCP audiobooks now start playing almost immediately. Resources encrypted with AES-CBC are decrypted and served in chunks, instead of being fully downloaded and decrypted upfront. ## [3.10.0] - 2026-06-24 diff --git a/Sources/Navigator/Audiobook/AudioNavigator.swift b/Sources/Navigator/Audiobook/AudioNavigator.swift index 17e654f7ce..4f2bd3be78 100644 --- a/Sources/Navigator/Audiobook/AudioNavigator.swift +++ b/Sources/Navigator/Audiobook/AudioNavigator.swift @@ -170,6 +170,15 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo return state } + /// Indicates whether the player is meant to be playing, even if it is + /// currently stalled waiting for data. + /// + /// Unlike `state`, this reflects the playback intent, which is what we + /// need to know when temporarily pausing the player to seek. + private var isPlaybackRequested: Bool { + player.timeControlStatus != .paused + } + /// Current playback info. public var playbackInfo: MediaPlaybackInfo { MediaPlaybackInfo( @@ -254,7 +263,7 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo /// Seeks to the given time in the current resource. public func seek(to time: Double) async { - let wasPlaying = (state == .playing) + let wasPlaying = isPlaybackRequested pause() await player.seek(to: CMTime(seconds: time, preferredTimescale: 1000)) @@ -512,7 +521,7 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo public private(set) var currentLocation: Locator? public func go(to locator: Locator, options: NavigatorGoOptions) async -> Bool { - let wasPlaying = (state == .playing) + let wasPlaying = isPlaybackRequested pause() guard let newResourceIndex = publication.readingOrder.firstIndexWithHREF(locator.href) else { diff --git a/Tests/LCPTests/Capture.swift b/Tests/LCPTests/Capture.swift new file mode 100644 index 0000000000..14042c76ab --- /dev/null +++ b/Tests/LCPTests/Capture.swift @@ -0,0 +1,19 @@ +// +// Copyright 2026 Readium Foundation. All rights reserved. +// Use of this source code is governed by the BSD-style license +// available in the top-level LICENSE file of the project. +// + +import Foundation + +/// A reference-type wrapper that allows a value to be captured and mutated +/// inside a `@Sendable` closure. +/// +/// Warning: Not thread-safe - only for sequential test code. +final nonisolated class Capture: @unchecked Sendable { + var value: T + + init(_ value: T) { + self.value = value + } +} diff --git a/Tests/LCPTests/LCPDecryptionTests.swift b/Tests/LCPTests/LCPDecryptionTests.swift index 975fa923d6..29c0fb9c8c 100644 --- a/Tests/LCPTests/LCPDecryptionTests.swift +++ b/Tests/LCPTests/LCPDecryptionTests.swift @@ -74,30 +74,35 @@ struct LCPDecryptionTests { /// caller can process the beginning of the resource without waiting for /// the whole range. @Test func streamsLargeRangeInChunks() async throws { - var chunks: [Data] = [] + let chunks = Capture<[Data]>([]) let result = await encryptedResource.stream(range: 0 ..< UInt64(clearData.count)) { chunk in - chunks.append(chunk) + chunks.value.append(chunk) } try result.get() - #expect(chunks.count > 1) - #expect(chunks.reduce(Data(), +) == clearData) + #expect(chunks.value.count > 1) + #expect(chunks.value.reduce(Data(), +) == clearData) } /// Cancelling the task stops the decryption loop, instead of streaming /// the remaining chunks. @Test func cancellationStopsStreaming() async { - var chunks: [Data] = [] - let result = await encryptedResource.stream(range: 0 ..< UInt64(clearData.count)) { chunk in - chunks.append(chunk) - withUnsafeCurrentTask { $0?.cancel() } - } + let chunks = Capture<[Data]>([]) + + // The streaming runs in a child task, as cancelling the test's own + // task would abort the test instead of the decryption loop. + let result = await Task { + await encryptedResource.stream(range: 0 ..< UInt64(clearData.count)) { chunk in + chunks.value.append(chunk) + withUnsafeCurrentTask { $0?.cancel() } + } + }.value guard case .failure(.cancelled) = result else { Issue.record("Expected a cancelled failure, got \(result)") return } - #expect(chunks.count == 1) + #expect(chunks.value.count == 1) } /// Reproduces the arithmetic overflow in From 097b77d250e01b9770ccfa254b569e7c3dca9c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Fri, 11 Sep 2026 17:41:43 +0200 Subject: [PATCH 11/11] Fix cancellation --- .../LCP/Content Protection/LCPDecryptor.swift | 20 ++++++----- .../Audiobook/PublicationMediaLoader.swift | 8 ++++- Sources/Shared/Toolkit/Data/ReadError.swift | 33 ++++++++----------- .../Shared/Toolkit/HTTP/HTTPResource.swift | 2 +- Sources/Shared/Toolkit/HTTP/HTTPServer.swift | 4 +-- .../Toolkit/Data/ReadErrorTests.swift | 33 ------------------- 6 files changed, 35 insertions(+), 65 deletions(-) delete mode 100644 Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift diff --git a/Sources/LCP/Content Protection/LCPDecryptor.swift b/Sources/LCP/Content Protection/LCPDecryptor.swift index 3b7617599e..d028faf6d7 100644 --- a/Sources/LCP/Content Protection/LCPDecryptor.swift +++ b/Sources/LCP/Content Protection/LCPDecryptor.swift @@ -135,7 +135,7 @@ final class LCPDecryptor: Sendable { } } - guard let plainTextSize = plainTextSize else { + guard let plainTextSize else { // Without the plaintext size, we can't compute the chunks to // decrypt; fall back on reading and decrypting the whole // resource in one shot. @@ -198,8 +198,9 @@ final class LCPDecryptor: Sendable { return failure(.invalidRange(range)) } - // Encrypted data is shifted by AESBlockSize, because of IV and because the - // previous block must be provided to perform XOR on intermediate blocks. + // Encrypted data is shifted by AESBlockSize, because of IV and + // because the previous block must be provided to perform XOR on + // intermediate blocks. let encryptedStart = rangeFirst.floorMultiple(of: AESBlockSize) let encryptedEndExclusive = min( (rangeLast + 1).ceilMultiple(of: AESBlockSize) + AESBlockSize, @@ -213,18 +214,21 @@ final class LCPDecryptor: Sendable { return failure(.emptyDecryptedData) } - // Exclude the bytes added to match a multiple of AESBlockSize. + // Exclude the bytes added to match a multiple of + // AESBlockSize. let sliceStart = (rangeFirst - encryptedStart) let isLastBlockRead = encryptedLength - encryptedEndExclusive <= AESBlockSize let rangeLength = isLastBlockRead - // Use decrypted length to ensure `rangeLast` doesn't exceed decrypted length - 1. + // Use decrypted length to ensure `rangeLast` + // doesn't exceed decrypted length - 1. ? min(rangeLast, plainTextSize - 1) - rangeFirst + 1 - // The last block won't be read, so there's no need to compute the length + // The last block won't be read, so there's no need + // to compute the length : rangeLast - rangeFirst + 1 - // Keep only enough bytes to fit the length-corrected request in order to never - // include padding. + // Keep only enough bytes to fit the length-corrected + // request in order to never include padding. let sliceEnd = sliceStart + rangeLength return .success(bytes[sliceStart ..< sliceEnd]) diff --git a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift index 46bab3833c..a886f5b4b3 100644 --- a/Sources/Navigator/Audiobook/PublicationMediaLoader.swift +++ b/Sources/Navigator/Audiobook/PublicationMediaLoader.swift @@ -181,6 +181,12 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log consume: { dataRequest.respond(with: $0) } ) + // The player abandons data requests regularly, e.g. when seeking. + // There's nothing to report or finish in this case. + guard !Task.isCancelled else { + return + } + queue.async { [weak self] in switch result { case .success: @@ -200,7 +206,7 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate, Log private func report(_ error: ReadError, forHREF href: AnyURL) { // Cancellation is not an error worth reporting, it occurs whenever // the player abandons a data request, e.g. when seeking. - guard !error.isCancellation else { + if case .cancelled = error { return } onLoadingError?(href, error) diff --git a/Sources/Shared/Toolkit/Data/ReadError.swift b/Sources/Shared/Toolkit/Data/ReadError.swift index 54ebca91e1..36f1ae6fe0 100644 --- a/Sources/Shared/Toolkit/Data/ReadError.swift +++ b/Sources/Shared/Toolkit/Data/ReadError.swift @@ -38,6 +38,18 @@ public enum ReadError: Error, Sendable { .decoding(DebugError(message, cause: cause)) } + /// Converts an ``HTTPError`` into a ``ReadError``. + /// + /// A cancelled request is normalized to ``cancelled``, so that callers + /// don't need to check for cancellation in two different places. + public init(_ error: HTTPError) { + if case .cancelled = error { + self = .cancelled + } else { + self = .access(.http(error)) + } + } + /// Wraps a native error into a `ReadError`, if possible. /// /// Returns `nil` if the error cannot be mapped to a known `ReadError`. @@ -53,10 +65,7 @@ public enum ReadError: Error, Sendable { return wrap(error) default: if let error = HTTPError.wrap(error) { - if case .cancelled = error { - return .cancelled - } - return .access(.http(error)) + return ReadError(error) } else { return nil } @@ -132,22 +141,6 @@ public enum ReadError: Error, Sendable { } } -public extension ReadError { - /// Indicates whether the error is caused by a cancelled task or HTTP - /// request, instead of a genuine failure. - var isCancellation: Bool { - switch self { - case .cancelled, .access(.http(.cancelled)): - return true - case let .decoding(error): - return (error as? ReadError)?.isCancellation - ?? (error is CancellationError) - default: - return false - } - } -} - public enum AccessError: Error, Sendable { /// An error occurred while accessing content over HTTP. case http(HTTPError) diff --git a/Sources/Shared/Toolkit/HTTP/HTTPResource.swift b/Sources/Shared/Toolkit/HTTP/HTTPResource.swift index 17f8f70479..31bfbb0627 100644 --- a/Sources/Shared/Toolkit/HTTP/HTTPResource.swift +++ b/Sources/Shared/Toolkit/HTTP/HTTPResource.swift @@ -107,6 +107,6 @@ public actor HTTPResource: Resource { } ) .map { _ in () } - .mapError { .access(.http($0)) } + .mapError { ReadError($0) } } } diff --git a/Sources/Shared/Toolkit/HTTP/HTTPServer.swift b/Sources/Shared/Toolkit/HTTP/HTTPServer.swift index b0d70493b5..73b3154bd1 100644 --- a/Sources/Shared/Toolkit/HTTP/HTTPServer.swift +++ b/Sources/Shared/Toolkit/HTTP/HTTPServer.swift @@ -86,7 +86,7 @@ public extension HTTPServer { let link = publication.linkWithHREF(href), let resource = publication.get(href) else { - onFailure?(request, .access(.http(notFound))) + onFailure?(request, ReadError(notFound)) return HTTPServerResponse(error: notFound) } @@ -136,7 +136,7 @@ public struct HTTPServerResponse: Sendable { public init(error: HTTPError) { self.init( - resource: FailureResource(error: .access(.http(error))), + resource: FailureResource(error: ReadError(error)), mediaType: nil ) } diff --git a/Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift b/Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift deleted file mode 100644 index 365c5f8e4e..0000000000 --- a/Tests/SharedTests/Toolkit/Data/ReadErrorTests.swift +++ /dev/null @@ -1,33 +0,0 @@ -// -// Copyright 2026 Readium Foundation. All rights reserved. -// Use of this source code is governed by the BSD-style license -// available in the top-level LICENSE file of the project. -// - -@testable import ReadiumShared -import XCTest - -class ReadErrorTests: XCTestCase { - /// A `ReadError` is passed through as-is, instead of being wrapped in - /// another `ReadError` losing its semantics. - func testWrapPassesThroughReadError() { - let error: Error = ReadError.access(.http(.cancelled)) - XCTAssertEqual(ReadError.wrap(error), .access(.http(.cancelled))) - } - - func testWrapCancellationError() { - XCTAssertEqual(ReadError.wrap(CancellationError()), .cancelled) - } - - func testIsCancellation() { - XCTAssertTrue(ReadError.cancelled.isCancellation) - XCTAssertTrue(ReadError.access(.http(.cancelled)).isCancellation) - XCTAssertTrue(ReadError.decoding(ReadError.cancelled).isCancellation) - XCTAssertTrue(ReadError.decoding(ReadError.access(.http(.cancelled))).isCancellation) - XCTAssertTrue(ReadError.decoding(CancellationError()).isCancellation) - - XCTAssertFalse(ReadError.decoding("Invalid data").isCancellation) - XCTAssertFalse(ReadError.access(.http(.rangeNotSupported)).isCancellation) - XCTAssertFalse(ReadError.access(.fileSystem(.fileNotFound(nil))).isCancellation) - } -}