Skip to content

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

Merged
hubcio merged 2 commits into
masterfrom
login-command-protocol-versin
Jun 30, 2026
Merged

feat(server-ng): reject incompatible clients at login via protocol version#3454
hubcio merged 2 commits 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 80.30303% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.33%. Comparing base (ec3a942) to head (e1a459f).

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.34%   74.33%   -0.01%     
  Complexity      937      937              
============================================
  Files          1256     1257       +1     
  Lines        130300   130671     +371     
  Branches     106169   106585     +416     
============================================
+ Hits          96873    97139     +266     
- Misses        30350    30401      +51     
- Partials       3077     3131      +54     
Components Coverage Δ
Rust Core 75.07% <80.30%> (+0.03%) ⬆️
Java SDK 62.44% <ø> (ø)
C# SDK 71.40% <ø> (-0.74%) ⬇️
Python SDK 88.88% <ø> (ø)
PHP SDK 84.29% <ø> (ø)
Node SDK 91.35% <ø> (ø)
Go SDK 40.14% <ø> (ø)
Files with missing lines Coverage Δ
core/binary_protocol/src/consensus/header.rs 82.14% <100.00%> (+1.78%) ⬆️
...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 86.66% <100.00%> (+53.33%) ⬆️
.../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.23% <ø> (ø)
core/sdk/src/tcp/tcp_client.rs 74.61% <ø> (ø)
... and 11 more

... and 33 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 2 times, most recently from b622a11 to ea88381 Compare June 16, 2026 07:59
krishvishal
krishvishal previously approved these changes Jun 16, 2026
@hubcio hubcio force-pushed the login-command-protocol-versin branch 2 times, most recently from a720e44 to 246da8b Compare June 23, 2026 07:12
…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 5ad6730 to f200135 Compare June 29, 2026 18:35
mmodzelewski
mmodzelewski previously approved these changes Jun 30, 2026
@hubcio hubcio merged commit 898ee5e into master Jun 30, 2026
95 checks passed
@hubcio hubcio deleted the login-command-protocol-versin branch June 30, 2026 07:49
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants