From ad03b5b0bf7e9d5bbc8149d475c60871ab04e3a9 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Tue, 8 Sep 2026 13:14:33 +0000 Subject: [PATCH] Check a census row against the field before its writer runs --- .../libraries/core-types/src/attribute.rs | 7 ++++ .../libraries/core-types/src/record/run.rs | 38 +++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/node-graph/libraries/core-types/src/attribute.rs b/node-graph/libraries/core-types/src/attribute.rs index 13943668c9..7738b1b03d 100644 --- a/node-graph/libraries/core-types/src/attribute.rs +++ b/node-graph/libraries/core-types/src/attribute.rs @@ -151,6 +151,13 @@ pub struct AttributeInfo { /// Writes a legacy stored value into a field of this marker, parking /// droppable payloads. A wrong-typed stored value leaves the field /// untouched; `None` reports arena exhaustion. + /// + /// `dst` must address a live field of *this row's* value type: the glue + /// writes its own `Value`'s worth of bytes there, so a caller resolving the + /// field by [`name`](Self::name) checks [`size`](Self::size) and + /// [`value_type`](Self::value_type) against the field first. The fields + /// here are public and the struct is `Copy`, so a row is a claim about a + /// marker rather than a proof about a field. pub write_stored: unsafe fn(&dyn AnyAttributeValue, *mut u8, &crate::arena::Arena) -> Option<()>, } diff --git a/node-graph/libraries/core-types/src/record/run.rs b/node-graph/libraries/core-types/src/record/run.rs index bda5392a94..9038f9059a 100644 --- a/node-graph/libraries/core-types/src/record/run.rs +++ b/node-graph/libraries/core-types/src/record/run.rs @@ -133,13 +133,23 @@ impl<'e> RunBuilder<'e> { /// Writes a legacy stored value on an already pushed lane through the /// census glue, parking droppable payloads. A marker outside the layout's - /// fields is dropped; a wrong-typed stored value leaves the field's + /// fields is dropped, as is one whose row and field agree on the name but + /// not on the value type; a wrong-typed stored value leaves the field's /// default. `None` reports arena exhaustion. pub fn attr_stored(&mut self, lane: usize, info: &crate::attribute::AttributeInfo, value: &dyn crate::list::AnyAttributeValue) -> Option<()> { assert!(lane < self.pushed, "attributes write onto pushed lanes"); - let Some(offset) = self.layout.offset_of(info.name, 0) else { return Some(()) }; - // SAFETY: the offset comes from the builder's own layout, and the - // census writer verifies the stored type before touching the field. + let Some(field) = self.layout.fields.iter().find(|field| field.name == info.name && field.level == 0) else { + return Some(()); + }; + let (offset, size, type_id) = (field.offset, field.size, field.type_id); + // The row reaches the field by name alone, so the writer's own value type + // is checked against the field before its glue writes there, as `push` + // checks the default writer's. + if info.size != size || info.value_type != type_id { + return Some(()); + } + // SAFETY: the offset comes from the builder's own layout, and the field + // was just checked to hold the writer's own value type at its size. unsafe { (info.write_stored)(value, self.frames.add(lane * self.layout.lane_stride() + offset), self.arena) } } @@ -938,6 +948,26 @@ mod tests { assert_eq!(run.attr::(0), &[crate::uuid::NodeId(3)]); } + #[test] + fn a_census_row_pointed_at_another_field_writes_nothing() { + use crate::attribute::{Attribute, Opacity, Transform}; + use crate::lane::LaneSource; + + let arena = crate::arena::Arena::new(1 << 16).unwrap(); + let mut builder = RunBuilder::new(&arena, element_write_hashed::(), &[FieldWrite::of::(0)], 1).unwrap(); + let lane = builder.push(0f64).unwrap(); + + // The transform writer renamed onto the 8-byte opacity field: a + // 48-byte write past the lane if the name were the only check. + let mut forged = crate::attribute::info(Transform::NAME).unwrap(); + forged.name = Opacity::NAME; + builder.attr_stored(lane, &forged, &glam::DAffine2::from_translation((5., 6.).into())).unwrap(); + + let item = builder.finish(); + let run = RunView::::new(&item).expect("the run holds f64 elements"); + assert_eq!(run.attr::(0), 1., "the mistyped row leaves the census default"); + } + #[test] fn a_wrong_typed_or_undeclared_column_leaves_the_default() { use crate::lane::LaneSource;