Repository navigation
Decode live messages with the channel's current options - #1378
Conversation
A realtime channel decoded received messages with a decoding context built once from the options it was created with, so SetOptions and Channels.Get(name, options) never changed the cipher used for them (RTL16, RTS3c). Presence, annotations and publishing already used the current options. Update the context's options whenever the channel's options change, rather than replacing the context, which also holds the delta base payload that must survive the change (RTL19).
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 1 minute. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Update decoding options src/Ably.PubSub.Shared/Realtime/RealtimeChannel.cs, CHANGELOG.md |
The Options setter updates an existing decoding context. The constructor initializes the context from normalized options. The changelog records decoding with current channel options. |
Verify current-options decoding src/Ably.PubSub.Tests.Shared/Realtime/ChannelOptionsDecodingSpecs.cs, src/Ably.PubSub.Tests.Shared/Ably.PubSub.Tests.Shared.projitems |
The shared tests cover cipher changes through SetOptionsAsync, Channels.Get, and the Options setter. They also cover cipher removal, delta payloads, presence, and annotations. |
Push.Admin Usage Note
| Layer / File(s) | Summary |
|---|---|
Document Push.Admin requirements README.md |
The README states that Push.Admin is available on Device and Server clients, requires the push-admin capability, and is intended for use from a Server backend. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related PRs
- ably/ably-pubsub-dotnet#1372: Updates the README with the same Push.Admin availability and capability guidance.
- ably/ably-pubsub-dotnet#1371: Documents the Push.Admin contract and changes realtime history handling for encrypted channels.
- ably/ably-pubsub-dotnet#1376: Changes realtime retrieval to use current channel options and tests encrypted history and presence retrieval.
Suggested reviewers: umair-ably, ttypic
Merge Risk: 
ecdbd
-
Channels use current decoding options while retaining the existing context. No material merge risk was identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: realtime messages now decode using the channel's current options. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the cipher's key,
As channel options change with care.
New messages shed their coded coat,
Presence and notes decode just so.
Then off I hop, with bytes to spare.
Summary
A realtime channel decoded the messages it receives with a decoding context built once, from the options the channel was created with.
SetOptionsandChannels.Get(name, options)updatedOptionsbut never that context, so after a cipher change the channel published with the new cipher but still decrypted received messages with the old one, delivering them asbyte[]with a residualcipher+...encoding. Presence, annotations, publishing and REST retrieval already used the current options. The bug dates from 1.x.This PR also updates the README's
Push.Adminnote to mention thepush-admincapability, matching UPDATING.md, CHANGELOG.md and the XML docs.Fix
RealtimeChannel.Optionssetter: also setMessageDecodingContext.ChannelOptions. Every way of changing options goes through it (SetOptions,Channels.Get(name, options)on an existing channel, theAddOrUpdatepath, direct assignment).Options(the normalised value) instead of the raw argument.Tests
New
ChannelOptionsDecodingSpecs(8 tests, fake transport):integration/v2SetOptionsadding a cipherSetOptionschanging the cipherChannels.Get(name, options)on an existing channel (RTS3c)Optionssetter (AES-256 to AES-128)SetOptionswithout a cipher stops decryptionWith the context replaced instead of updated, only the delta test fails (the delta is dropped and the channel goes to
ATTACHING).Validation
All on
ecdbd77d(the pushed head):./build.sh --target=Build.NetStandard: build succeeded, 0 warnings, 0 errors../build.sh --target=Test.NetStandard.Unit.WithRetry --framework=net6.0: 1515 total, 1496 passed, 19 skipped, 0 failed.Spec and ably-java
setOptions"sets or updates the stored channel options"; with no params or modes it "indicates success immediately" (RTL16a), so a cipher-only change must apply to the next received message.Channels#get(channel, options)on an existing channel "will update the options on the channel".cipher, payloads are "decrypted when received on this channel".ChannelBasepasses the currentoptionstomsg.decode(options, decodingContext)for every message, and itsDecodingContext(delta base only) is created once and kept acrosssetOptions.Summary by CodeRabbit
Bug Fixes
Documentation
Push.Adminis intended for server-side use, requires thepush-admincapability, and is available through both device and server clients. The README recommends using it from a backend.