Skip to content

fix(protocol): derive the xattr digest length from the hasher - #7786

Merged
oferchen merged 2 commits into
masterfrom
fix/derive-xattr-digest-len
Sep 9, 2026
Merged

fix(protocol): derive the xattr digest length from the hasher#7786
oferchen merged 2 commits into
masterfrom
fix/derive-xattr-digest-len

Conversation

@oferchen

@oferchen oferchen commented Sep 9, 2026

Copy link
Copy Markdown
Owner

MAX_XATTR_DIGEST_LEN was a bare 16. Upstream does not write a number
there: xattrs.c:48 defines it as MD5_DIGEST_LEN, and lib/md-defines.h:12
sets that to 16. The value was right; the construction was fragile — the buffer
bound and the hasher that fills it had no relationship a compiler could check.

It is now derived from the hasher itself:

pub const MAX_XATTR_DIGEST_LEN: usize = <Md5 as OutputSizeUser>::OutputSize::USIZE;

compute_xattr_checksum runs Md5, so the buffer and the digest cannot
disagree by construction.

The distinction the derivation has to preserve

Upstream pins this to MD5's length specifically, not to MAX_DIGEST_LEN
(lib/md-defines.h:13-21, up to SHA-512's 64). compat.c:835-836 fixes
xattr_sum_nni to the implied MD5 choice, above a comment reserving the right
to make the algorithm negotiable later. Deriving from "the longest digest oc can
produce" would have been a wire-format change, not a cleanup — the abbreviated
xattr digest is a wire field.

A const assertion pins the value at 16 so the derivation cannot drift into a
protocol break, and digest_len_is_upstream_md5_digest_len_not_max_digest_len
pins the distinction itself.

Tests

  • digest_len_tracks_the_hasher_that_fills_the_buffer — drives the expected
    length from a live Md5::digest(DATUM).len() rather than restating 16, so
    it fails if the constant is ever re-pointed at a different hasher.
  • digest_len_is_upstream_md5_digest_len_not_max_digest_len — encodes why 16
    and not 64.

Footprint

One file, +48/-2. No behaviour change: the value is identical before and after.

Upstream defines the constant, not a number: xattrs.c:48 sets
MAX_XATTR_DIGEST_LEN to MD5_DIGEST_LEN, which lib/md-defines.h:12 fixes at
16. oc carried the 16 as a bare literal, so it could not track the hasher
that compute_xattr_checksum actually runs.

Derive it from the md-5 output size instead, add a const assertion pinning
the derived value to upstream's 16 (the digest is a wire field, so a length
change is a protocol break), and cover the drift with tests driven from the
live hasher output rather than a second literal.
@oferchen
oferchen enabled auto-merge (squash) September 9, 2026 12:04
@github-actions github-actions Bot added the bug Something isn't working label Sep 9, 2026
@oferchen
oferchen merged commit dfde36e into master Sep 9, 2026
74 of 75 checks passed
@oferchen
oferchen deleted the fix/derive-xattr-digest-len branch September 9, 2026 14:32
oferchen added a commit that referenced this pull request Sep 9, 2026
The measurement-gate note's "Three that look mirrored and are not" section
landed as #7779 and went stale within a day: all three entries have since
been fixed, and one of them cited a file that is being deleted.

Each closure verified in the tree, not taken from a changelog:

- hash table floor/growth -> #7790. `TAG_TABLE_SIZE` now exists at exactly
  one site (`crates/matching/src/index/mod.rs:68`) and its own doc says the
  growth role "belongs to CompactLookup", which grows on upstream's
  `(count/8) * 10 + 11` rule. The entry's second cited site,
  `optimized_search.rs:12`, is dropped: that module is dead and its removal
  is in flight, and the live constant was never the growth-bearing one.

- `MAX_XATTR_DIGEST_LEN` literal -> #7786. Now
  `<Md5 as OutputSizeUser>::OutputSize::USIZE` with
  `const _: () = assert!(MAX_XATTR_DIGEST_LEN == 16)` beneath it
  (`crates/protocol/src/xattr/mod.rs:76,81`), so a digest change cannot
  silently move a wire length.

- `MAX_INPUT_SIZE` copies -> #7787. One definition at
  `crates/checksums/src/simd_batch/mod.rs:46`; every backend imports it.
  The entry said nine copies; the sweep found twelve sites, eleven constants
  plus one bare literal no grep for the name could reach. The count is
  corrected rather than left flattering.

The section is kept rather than deleted: the lesson it teaches (value
equality is not provenance) is the reason the gate exists, and each fix is
evidence the reading was right. The upstream anchors are unchanged except
`match.c:86-88` -> `match.c:84-88`, matching the range the code itself
cites for the same rule.

Also drops the deleted module from the zsync design note's file list and
corrects that entry's crate directory (`crates/match/` -> `crates/matching/`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant