From b91ad99235b56bf5b44c549efcee7d45460eb0e1 Mon Sep 17 00:00:00 2001 From: Simon Woolf Date: Wed, 29 Jul 2026 11:06:36 +0100 Subject: [PATCH] Read the channel's data encoder live when encoding annotations ARTRestAnnotationsInternal and ARTRealtimeAnnotationsInternal each captured _channel.dataEncoder into an ivar at construction. ARTChannel replaces its _dataEncoder with a new instance whenever setOptions is called (recreateDataEncoderWith:), and there is no hook to refresh the annotations objects' copies, so after a setOptions that added or changed cipher params the annotations objects kept using the encoder built at channel-creation time. Annotation data would then be published and decoded with the stale cipher config (typically none at all) while messages, which read the channel's dataEncoder property live, correctly picked up the new cipher. Read _channel.dataEncoder at each encode/decode site instead of caching it, matching ARTChannel's encodeMessageIfNeeded: and ARTRealtimePresence's publish path. The REST decode path already read it live. Co-Authored-By: Claude Opus 5 (1M context) --- Source/ARTRealtimeAnnotations.m | 10 +-- Source/ARTRestAnnotations.m | 5 +- .../Tests/RealtimeAnnotationsTests.swift | 71 +++++++++++++++++++ 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/Source/ARTRealtimeAnnotations.m b/Source/ARTRealtimeAnnotations.m index 62dee31f1..57881450e 100644 --- a/Source/ARTRealtimeAnnotations.m +++ b/Source/ARTRealtimeAnnotations.m @@ -98,7 +98,6 @@ @implementation ARTRealtimeAnnotationsInternal { __weak ARTRealtimeInternal *_realtime; dispatch_queue_t _userQueue; ARTEventEmitter *_eventEmitter; - ARTDataEncoder *_dataEncoder; } - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(ARTInternalLog *)logger { @@ -109,7 +108,6 @@ - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(AR _queue = _realtime.rest.queue; _logger = logger; _eventEmitter = [[ARTInternalEventEmitter alloc] initWithQueue:_queue timeProvider:_realtime.rest.timeProvider]; - _dataEncoder = _channel.dataEncoder; } return self; } @@ -150,7 +148,8 @@ - (void)publishAnnotation:(ARTOutboundAnnotation *)outboundAnnotation extras:outboundAnnotation.extras]; art_dispatch_sync(_queue, ^{ NSError *error = nil; - ARTAnnotation *annotationToPublish = _dataEncoder ? [annotation encodeDataWithEncoder:_dataEncoder error:&error] : annotation; // RSAN1c3 + ARTDataEncoder *dataEncoder = self->_channel.dataEncoder; + ARTAnnotation *annotationToPublish = dataEncoder ? [annotation encodeDataWithEncoder:dataEncoder error:&error] : annotation; // RSAN1c3 if (error) { if (callback) { callback([ARTErrorInfo createFromNSError:error]); @@ -301,11 +300,12 @@ - (void)unsubscribe:(NSString *)type listener:(ARTEventListener *)listener { } - (void)onMessage:(ARTProtocolMessage *)message { + ARTDataEncoder *dataEncoder = _channel.dataEncoder; for (ARTAnnotation *a in message.annotations) { ARTAnnotation *annotation = a; - if (annotation.data && _dataEncoder) { + if (annotation.data && dataEncoder) { NSError *decodeError = nil; - annotation = [a decodeDataWithEncoder:_dataEncoder error:&decodeError]; + annotation = [a decodeDataWithEncoder:dataEncoder error:&decodeError]; if (decodeError != nil) { ARTErrorInfo *errorInfo = [ARTErrorInfo wrap:[ARTErrorInfo createWithCode:ARTErrorUnableToDecodeMessage message:decodeError.localizedFailureReason] prepend:@"Failed to decode data: "]; ARTLogError(self.logger, @"RT:%p C:%p (%@) %@", _realtime, _channel, _channel.name, errorInfo.message); diff --git a/Source/ARTRestAnnotations.m b/Source/ARTRestAnnotations.m index 2517cd4b2..6c17f843d 100644 --- a/Source/ARTRestAnnotations.m +++ b/Source/ARTRestAnnotations.m @@ -100,7 +100,6 @@ @implementation ARTRestAnnotationsInternal { __weak ARTRestChannelInternal *_channel; // weak because channel owns self dispatch_queue_t _userQueue; dispatch_queue_t _queue; - ARTDataEncoder *_dataEncoder; } - (instancetype)initWithChannel:(ARTRestChannelInternal *)channel logger:(ARTInternalLog *)logger { @@ -109,7 +108,6 @@ - (instancetype)initWithChannel:(ARTRestChannelInternal *)channel logger:(ARTInt _userQueue = channel.rest.userQueue; _queue = channel.rest.queue; _logger = logger; - _dataEncoder = _channel.dataEncoder; } return self; } @@ -158,7 +156,8 @@ - (void)publishAnnotation:(ARTOutboundAnnotation *)outboundAnnotation art_dispatch_async(_queue, ^{ // RSAN1c3: encode annotation data NSError *encodeError = nil; - ARTAnnotation *annotationToPublish = self->_dataEncoder ? [annotation encodeDataWithEncoder:self->_dataEncoder error:&encodeError] : annotation; + ARTDataEncoder *dataEncoder = self->_channel.dataEncoder; + ARTAnnotation *annotationToPublish = dataEncoder ? [annotation encodeDataWithEncoder:dataEncoder error:&encodeError] : annotation; if (encodeError) { if (callback) { callback([ARTErrorInfo createFromNSError:encodeError]); diff --git a/Test/AblyTests/Tests/RealtimeAnnotationsTests.swift b/Test/AblyTests/Tests/RealtimeAnnotationsTests.swift index 413f767bd..2e97cb73a 100644 --- a/Test/AblyTests/Tests/RealtimeAnnotationsTests.swift +++ b/Test/AblyTests/Tests/RealtimeAnnotationsTests.swift @@ -379,4 +379,75 @@ class RealtimeAnnotationsTests: XCTestCase { } } } + + // The annotations object must read the channel's data encoder at encode/decode time + // rather than capturing it at construction, so that a cipher added by setOptions + // after the channel was created is still applied to annotation data. + func test__annotation_data_uses_a_cipher_added_by_setOptions_after_channel_creation() throws { + let test = Test() + let options = try AblyTests.commonAppSetup(for: test) + options.testOptions.channelNamePrefix = nil + + let realtimeClient = ARTRealtime(options: options) + defer { realtimeClient.dispose(); realtimeClient.close() } + + let channelName = test.uniqueChannelName(prefix: "mutable:") + + // Create the channel with no cipher, so the annotations object would capture a + // cipher-less encoder if it cached one. + let initialOptions = ARTRealtimeChannelOptions() + initialOptions.modes = [.publish, .subscribe, .annotationPublish, .annotationSubscribe] + let channel = realtimeClient.channels.get(channelName, options: initialOptions) + + // Now add a cipher. + let key = ARTCrypto.generateRandomKey() + let encryptedOptions = ARTRealtimeChannelOptions(cipherKey: key as ARTCipherKeyCompatible) + encryptedOptions.modes = [.publish, .subscribe, .annotationPublish, .annotationSubscribe] + waitUntil(timeout: testTimeout) { done in + channel.setOptions(encryptedOptions) { error in + XCTAssertNil(error) + done() + } + } + + let annotationData = "secret annotation data" + var receivedAnnotation: ARTAnnotation! + + waitUntil(timeout: testTimeout) { done in + channel.subscribe { message in + guard message.action == .create else { + return + } + // multiple.v1 because an anonymous client may only publish the + // multiple.v1 and total.v1 aggregation methods + let annotation = ARTOutboundAnnotation( + id: nil, + type: "reaction:multiple.v1", + clientId: nil, + name: "👍", + count: 1, + data: annotationData, + extras: nil + ) + channel.annotations.publish(for: message, annotation: annotation) { error in + XCTAssertNil(error) + } + } + + channel.annotations.subscribe { annotation in + receivedAnnotation = annotation + done() + } + + channel.publish([ARTMessage(name: "test", data: "test message")]) + } + + // Decrypted with the cipher set by setOptions; if the stale encoder were used the + // data would have been published as plaintext and arrive as ciphertext. + guard let receivedAnnotation else { + XCTFail("No annotation received") + return + } + XCTAssertEqual(receivedAnnotation.data as? String, annotationData) + } }