view: require an unsafe ViewLifetimeParametric marker on OwnedView views - #381
Merged
Merged
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
OwnedView::decode transmutes the Bytes slice to &'static [u8] and hands it to
V::decode_view with only V: MessageView<'static> as a bound. Nothing required
V to be parametric in the buffer lifetime, so a hand-written
impl MessageView<'static> for MyView (or its own Debug/Clone/PartialEq/Drop/
Serialize impl, which OwnedView invoked on the 'static-typed view) could copy
a &'static str out of the buffer into longer-lived storage and read it after
the OwnedView dropped.
Add `pub unsafe trait LifetimeParametric: ViewReborrow {}` whose # Safety
section states the contract, require it on every OwnedView constructor and
on HasMessageView::decode_view_handle{,_with_options}, and route OwnedView's
Debug/PartialEq/Eq/Serialize/to_owned_message through ViewReborrow::reborrow
so the view's impls only ever run at the real buffer lifetime. Split the
inherent impl so bytes/into_bytes need no bound and reborrow/to_owned_message
need only ViewReborrow; put a Debug bound on ViewReborrow::Reborrowed so
OwnedView<V>: Debug holds for every V: ViewReborrow without a for<'b> clause.
Codegen emits `::buffa::unsafe_impl_lifetime_parametric!(FooView)` for every
eager view; the macro keeps generated output valid under forbid(unsafe_code).
Regenerate the checked-in buffa-types and buffa-descriptor views, add a
compile_fail doctest for a non-parametric capturing view and a Miri-targeted
positive test wired into the CI Miri step, and update the guide and DESIGN.md.
Closes #376.
iainmcgin
force-pushed
the
iain/issue-376-ownedview-contract
branch
from
August 27, 2026 08:00
7f0ef17 to
b827498
Compare
… per-constructor bounds Merge main (#379 removed OwnedView's Drop impl in favour of field order + MaybeDangling; #394 added three WKT view files) and rework on top: - rename the marker LifetimeParametric -> ViewLifetimeParametric and the macro to unsafe_impl_view_lifetime_parametric!, which now accepts a path; - move the marker bound from the constructor impl block to a where-clause on each constructor (decode, decode_with_options, from_owned, from_parts) so a missing impl reports E0277 with the on_unimplemented notes instead of E0599, and update the compile_fail doctest accordingly; Clone carries the bound too; - # Safety docs: drop-order wording follows main, state that only decode_view* and Clone observe the forged 'static after the reborrow routing, why Drop and V::Owned cannot leak, and cite yoke::Yokeable; - guide: hand-written views used through OwnedView also need Debug and ViewReborrow; macro example; correct the PartialEq claim for generated views; - changelog: split into a Security entry and a migration-led Breaking entry; - buffa-test: a #[forbid(unsafe_code)] module including generated views, and a generic HRTB Serialize caller; the PanicOnFirstDropView test from main gains the marker; - CI: fold the Miri test into main's OwnedView soundness step; - regenerate WKT and descriptor view types (incl. api/type/source_context); add the macro call to the checked-in bsr-quickstart output.
iainmcgin
changed the title
…o accepts ::paths Re-review follow-ups: the Safety audit and the delegation comment now list reborrow alongside decode_view* and Clone; the Drop claim is scoped to lifetime-generic views; the macro takes leading-:: paths; the forbid(unsafe_code) canary moves onto the existing inline_field module instead of including the generated code twice.
….75 cannot infer it through AsRef)
rustc 1.75 registers the higher-ranked bound before inferring V from the argument; reproduced locally with the MSRV lockfile. Changelog notes the same for downstream generic callers on old toolchains.
iainmcgin
force-pushed
the
iain/issue-376-ownedview-contract
branch
from
September 6, 2026 16:49
e3dccab to
c1d39a0
Compare
iainmcgin
marked this pull request as ready for review
September 6, 2026 17:22
azdagron
approved these changes
Sep 8, 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.
OwnedView::<V>::decodehandsV::decode_viewa slice forged to&'static [u8], withV: MessageView<'static>as the only bound.MessageViewis safe and unsealed, so a hand-written non-parametricimpl MessageView<'static> for MyView— or its ownDebug/PartialEq/Serialize/Clone, whichOwnedViewinvoked on the'static-typed view — could copy a&'static strout of the buffer and read it after theOwnedViewdropped: a use-after-free from safe code. Generated views were never exposed (everyFooView<'a>is parametric in'a), and no wire input reaches it.Every
OwnedViewconstructor now requiresV: ViewLifetimeParametric, anunsafemarker trait withViewReborrowas supertrait; codegen emits it for every generated view viabuffa::unsafe_impl_view_lifetime_parametric!, whose expansion is accepted under a consumer's#![forbid(unsafe_code)].OwnedView'sDebug,PartialEq,Eq,Serializeandto_owned_messagenow call the view's impls on areborrow()edV::Reborrowed<'b>, so after this change onlydecode_view*andCloneobserve the forged lifetime; the trait's# Safetysection reduces a hand-written view's audit to those two.A type-level fix (decode at a fresh lifetime, then extend
V::Reborrowed<'a>toV) would needViewReborrowitself to becomeunsafeplus a transmute in buffa.Breaking, for 0.10.0. Upgrading: regenerate; generic callers of
decode_view_handleaddM::View<'static>: buffa::ViewLifetimeParametric;ViewReborrow::Reborrowedgains aDebugbound, so any hand-writtenViewReborrowimpl needs aDebugview; a hand-written view used throughOwnedViewimplementsDebugandViewReborrow, thenexamples/bsr-quickstart's checked-in output gets the macro line by hand; it cannot be regenerated until a 0.10 BSR plugin is published, and it already fails to build onmainfor unrelated staleness.Closes #376. Reported by HackerOne researcher waynezinn.