view: enforce the unknown-field limit and coalesce adjacent unknown spans - #186
Merged
iainmcgin merged 2 commits intoJun 11, 2026
Merged
Conversation
…pans View decoding stored one borrowed 16-byte span per unknown wire record with no bound beyond the input size, so a run of minimal 2-byte unknown fields amplified ~8x into span storage -- the same shape of overhead the unknown-field limit bounds on the owned path, which views ignored. Two changes, sharing the limit machinery the owned path introduced: Adjacent unknown records now coalesce into a single span. Generated view decode calls UnknownFieldsView::push_record with the input-buffer tail at the record start; when a record begins exactly where the previous span ended, the span is extended by re-slicing the stored tail (never by widening the narrowed span reference, which would be provenance-unsound), so a contiguous run of unknown fields of any length costs one Vec slot and re-encodes byte-identically. push_raw remains for manual construction and disables coalescing for the next record. Each new span -- one per contiguous unknown run -- is counted against the same unknown-field allowance as owned decoding. The view decode path now threads DecodeContext: MessageView::decode_view_with_limit (buf, depth) is replaced by decode_view_with_ctx(buf, ctx), generated views' hidden _decode_depth helpers become _decode_ctx, and DecodeOptions::decode_view passes its configured recursion and unknown-field limits (the latter was previously ignored by views). Breaking for code generated by earlier releases, consistent with the owned-path change; all checked-in generated code is regenerated. UnknownFieldsView::to_owned now parses every record within a coalesced span, and Debug no longer derives over the internal coalescing cursor (which would dump the remaining input buffer). All 12 conformance suites pass with no unexpected failures.
|
All contributors have signed the CLA ✍️ ✅ |
iainmcgin
marked this pull request as ready for review
June 11, 2026 15:05
Generated to_owned_message conversions swallowed unknown-field re-materialization errors via unwrap_or_default(), silently dropping every unknown field from the converted message. And the unknown-field limit configured at decode_view time was discarded at the conversion boundary: UnknownFieldsView::to_owned always re-materialized under a fresh default allowance, so a deliberately tight limit evaporated exactly where per-field allocation actually happens. MessageView::to_owned_message and to_owned_from_source (and the generated OwnedView wrapper) now return Result<Owned, DecodeError>, propagating conversion errors instead of dropping data. Codegen threads the Result through nested message, repeated, map, and oneof conversions; oneof groups with message-typed variants convert via a match so the ? propagates (scalar-only groups keep Option::map, which clippy requires). UnknownFieldsView captures the allowance remaining when its first record is pushed, and to_owned re-materializes under that allowance -- one owned UnknownField per record, matching how the owned decode path counts. Views built manually via push_raw fall back to the default limit. A view of a coalesced flood decoded under a tight limit now fails at to_owned_message with UnknownFieldLimitExceeded rather than succeeding at decode and silently losing the fields at conversion. All 12 conformance suites pass with no unexpected failures.
asacamano
approved these changes
Jun 11, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
What this does
Extends the unknown-field decode limit from #184 to the zero-copy view path, makes view storage of unknown fields dramatically cheaper via span coalescing, and fixes two defects at the view→owned conversion boundary: silent unknown-field loss and the decode-time limit being discarded.
What changed
Span coalescing. Generated view decode records unknown fields via
UnknownFieldsView::push_record(tail, span_len, ctx). When a record begins exactly where the previous span ended, the span is extended in place by re-slicing the stored input-buffer tail — deliberately not by widening the narrowed span reference, which would be provenance-unsound; the implementation needs nounsafe. A contiguous run of unknown fields of any length costs oneVecslot and re-encodes byte-identically.Limit enforcement in view decode. Each new span — one per contiguous unknown run — consumes one slot of the same unknown-field allowance the owned path uses. The view decode path threads
DecodeContext:MessageView::decode_view_with_limit(buf, depth)→decode_view_with_ctx(buf, ctx), generated_decode_depth→_decode_ctx, andDecodeOptions::decode_viewnow honorswith_unknown_field_limit(previously ignored by views).Fallible conversion, no silent data loss.
MessageView::to_owned_message/to_owned_from_source(and theOwnedViewwrapper) now returnResult<Owned, DecodeError>. Generated conversions previously didto_owned().unwrap_or_default(), silently dropping every unknown field on error; codegen now propagates theResultthrough nested message, repeated, map, and oneof conversions.Decode-time limit carries through conversion.
UnknownFieldsViewcaptures the allowance remaining at its first record;to_ownedre-materializes under that allowance, one ownedUnknownFieldper record. A flood decoded under a tight limit (one coalesced span) now fails at conversion withUnknownFieldLimitExceededinstead of silently losing the fields. Manual views (push_raw) fall back to the default limit.All three changes are breaking for code generated by earlier releases (regeneration required), consistent with #184; checked-in generated code is regenerated here.
How we know it works
push_recordadjacency/limit/push_rawinteraction, allowance capture, default fallback, multi-recordto_owned, conversion failure under tight allowance.limit=1, then fails atto_owned_messageunder that allowance and round-trips byte-identically under the default; interleaved runs counted exactly; group payload is one span.