types: make WirePayload opaque so to_str reaches the slack-aware validator#247
Open
iainmcgin wants to merge 1 commit into
Open
types: make WirePayload opaque so to_str reaches the slack-aware validator#247iainmcgin wants to merge 1 commit into
iainmcgin wants to merge 1 commit into
Conversation
…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.
|
All contributors have signed the CLA ✍️ ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
WirePayload(introduced in 0.8.0, #206/#241) is the argument toProtoString::from_wire/ProtoBytes::from_wire. Itsto_str()is the recommended UTF-8 validator for custom string representations, butread_field_payloadpre-sliced to&chunk[..len]before constructingBorrowed, soto_str()only ever saw the field bytes and called plainvalidate_str— the onestringdecode site that #241's slack-aware integration didn't reach. Traced as part of theSlackBufdesign pass (#246's follow-up) but orthogonal to it.What
WirePayloadbecomes an opaque struct over a private repr enum whoseBorrowedvariant carries(chunk, len)with the invariantlen <= chunk.len().to_str()now callsvalidate_str_in(chunk, len)and reachesverify_with_slackwhenever ≥SLACKreadable bytes follow the field in the chunk.read_field_payloadpasses the full remaining chunk instead of pre-slicing.Public surface: same accessors (
as_slice,to_str,into_bytes) plus newlen/is_empty/is_owned, andWirePayload::borrowed(&[u8])/WirePayload::owned(Bytes)constructors. ManualDebugprints only the field bytes (the derived one would have leaked the surrounding chunk tail).Breaking change (0.9.0-tier)
WirePayloadshipped as a public-variant enum one day ago in 0.8.0. Migration:from_wireusingas_slice()/to_str()/into_bytes()WirePayload::Borrowed(s)/::Owned(b)constructionWirePayload::borrowed(s)/::owned(b)match payload { Borrowed(s) => …, Owned(b) => … }is_owned()+ accessorsMaking it opaque now means the next repr change (single-chunk-
Byteszero-copy, already noted ininto_bytes's doc) is non-breaking.cargo public-api/cargo semver-checksare both unavailable on this toolchain; the break is asserted from the diff (public enum → struct, variants removed,borrowed/owned/len/is_empty/is_ownedadded).