Skip to content

types: make WirePayload opaque so to_str reaches the slack-aware validator#247

Open
iainmcgin wants to merge 1 commit into
mainfrom
iain/wirepayload-slack
Open

types: make WirePayload opaque so to_str reaches the slack-aware validator#247
iainmcgin wants to merge 1 commit into
mainfrom
iain/wirepayload-slack

Conversation

@iainmcgin

Copy link
Copy Markdown
Collaborator

Why

WirePayload (introduced in 0.8.0, #206/#241) is the argument to ProtoString::from_wire / ProtoBytes::from_wire. Its to_str() is the recommended UTF-8 validator for custom string representations, but read_field_payload pre-sliced to &chunk[..len] before constructing Borrowed, so to_str() only ever saw the field bytes and called plain validate_str — the one string decode site that #241's slack-aware integration didn't reach. Traced as part of the SlackBuf design pass (#246's follow-up) but orthogonal to it.

What

WirePayload becomes an opaque struct over a private repr enum whose Borrowed variant carries (chunk, len) with the invariant len <= chunk.len(). to_str() now calls validate_str_in(chunk, len) and reaches verify_with_slack whenever ≥ SLACK readable bytes follow the field in the chunk. read_field_payload passes the full remaining chunk instead of pre-slicing.

Public surface: same accessors (as_slice, to_str, into_bytes) plus new len / is_empty / is_owned, and WirePayload::borrowed(&[u8]) / WirePayload::owned(Bytes) constructors. Manual Debug prints only the field bytes (the derived one would have leaked the surrounding chunk tail).

Breaking change (0.9.0-tier)

WirePayload shipped as a public-variant enum one day ago in 0.8.0. Migration:

0.8.0 now
from_wire using as_slice() / to_str() / into_bytes() unchanged
WirePayload::Borrowed(s) / ::Owned(b) construction WirePayload::borrowed(s) / ::owned(b)
match payload { Borrowed(s) => …, Owned(b) => … } is_owned() + accessors

Making it opaque now means the next repr change (single-chunk-Bytes zero-copy, already noted in into_bytes's doc) is non-breaking.

cargo public-api / cargo semver-checks are both unavailable on this toolchain; the break is asserted from the diff (public enum → struct, variants removed, borrowed/owned/len/is_empty/is_owned added).

…dator

WirePayload was a public-variant enum (Borrowed(&[u8]) / Owned(Bytes)).
read_field_payload pre-sliced to &chunk[..len] before constructing
Borrowed, so to_str() could only call plain validate_str — the
custom-ProtoString decode path (decode_string_to<S>) was the one string
site #241's slack integration didn't reach.

This makes WirePayload an opaque struct over a private repr enum whose
Borrowed variant carries (chunk, len) with the invariant len <=
chunk.len(); to_str() now calls validate_str_in(chunk, len) and reaches
verify_with_slack whenever the chunk has SLACK readable bytes after the
field. read_field_payload passes the full remaining chunk instead of
pre-slicing.

Public surface: same accessors (as_slice, to_str, into_bytes) plus new
len / is_empty / is_owned, and borrowed(&[u8]) / owned(Bytes)
constructors. Manual Debug prints only the field bytes, never the
surrounding tail. from_wire impls that use the accessors are unaffected
(all in-tree ones do); construct sites lowercase the constructor;
match-on-variant moves to is_owned() + accessors.

Breaking (0.9.0-tier): WirePayload shipped in 0.8.0 one day ago, and
making it opaque now means the next repr change (single-chunk-Bytes
zero-copy) is non-breaking.
@github-actions

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@iainmcgin iainmcgin marked this pull request as ready for review June 26, 2026 22:19
@iainmcgin iainmcgin requested a review from rpb-ant June 26, 2026 22:19
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