Skip to content

feat(server-ng): reject incompatible clients at login via protocol version#3454

Open
hubcio wants to merge 1 commit into
masterfrom
login-command-protocol-versin
Open

feat(server-ng): reject incompatible clients at login via protocol version#3454
hubcio wants to merge 1 commit into
masterfrom
login-command-protocol-versin

Conversation

@hubcio

@hubcio hubcio commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Version mismatches between SDK and server surfaced as runtime decode
failures instead of a clear error at login. The VSR login-register
body now starts with a required ClientVersionInfo prefix: the packed
iggy_binary_protocol crate semver plus sdk name and version. The
server gates on a [min, current-minor] range before touching
credentials (patch never changes the wire) and rejects with a typed
Eviction frame carrying the accepted window; a body without a
decodable prefix is rejected with a dedicated MalformedLogin reason
instead of the version window. The response advertises the server's
protocol and build version; the language-neutral wire spec lives in
the version module docs.

The SDK decodes eviction and reply headers by byte offset instead of
struct casts: response read buffers are not 16-aligned, and offset
reads let the client sanity-check the protocol window. The new
EvictionReason variants and the EvictionHeader protocol window are
appended into reserved space, leaving existing consensus frames and
discriminants byte-compatible.

The SDK validates password and PAT token lengths before encoding,
with the same bounds and errors the servers already enforce, so an
oversized secret cannot desync the u8 length prefix; server-ng
mirrors the legacy username/password bounds before lookup or
hashing. SDK identity is recorded per connection in the
SessionManager; get_clients wire exposure is a follow-up. The
legacy server is untouched.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jun 11, 2026
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.77778% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.66%. Comparing base (fe168ee) to head (ea88381).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
core/server-ng/src/dispatch.rs 0.00% 51 Missing ⚠️
core/server-ng/src/auth.rs 0.00% 14 Missing ⚠️
core/consensus/src/metadata_helpers.rs 45.45% 12 Missing ⚠️
core/server-ng/src/responses.rs 0.00% 8 Missing ⚠️
...nary_protocol/src/requests/users/login_register.rs 95.23% 0 Missing and 2 partials ⚠️
...ocol/src/requests/users/login_register_with_pat.rs 94.73% 0 Missing and 2 partials ⚠️
...ary_protocol/src/responses/users/login_register.rs 97.05% 0 Missing and 1 partial ⚠️
core/binary_protocol/src/version.rs 99.22% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3454      +/-   ##
============================================
- Coverage     74.68%   74.66%   -0.02%     
  Complexity      937      937              
============================================
  Files          1256     1257       +1     
  Lines        124732   125222     +490     
  Branches     100454   100986     +532     
============================================
+ Hits          93151    93492     +341     
- Misses        28586    28672      +86     
- Partials       2995     3058      +63     
Components Coverage Δ
Rust Core 75.71% <79.77%> (+0.03%) ⬆️
Java SDK 58.57% <ø> (ø)
C# SDK 71.40% <ø> (-0.71%) ⬇️
Python SDK 88.88% <ø> (ø)
PHP SDK 83.57% <ø> (ø)
Node SDK 91.13% <ø> (-0.23%) ⬇️
Go SDK 40.36% <ø> (ø)
Files with missing lines Coverage Δ
core/binary_protocol/src/consensus/header.rs 81.54% <100.00%> (+1.85%) ⬆️
...ary_protocol/src/requests/users/change_password.rs 100.00% <100.00%> (ø)
.../binary_protocol/src/requests/users/create_user.rs 96.24% <100.00%> (+0.05%) ⬆️
...e/binary_protocol/src/requests/users/login_user.rs 100.00% <100.00%> (ø)
core/common/src/error/iggy_error.rs 100.00% <ø> (ø)
core/common/src/traits/binary_impls/mod.rs 77.77% <100.00%> (+44.44%) ⬆️
.../src/traits/binary_impls/personal_access_tokens.rs 100.00% <ø> (ø)
core/common/src/traits/binary_impls/users.rs 100.00% <ø> (ø)
core/sdk/src/quic/quic_client.rs 73.15% <ø> (-0.16%) ⬇️
core/sdk/src/tcp/tcp_client.rs 74.14% <ø> (ø)
... and 11 more

... and 38 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread core/binary_protocol/src/consensus/header.rs
Comment thread core/binary_protocol/src/consensus/header.rs
@hubcio hubcio force-pushed the login-command-protocol-versin branch from 74b2928 to b622a11 Compare June 16, 2026 06:45
…rsion

Version mismatches between SDK and server surfaced as runtime decode
failures instead of a clear error at login. The VSR login-register
body now starts with a required ClientVersionInfo prefix: the packed
iggy_binary_protocol crate semver plus sdk name and version. The
server gates on a [min, current-minor] range before touching
credentials (patch never changes the wire) and rejects with a typed
Eviction frame carrying the accepted window; a body without a
decodable prefix is rejected with a dedicated MalformedLogin reason
instead of the version window. The response advertises the server's
protocol and build version; the language-neutral wire spec lives in
the version module docs.

The SDK decodes eviction and reply headers by byte offset instead of
struct casts: response read buffers are not 16-aligned, and offset
reads let the client sanity-check the protocol window. The new
EvictionReason variants and the EvictionHeader protocol window are
appended into reserved space, leaving existing consensus frames and
discriminants byte-compatible.

The SDK validates password and PAT token lengths before encoding,
with the same bounds and errors the servers already enforce, so an
oversized secret cannot desync the u8 length prefix; server-ng
mirrors the legacy username/password bounds before lookup or
hashing. SDK identity is recorded per connection in the
SessionManager; get_clients wire exposure is a follow-up. The
legacy server is untouched.
@hubcio hubcio force-pushed the login-command-protocol-versin branch from b622a11 to ea88381 Compare June 16, 2026 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants