Skip to content

fix(serder)!: guard version-string fixed widths and validate at every deserialize entry#139

Merged
joeldsouzax merged 1 commit into
mainfrom
fix/serder-version-string-guards
Jul 10, 2026
Merged

fix(serder)!: guard version-string fixed widths and validate at every deserialize entry#139
joeldsouzax merged 1 commit into
mainfrom
fix/serder-version-string-guards

Conversation

@joeldsouzax

Copy link
Copy Markdown
Contributor

Summary

Fixes three defects on the serder edge, found during the #79 research phase (write-up in #138). Each landed with a bug probe that fails while the defect exists.

1. VersionString::to_str() silently corrupted the 17-byte frame

{:x}/{:06x} rendering had no width guards: major/minor > 0xF or size > 0xFF_FFFF (16 MiB − 1) widened the version string past its documented 17 bytes, corrupting the frame instead of erroring. to_str() now returns Result and rejects overflow with the new SerderError::VersionStringOverflow variant.

2. Serializers never checked the size-field capacity (and misfiled the error)

All five event serializers could produce a corrupt frame for a > 16 MiB event, and mapped the u32 length-conversion failure into SerderError::DigestError — the wrong failure domain. Both fixed; an end-to-end probe serializes a 340k-anchor interaction event and asserts the typed error.

3. Per-ilk deserializers skipped version-string validation

Only deserialize_event called validate_version_string; the five per-ilk public entry points did not. Because SAID verification hashes the re-serialized compact form, whitespace-padded raw — valid JSON, intact SAID, length contradicting the version-string size — was accepted by deserialize_inception et al. Every public deserializer now validates first (probe: deserialize_*_rejects_length_mismatched_raw, red before the fix).

Breaking change (0.x minor bump)

  • VersionString::to_str()Result<String, SerderError>
  • SerderError gains VersionStringOverflow { field, max }

Both intentional; per-ilk deserializers returning InvalidVersionString on inputs previously accepted is the bug fix itself.

Verification

  • nix flake check green (pre-push hook, committed tree)
  • Bug probes red-before/green-after: whitespace-padding probe confirmed failing pre-fix in-session
  • Existing 175 serder/version tests unchanged and passing

Refs #79 (research: #138)

🤖 Generated with Claude Code

… deserialize entry

Three defects on the serder edge:

- VersionString::to_str() rendered major/minor/size with unguarded {:x}/
  {:06x}: any value beyond its fixed hex width (major/minor > 0xF, size
  > 0xFFFFFF) silently widened the documented 17-byte version string and
  corrupted the frame. to_str() now returns Result and rejects overflow
  with the new SerderError::VersionStringOverflow variant.

- All five event serializers mapped the measured-length u32 conversion
  failure into SerderError::DigestError (wrong failure domain) and never
  checked the 6-hex-digit size capacity; a >16 MiB event serialized to a
  corrupt frame. They now reject it as VersionStringOverflow.

- Only deserialize_event validated the version string; the per-ilk public
  entry points (deserialize_inception/rotation/interaction/
  delegated_inception/delegated_rotation) skipped it, so raw whose length
  contradicts its version-string size — e.g. whitespace-padded JSON with
  an intact SAID — was accepted. Every public deserializer now calls
  validate_version_string first.

BREAKING CHANGE: VersionString::to_str() returns Result<String,
SerderError>; SerderError gains the VersionStringOverflow variant.

Bug probes (red before fix): to_str_rejects_size_beyond_fixed_width,
to_str_rejects_{major,minor}_beyond_one_hex_digit,
serialize_ixn_rejects_event_beyond_version_size_capacity,
all_public_deserializers_reject_length_mismatched_raw.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
joeldsouzax added a commit that referenced this pull request Jul 10, 2026
…d in #139

Answers the review question 'why do we need serde at all?': deserialization
happens once at the edge and the write path uses none of serde's data model,
so the end state is a serde-free production path with serde_json demoted to
a dev-dependency oracle. Read path becomes a strict canonical parser
(borrowed fields + SAID offsets, feeding C-a #129); strictness is itself a
conformance feature — the whitespace-acceptance defect fixed in #139 is the
proof case. Staged migration spelled out in §6.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codspeed-hq

codspeed-hq Bot commented Jul 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 21 untouched benchmarks
⏩ 21 skipped benchmarks1


Comparing fix/serder-version-string-guards (68cfc6b) with main (c435523)

Open in CodSpeed

Footnotes

  1. 21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@joeldsouzax joeldsouzax enabled auto-merge (squash) July 10, 2026 14:38
@joeldsouzax joeldsouzax merged commit f274048 into main Jul 10, 2026
32 checks passed
joeldsouzax added a commit that referenced this pull request Jul 10, 2026
… seam (#138)

* docs(serder): #79 research write-up — pluggable serialization backend seam design

Research-first deliverable for #79: status-quo allocation audit (3x
to_string per serialize, 2-3x from_slice + re-render per deserialize),
body-encoder seam design with fixed-width slot backpatching, crate
survey (zero new deps), cross-backend conformance strategy, and a
latent version-string size-overflow defect to fix under this card.

Refs #79

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(serder): #79 write-up — serde-free endgame (§3.5) + defects fixed in #139

Answers the review question 'why do we need serde at all?': deserialization
happens once at the edge and the write path uses none of serde's data model,
so the end state is a serde-free production path with serde_json demoted to
a dev-dependency oracle. Read path becomes a strict canonical parser
(borrowed fields + SAID offsets, feeding C-a #129); strictness is itself a
conformance feature — the whitespace-acceptance defect fixed in #139 is the
proof case. Staged migration spelled out in §6.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant