Re: [PATCH proxmox-backup v3 25/30] sync: pull: extend encountered chunk by optional decrypted digest

Thomas Lamprecht
2026-04-19T21:08:31Z

Am 14.04.26 um 14:59 schrieb Christian Ebner:
> diff --git a/src/server/pull.rs b/src/server/pull.rs
>
> + fn mark_reusable(&mut self, digest: &[u8; 32], decrypted_digest: Option<[u8; 32]>) {
> match self.chunk_set.entry(*digest) {
> Entry::Occupied(mut occupied) => {
> - let (reusable, _touched) = occupied.get_mut();
> - *reusable = true;
> + let chunk_info = occupied.get_mut();
> + chunk_info.reusable = true;
> }
> Entry::Vacant(vacant) => {
> - vacant.insert((true, false));
> + vacant.insert(EncounteredChunkInfo {
> + reusable: true,
> + touched: false,
> + decrypted_digest,
> + });
> }
> }
> }
> [...]
> + fn mark_touched(&mut self, digest: &[u8; 32], decrypted_digest: Option<[u8; 32]>) {
> match self.chunk_set.entry(*digest) {
> Entry::Occupied(mut occupied) => {
> - let (_reusable, touched) = occupied.get_mut();
> - *touched = true;
> + let chunk_info = occupied.get_mut();
> + chunk_info.touched = true;
> }
> Entry::Vacant(vacant) => {
> - vacant.insert((false, true));
> + vacant.insert(EncounteredChunkInfo {
> + reusable: false,
> + touched: true,
> + decrypted_digest,
> + });
> }
> }
> }

Both functions silently drop the passed `decrypted_digest` in the Occupied
arm - only the Vacant branch uses it. So the first caller to touch a given
digest wins, and every subsequent call with a different (or
present-vs-absent) decrypted_digest is discarded.

Today all callers in the series pass None except the decrypt path in patch
27, so there's no observed misbehavior, but this is somewhat of a latent
footgun: any future code path that calls `mark_reusable(digest, Some(d1))`
*before* the decrypt code reaches `mark_reusable(digest, Some(d2))` (or even
just `None`) will silently lose the mapping.

Either:
- Document the "first insert wins for decrypted_digest" semantics
explicitly (doc comment + debug_assert on mismatch), or
- Also update the field in the Occupied arm (probably only when the stored
value is currently None, to preserve a decrypted_digest once it's been
learned).

Minor: the parameter name `decrypted_digest` reads naturally in the mark
functions, but check_reusable returns it with just the cryptic `(touched,
Some(&[u8; 32]))` tuple - maybe give it a name via a small struct return or
at least a doc comment line on what the second element means.