This is an automated email from the ASF dual-hosted git repository.
spetz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iggy.git
The following commit(s) were added to refs/heads/master by this push:
new b6a2f81c0 fix(sdk): forward unknown command codes instead of rejecting
them (#3766)
b6a2f81c0 is described below
commit b6a2f81c0537276328dc6baadc0a7399ef7b6b67
Author: Hubert Gruszecki <[email protected]>
AuthorDate: Tue Jul 28 17:23:16 2026 +0200
fix(sdk): forward unknown command codes instead of rejecting them (#3766)
An application built on this SDK cannot reach a command its SDK build
has never heard of. operation_for_code ended None => InvalidCommand,
so a code absent from COMMAND_TABLE failed at encode time, before any
byte left the process, and a server that does implement it never got
the chance to answer. COMMAND_TABLE is a protocol registry, not a
per-server capability list, so the server is the authority on whether
a code exists: an unknown code now ships as non-replicated, carrying
the code in RequestHeader.reserved, and the server answers with a
proper error if it does not know it.
The SDK no longer second-guesses that classification either. It used
to refuse a code the registry knows as replicated, but the server
re-derives the class from the same registry and denies a mismatch on
a path that commits nothing, so the client check duplicated a guard
it does not own. It was unreachable besides, since every replicated
entry resolves through from_command_code first, and it was the sole
construction of an UnknownReplicatedCommand variant nothing in the
tree consumed. operation_for_code is now infallible, the raw-command
documentation no longer explains consensus to callers, and error code
14002 is retired rather than reused.
---
core/common/src/error/iggy_error.rs | 2 --
core/integration/tests/sdk/raw.rs | 13 +++++--
core/sdk/src/clients/client.rs | 5 ++-
core/sdk/src/vsr.rs | 69 ++++++++++++++++++++++++++++++-------
4 files changed, 70 insertions(+), 19 deletions(-)
diff --git a/core/common/src/error/iggy_error.rs
b/core/common/src/error/iggy_error.rs
index 878704e0a..8d2edc9eb 100644
--- a/core/common/src/error/iggy_error.rs
+++ b/core/common/src/error/iggy_error.rs
@@ -522,8 +522,6 @@ pub enum IggyError {
AlreadyAuthenticated = 14000,
#[error("VSR session value {0} is invalid (must be non-zero)")]
InvalidSession(u64) = 14001,
- #[error("Replicated command with unknown code {0}")]
- UnknownReplicatedCommand(u32) = 14002,
/// Packed protocol versions, see `iggy_binary_protocol::ProtocolVersion`.
/// Field order: `(client_version, server_min, server_max)`.
#[error(
diff --git a/core/integration/tests/sdk/raw.rs
b/core/integration/tests/sdk/raw.rs
index cffeec954..0bf004947 100644
--- a/core/integration/tests/sdk/raw.rs
+++ b/core/integration/tests/sdk/raw.rs
@@ -87,14 +87,23 @@ async fn assert_raw_round_trip(client: &IggyClient) {
.expect_err("session-control codes must be rejected by the raw
path");
assert_eq!(error, IggyError::InvalidCommand);
- // VSR encoder is closed-world: unknown code rejected at encode
time.
+ // An SDK build cannot know which codes a server implements, so an
+ // unknown one ships as non-replicated and the server decides. The
+ // follow-up ping is the point of the case: the server must answer
+ // with a deny frame, not drop the frame and leave the connection
+ // wedged until the read timeout.
#[cfg(feature = "vsr")]
{
let error = client
.send_binary_request(60_000, Bytes::new())
.await
- .expect_err("unknown code must be rejected under VSR");
+ .expect_err("unknown code must be refused by the server");
assert_eq!(error, IggyError::InvalidCommand);
+
+ client
+ .send_binary_request(PING_CODE, PingRequest.to_bytes())
+ .await
+ .expect("connection must survive an unknown code");
}
let error = client
diff --git a/core/sdk/src/clients/client.rs b/core/sdk/src/clients/client.rs
index 98beb9830..dd7f91fd9 100644
--- a/core/sdk/src/clients/client.rs
+++ b/core/sdk/src/clients/client.rs
@@ -204,9 +204,8 @@ impl IggyClient {
/// Login and logout codes are rejected with `InvalidCommand`. Use the
/// `login_user` / `logout_user` methods so SDK session state stays
correct.
///
- /// Custom codes only work on the classic protocol. Under `vsr` the encoder
- /// is closed-world: an unknown code yields `InvalidCommand`, a replicated
- /// code with no mapping yields `UnknownReplicatedCommand`.
+ /// Custom codes are forwarded to the server, which is the authority on
+ /// whether it implements them.
pub async fn send_binary_request(&self, code: u32, payload: Bytes) ->
Result<Bytes, IggyError> {
if SESSION_CONTROL_CODES.contains(&code) {
return Err(IggyError::InvalidCommand);
diff --git a/core/sdk/src/vsr.rs b/core/sdk/src/vsr.rs
index 90a10a552..9bc5d1cd1 100644
--- a/core/sdk/src/vsr.rs
+++ b/core/sdk/src/vsr.rs
@@ -88,7 +88,7 @@ pub(crate) fn encode_request_header(
(Operation::Register, session.begin_register(), 0)
}
_ => {
- let operation = operation_for_code(code)?;
+ let operation = operation_for_code(code);
// NonReplicated ops (ping, reads) bypass server-side dedup --
// `ClientTable` only tracks request_ids for replicated ops. If
// they consumed the monotonic counter, the next replicated
@@ -153,20 +153,17 @@ pub(crate) fn encode_request_header(
Ok((header, total_size))
}
-fn operation_for_code(code: u32) -> Result<Operation, IggyError> {
+/// `COMMAND_TABLE` is a protocol registry, not a per-server capability list,
so
+/// an SDK build cannot know which codes a given server implements. The server
is
+/// the authority: an unmapped code is forwarded as non-replicated (the code
+/// rides `RequestHeader.reserved`, which that path already stamps) and the
+/// server answers with a proper error if it does not know it.
+fn operation_for_code(code: u32) -> Operation {
if code == LOGOUT_USER_CODE {
- return Ok(Operation::Logout);
+ return Operation::Logout;
}
- if let Some(operation) = Operation::from_command_code(code) {
- return Ok(operation);
- }
-
- match iggy_binary_protocol::dispatch::lookup_command(code) {
- Some(meta) if !meta.is_replicated() => Ok(Operation::NonReplicated),
- Some(_) => Err(IggyError::UnknownReplicatedCommand(code)),
- None => Err(IggyError::InvalidCommand),
- }
+ Operation::from_command_code(code).unwrap_or(Operation::NonReplicated)
}
pub(crate) fn response_size(header: &[u8]) -> Result<usize, IggyError> {
@@ -692,6 +689,54 @@ mod tests {
assert_eq!(header.session, 99);
}
+ #[test]
+ fn unknown_code_encodes_as_non_replicated_and_carries_the_code() {
+ // An extended server may implement codes this SDK build has never
heard
+ // of. The registry is not a capability list, so the request must reach
+ // the server rather than fail at encode time.
+ const UNKNOWN_CODE: u32 = 60_000;
+ assert!(
+
iggy_binary_protocol::dispatch::lookup_command(UNKNOWN_CODE).is_none(),
+ "test needs a code absent from COMMAND_TABLE"
+ );
+
+ let mut session = ConsensusSession::with_client_id(42);
+ session.bind(99);
+ let bytes = encode_contiguous_request(&mut session, UNKNOWN_CODE,
&Bytes::new()).unwrap();
+ let header = decode_request_header(&bytes);
+
+ assert_eq!(header.operation, Operation::NonReplicated);
+ assert_eq!(
+ u32::from_le_bytes(
+ header.reserved[NON_REPLICATED_CODE_RANGE]
+ .try_into()
+ .unwrap()
+ ),
+ UNKNOWN_CODE
+ );
+ }
+
+ #[test]
+ fn no_replicated_command_ever_resolves_to_non_replicated() {
+ // The safety asymmetry that makes forwarding unknown codes acceptable:
+ // an unknown code is the server's business, but a *known* replicated
+ // command sent as non-replicated would apply on one node only and
+ // silently diverge the replicas. Swept over the whole registry so a
+ // future entry cannot regress it.
+ for meta in iggy_binary_protocol::dispatch::COMMAND_TABLE {
+ if !meta.is_replicated() {
+ continue;
+ }
+ assert_ne!(
+ operation_for_code(meta.code),
+ Operation::NonReplicated,
+ "replicated command {} ({}) must never encode as
NonReplicated",
+ meta.name,
+ meta.code
+ );
+ }
+ }
+
#[test]
fn namespace_defers_named_identifiers_to_server_resolution() {
let stream = WireIdentifier::named("stream").unwrap();
