M12: PRC reader: answer review (#1297)

- Schema programs draw on one step allowance per reading pass:
  MAX_SCHEMA_STEPS (2^22) moves into the BitReader's budget next to the
  decoded-element allowance, and the tree walk carries both from reader
  to reader. A per-call ceiling let a cheap entity repeated many times
  run ~2^22 steps each.
- The vertex-colour pre-check charges the minimum encoding (one full
  colour, then one reuse bit each), so a run of reused colours may end a
  section.
- placements() reports unreadable globals when an entity reference, not
  only an item, names a reference coordinate system.
- chunks_exact -> as_chunks for clippy 1.99 (chunks_exact_to_as_chunks).

Tests: programs_on_one_stream_share_the_step_allowance,
one_step_allowance_covers_the_whole_walk,
reused_vertex_colours_may_be_the_last_data,
unreadable_globals_are_an_error_when_a_reference_names_a_coordinate_system;
each fails with its fix reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9
This commit is contained in:
KenM76andClaude Opus 5.5 committed 2026-11-02 14:38:13 -05:00
1 parent 5eb484c235
commit cb317560fe
9 files changed
+174 -57

No files matched your search

+1 -1
View File
@@ -24,7 +24,7 @@ BitReader: bit, bits, character, unsigned_integer, integer, integer_vbn, double_
- `MAX_INFLATED_BYTES` (512 MiB) bounds every section together; `TooLarge` reports the limit hit.
- `MAX_DECODED_ELEMENTS` (2^24) bounds the array elements one pass decodes (a tree walk, or one `tessellations` call), since a single bit can decode to a 4-8 byte element; `TooManyElements` reports it.
- Schema programs (how a newer producer describes fields it added) run under a step ceiling (2^22) and a nesting ceiling (64).
- Schema programs (how a newer producer describes fields it added) run under a nesting ceiling (64) and `MAX_SCHEMA_STEPS` (2^22), shared like `MAX_DECODED_ELEMENTS` by every program one pass runs.
- `Unsupported` names constructs the reader cannot step over (schema pointer fields, `Value_CurveIs3D`).
- `Tessellation::Mesh` is a `TriangleMesh`: positions, triangles, one triangle range per face, the stored normals and
texture coordinates with each triangle's corner indices into them, and per-triangle line-attribute styles.
+49 -9
View File
@@ -12,30 +12,59 @@ use crate::acof::{self, Entry};
/// bytes each, so 128 MiB.
pub const MAX_DECODED_ELEMENTS: usize = 1 << 24;
/// The most schema-program steps (tokens executed, loop iterations included) the readers of
/// one PRC stream run together.
pub const MAX_SCHEMA_STEPS: u64 = 1 << 22;
/// What the readers of one stream may still spend, carried from each reader to the next.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct Budget {
/// Array elements still to decode.
pub(crate) elements: usize,
/// Schema-program steps still to run.
pub(crate) steps: u64,
}
impl Budget {
/// A whole stream's allowance.
pub(crate) const FULL: Self = Self { elements: MAX_DECODED_ELEMENTS, steps: MAX_SCHEMA_STEPS };
}
/// A cursor over one inflated section.
#[derive(Debug, Clone)]
pub struct BitReader<'a> {
data: &'a [u8],
pos: usize,
/// Array elements this reader may still decode.
elements: usize,
budget: Budget,
}
impl<'a> BitReader<'a> {
/// A reader positioned at the first bit of `data`, allowed [`MAX_DECODED_ELEMENTS`].
/// A reader positioned at the first bit of `data`, allowed [`MAX_DECODED_ELEMENTS`] and
/// [`MAX_SCHEMA_STEPS`].
pub fn new(data: &'a [u8]) -> Self {
Self::with_elements(data, MAX_DECODED_ELEMENTS)
Self::with_budget(data, Budget::FULL)
}
/// A reader of `data` allowed `elements` array elements: what an earlier reader of the
/// same stream left.
/// A reader of `data` allowed `budget`: what an earlier reader of the same stream left.
pub(crate) fn with_budget(data: &'a [u8], budget: Budget) -> Self {
Self { data, pos: 0, budget }
}
/// A reader of `data` allowed `elements` array elements and a full step allowance.
#[cfg(test)]
pub(crate) fn with_elements(data: &'a [u8], elements: usize) -> Self {
Self { data, pos: 0, elements }
Self::with_budget(data, Budget { elements, ..Budget::FULL })
}
/// What this reader may still spend.
pub(crate) fn budget(&self) -> Budget {
self.budget
}
/// Array elements this reader may still decode.
#[cfg(test)]
pub(crate) fn elements_left(&self) -> usize {
self.elements
self.budget.elements
}
/// Charge `n` decoded array elements against the allowance.
@@ -43,10 +72,21 @@ impl<'a> BitReader<'a> {
/// # Errors
/// [`PrcError::TooManyElements`] when fewer than `n` are left.
pub(crate) fn take_elements(&mut self, n: usize) -> Result<(), PrcError> {
self.elements = self.elements.checked_sub(n).ok_or(PrcError::TooManyElements { limit: MAX_DECODED_ELEMENTS })?;
self.budget.elements = self.budget.elements.checked_sub(n).ok_or(PrcError::TooManyElements { limit: MAX_DECODED_ELEMENTS })?;
Ok(())
}
/// Charge one schema-program step; false when none is left.
pub(crate) fn take_step(&mut self) -> bool {
match self.budget.steps.checked_sub(1) {
Some(left) => {
self.budget.steps = left;
true
}
None => false,
}
}
/// Bits consumed so far.
pub fn position(&self) -> usize {
self.pos
+3 -3
View File
@@ -10,8 +10,8 @@
//! Clause numbers in `[WD x.y]` refer to the PRC working draft, ISO/TC 171/SC 2 N570.
//!
//! Every read is bounded: inflation by [`MAX_INFLATED_BYTES`] (or a caller's limit), decoded
//! array elements by [`MAX_DECODED_ELEMENTS`] per reading pass, schema programs by a step and
//! depth ceiling. Damaged data is an error, never a panic.
//! array elements by [`MAX_DECODED_ELEMENTS`] and schema-program steps by [`MAX_SCHEMA_STEPS`]
//! per reading pass, schema programs by a depth ceiling. Damaged data is an error, never a panic.
#![forbid(unsafe_code)]
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
@@ -28,7 +28,7 @@ mod tess;
mod testw;
mod tree;
pub use bits::{BitReader, MAX_DECODED_ELEMENTS};
pub use bits::{BitReader, MAX_DECODED_ELEMENTS, MAX_SCHEMA_STEPS};
pub use container::{FileStructure, MAX_FILE_STRUCTURES, MAX_INFLATED_BYTES, PRC_READER_VERSION, PrcFile, PrcHeader, SectionKind, UniqueId};
pub use error::PrcError;
pub use geometry::Matrix;
+18 -11
View File
@@ -25,10 +25,6 @@ use std::collections::HashMap;
use crate::PrcError;
use crate::bits::BitReader;
/// Steps (tokens executed, loop iterations included) one
/// [`Schema::skip_added_fields`] call may take.
const MAX_STEPS: u64 = 1 << 22;
/// Nesting depth of blocks, loops, expressions and `Father_Type` calls.
const MAX_DEPTH: u32 = 64;
@@ -80,17 +76,18 @@ impl Schema {
}
/// Consume the fields a newer producer appended to `entity_type`, if the
/// schema describes it; a no-op otherwise.
/// schema describes it; a no-op otherwise. Each step is charged to `r`, so every
/// program run on one stream shares [`crate::MAX_SCHEMA_STEPS`].
///
/// # Errors
/// [`PrcError::Unsupported`] for a token that needs exact geometry;
/// [`PrcError::Malformed`] for an ill-formed program or one past the step
/// or depth ceiling; any read error from `r`.
/// [`PrcError::Malformed`] for an ill-formed program, one past the depth ceiling, or
/// one that runs out of the stream's steps; any read error from `r`.
pub fn skip_added_fields(&self, entity_type: u32, r: &mut BitReader<'_>) -> Result<(), PrcError> {
let Some(program) = self.programs.get(&entity_type) else {
return Ok(());
};
let mut exec = Exec { schema: self, r, vars: HashMap::new(), loop_counter: 0, steps: 0 };
let mut exec = Exec { schema: self, r, vars: HashMap::new(), loop_counter: 0 };
exec.program(program, 0)
}
}
@@ -100,7 +97,6 @@ struct Exec<'s, 'r, 'd> {
r: &'r mut BitReader<'d>,
vars: HashMap<u32, i64>,
loop_counter: i64,
steps: u64,
}
fn malformed(what: &str) -> PrcError {
@@ -116,8 +112,7 @@ fn next(prog: &[u32], pos: &mut usize) -> Result<u32, PrcError> {
impl Exec<'_, '_, '_> {
fn tick(&mut self, depth: u32) -> Result<(), PrcError> {
self.steps += 1;
if self.steps > MAX_STEPS {
if !self.r.take_step() {
return Err(malformed("step ceiling reached"));
}
if depth > MAX_DEPTH {
@@ -526,6 +521,18 @@ mod tests {
assert!(matches!(run(&[(172, &[23, 0, 30, 26, 1, 26, 0])], &d).0, Err(PrcError::Malformed(_))));
}
/// Every program run on one reader draws on one step allowance.
#[test]
fn programs_on_one_stream_share_the_step_allowance() {
// For (2^19) { empty block }: about 2^20 steps, so the 2^22 allowance fits three runs.
let bytes = schema(&[(172, &[15, 26, 1 << 19, 19, 21])]).bytes();
let mut r = BitReader::new(&bytes);
let s = Schema::read(&mut r).unwrap();
let runs = (0..5).take_while(|_| s.skip_added_fields(172, &mut r).is_ok()).count();
assert_eq!(runs, 3);
assert!(matches!(s.skip_added_fields(172, &mut r), Err(PrcError::Malformed(m)) if m.contains("step ceiling")));
}
#[test]
fn a_duplicated_entity_type_is_malformed() {
let bytes = schema(&[(172, &[3]), (172, &[3])]).bytes();
+4 -4
View File
@@ -155,8 +155,8 @@ impl Ctx<'_, '_> {
}
let count = if per_segment { count / 2 } else { count };
let bytes = if rgba { 4 } else { 3 };
// Every colour but the first is at least its reuse bit.
if count.saturating_mul(1 + 8 * bytes) > self.r.remaining().saturating_add(8 * bytes) {
// The first colour is `8 * bytes` bits; every later one is at least its reuse bit.
if count > 0 && (count - 1).saturating_add(8 * bytes) > self.r.remaining() {
return Err(PrcError::Truncated("vertex colours"));
}
for i in 0..count {
@@ -393,7 +393,7 @@ type UvSets = Vec<Vec<Option<[u32; 3]>>>;
/// indices (a slot is a `Double` offset, so even); a triangle with a slot naming no pair is
/// `None`.
fn stored_uvs(coords: &[f64], sets: UvSets) -> (Vec<[f64; 2]>, UvSets) {
let uvs: Vec<[f64; 2]> = coords.chunks_exact(2).map(|c| [c.first().copied().unwrap_or(0.0), c.get(1).copied().unwrap_or(0.0)]).collect();
let uvs: Vec<[f64; 2]> = coords.as_chunks::<2>().0.to_vec();
if uvs.is_empty() {
return (Vec::new(), Vec::new());
}
@@ -404,7 +404,7 @@ fn stored_uvs(coords: &[f64], sets: UvSets) -> (Vec<[f64; 2]>, UvSets) {
}
fn points(coords: &[f64]) -> Vec<[f64; 3]> {
coords.chunks_exact(3).map(|c| [0, 1, 2].map(|i| c.get(i).copied().unwrap_or(0.0))).collect()
coords.as_chunks::<3>().0.to_vec()
}
/// The point a `Double`-offset index names [WD 7.8.5.2: multiples of 3].
+19
View File
@@ -310,6 +310,25 @@ fn a_wire_without_indices_is_the_coordinate_polyline() {
assert!(matches!(t[1], Tessellation::Wire(_)));
}
/// Reused colours cost one bit each, so a long run of them may end the section.
#[test]
fn reused_vertex_colours_may_be_the_last_data() {
let n = 40;
let mut b = W::default();
b.uint(175).bit(false).uint(3 * n);
for i in 0..3 * n {
b.double(f64::from(i));
}
// No indices: the polyline of all `n` points, one RGB colour then `n - 1` reuses.
b.uint(0).bit(true).bit(false).bit(false).bit(false).put(1, 24);
for _ in 1..n {
b.bit(true);
}
let t = decode(&section(1, &b), &Schema::default(), 8137).unwrap();
let Tessellation::Wire(w) = &t[0] else { panic!("not a wire") };
assert_eq!(w[0].len(), n as usize);
}
/// A square as one triangle plus one 4-vertex fan (two entities, three triangles), the face
/// carrying `attrs` and, when it has any, behaviour 0x30; then `colours` per-point colours
/// and a trailing wire.
+1 -4
View File
@@ -237,10 +237,7 @@ fn emit(shape: Shape, slots: &Slots, out: &mut Vec<[u32; 3]>, corners: &mut Corn
/// Append the triangles `shape` makes of `verts`.
fn assemble(shape: Shape, verts: &[u32], out: &mut Vec<[u32; 3]>) {
match shape {
Shape::Triangles => out.extend(verts.chunks_exact(3).filter_map(|c| match c {
[a, b, c] => Some([*a, *b, *c]),
_ => None,
})),
Shape::Triangles => out.extend(verts.as_chunks::<3>().0.iter().copied()),
Shape::Fan => {
if let Some((&c, rim)) = verts.split_first() {
out.extend(rim.windows(2).filter_map(|w| match w {
+20 -19
View File
@@ -9,7 +9,7 @@
use std::sync::Arc;
use crate::bits::{BitReader, MAX_DECODED_ELEMENTS};
use crate::bits::{BitReader, Budget};
use crate::container::offset;
use crate::entity::{Ctx, malformed};
use crate::geometry::{Matrix, Vec3};
@@ -231,8 +231,8 @@ impl PrcFile {
/// the file stores no tree ([`Self::stores_tree`]).
///
/// # Errors
/// [`PrcError::Unsupported`] when an item is placed by a reference coordinate system and
/// the globals carry fonts or fill patterns; [`PrcError::Malformed`] for an index naming no entity,
/// [`PrcError::Unsupported`] when an item or an entity reference names a reference
/// coordinate system and the globals carry fonts or fill patterns; [`PrcError::Malformed`] for an index naming no entity,
/// a cycle of occurrences or prototypes, or a tree past [`MAX_PLACEMENTS`] or the depth
/// ceiling; read errors as [`crate::FileStructure::tessellations`].
pub fn placements(&self) -> Result<Vec<Placement>, PrcError> {
@@ -269,45 +269,40 @@ impl PrcFile {
}
fn walk(&self, rule: StyleAlpha, scope: EntityOverrides) -> Result<(Vec<Placement>, Vec<ModelNode>), PrcError> {
self.walk_within(rule, scope, MAX_DECODED_ELEMENTS)
self.walk_within(rule, scope, Budget::FULL)
}
/// [`Self::walk`] with `elements` decoded array elements allowed across all its readers.
pub(crate) fn walk_within(
&self,
rule: StyleAlpha,
scope: EntityOverrides,
elements: usize,
) -> Result<(Vec<Placement>, Vec<ModelNode>), PrcError> {
/// [`Self::walk`] with `budget` shared by all its readers.
pub(crate) fn walk_within(&self, rule: StyleAlpha, scope: EntityOverrides, budget: Budget) -> Result<(Vec<Placement>, Vec<ModelNode>), PrcError> {
if !self.stores_tree() {
return Ok((Vec::new(), Vec::new()));
}
// One element allowance for the whole walk, so many sections cannot each decode a full one.
let mut left = elements;
// One allowance for the whole walk, so many sections cannot each spend a full one.
let mut left = budget;
let mut trees = Vec::with_capacity(self.file_structures.len());
for fs in &self.file_structures {
let schema = fs.schema()?;
let mut ctx = Ctx::new(BitReader::with_elements(fs.section(SectionKind::Tree), left), &schema, fs.authoring_version);
let mut ctx = Ctx::new(BitReader::with_budget(fs.section(SectionKind::Tree), left), &schema, fs.authoring_version);
let tree = ctx.file_structure_tree()?;
left = ctx.r.elements_left();
left = ctx.r.budget();
// The globals carry the coordinate systems and the colours; colours alone are not
// worth refusing the placements over.
let globals = (|| {
let mut r = BitReader::with_elements(fs.section(SectionKind::Globals), left);
let mut r = BitReader::with_budget(fs.section(SectionKind::Globals), left);
Schema::read(&mut r)?;
let mut ctx = Ctx::new(r, &schema, fs.authoring_version);
let globals = ctx.globals();
left = ctx.r.elements_left();
left = ctx.r.budget();
globals
})();
let globals = match globals {
Ok(g) => g,
Err(e) if tree.parts.iter().flatten().any(|i| !i.local.is_empty()) => return Err(e),
Err(e) if places_by_system(&tree) => return Err(e),
Err(_) => Globals::default(),
};
trees.push((fs.id, tree, globals));
}
let mut r = BitReader::with_elements(&self.model_file, left);
let mut r = BitReader::with_budget(&self.model_file, left);
let schema = Schema::read(&mut r)?;
let model = Ctx::new(r, &schema, self.header.authoring_version).model_file()?;
let mut walk = Walk::new(trees.iter().map(|(id, t, g)| (*id, t, g)).collect(), rule, scope);
@@ -321,6 +316,12 @@ impl PrcFile {
}
}
/// Whether an item of `tree`, or an entity reference one of its occurrences holds, names a
/// coordinate system: then the globals that store them must have been read.
fn places_by_system(tree: &Tree) -> bool {
tree.parts.iter().flatten().any(|i| !i.local.is_empty()) || tree.products.iter().flat_map(|p| &p.overrides).any(|o| o.local != 0)
}
#[cfg(test)]
mod markup_tests;
#[cfg(test)]
+59 -6
View File
@@ -1,8 +1,11 @@
#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
use super::style::{Material, Style};
use super::{EntityOverrides, FATHER_HERIT_COLOR, Globals, Graphics, MAX_PLACEMENTS, NameSource, SHOW, SON_HERIT_COLOR, StyleAlpha, resolve_style};
use crate::bits::BitReader;
use super::{
EntityOverrides, FATHER_HERIT_COLOR, FILE_STRUCTURE_TREE, Globals, Graphics, MAX_PLACEMENTS, NameSource, SHOW, SON_HERIT_COLOR, StyleAlpha,
resolve_style,
};
use crate::bits::{BitReader, Budget};
use crate::entity::Ctx;
use crate::tess::tests::two_face_square;
use crate::testw::W;
@@ -191,8 +194,14 @@ fn material(diffuse: u32, alpha: f64) -> W {
/// A globals section (schema included) with `colours`, `materials`, `styles` and reference
/// coordinate systems `systems` (transformations).
fn globals(colours: &[Rgba], materials: &[W], styles: &[W], systems: &[W]) -> W {
let mut w = W::default();
w.uint(0).uint(303);
let mut schema = W::default();
schema.uint(0);
globals_after(schema, colours, materials, styles, systems)
}
/// [`globals`] after the already-written `schema`.
fn globals_after(mut w: W, colours: &[Rgba], materials: &[W], styles: &[W], systems: &[W]) -> W {
w.uint(303);
base(&mut w, None);
w.uint(0).double(0.).double(0.).string(None).uint(0);
w.uint(len(colours.len()));
@@ -238,10 +247,15 @@ fn translate_scale(t: [f64; 3], s: f64) -> W {
/// An entity reference giving the item with PRC unique id `uid` the graphics `g`.
fn ref_entity(uid: u32, g: (u32, u16)) -> W {
ref_entity_at(uid, g, 0)
}
/// [`ref_entity`], also placing its target by biased coordinate system `local`.
fn ref_entity_at(uid: u32, g: (u32, u16), local: u32) -> W {
let mut w = W::default();
w.uint(203);
graphics(&mut w, None, 0, g);
w.uint(0).bit(true).uint(205).uint(RI_POLY_BREP).bit(true).uint(uid).uint(0);
w.uint(local).bit(true).uint(205).uint(RI_POLY_BREP).bit(true).uint(uid).uint(0);
w
}
@@ -408,7 +422,7 @@ fn one_element_allowance_covers_the_whole_walk() {
let fs = || Fs { globals: palette(), tree: tree(&[wide()], &[Occ { part: 1, ..Occ::default() }]), tess: two_face_square() };
let one = PrcFile::parse(&prc(&[fs()], &model(&[(0, 1)]))).unwrap();
let two = PrcFile::parse(&prc(&[fs(), fs()], &model(&[(0, 1), (1, 1)]))).unwrap();
let walk = |f: &PrcFile, n| f.walk_within(StyleAlpha::default(), EntityOverrides::default(), n);
let walk = |f: &PrcFile, n| f.walk_within(StyleAlpha::default(), EntityOverrides::default(), Budget { elements: n, ..Budget::FULL });
let short = |f: &PrcFile, n| matches!(walk(f, n), Err(PrcError::TooManyElements { .. }));
// One structure decodes about 70 elements, two about 140.
assert!(short(&one, 32));
@@ -417,6 +431,45 @@ fn one_element_allowance_covers_the_whole_walk() {
assert_eq!(walk(&two, 200).map(|w| w.0.len()), Ok(128));
}
/// One schema-step allowance covers the whole walk: each structure's tree runs a 2,000-step
/// program, which a 3,000-step allowance fits once but not twice.
#[test]
fn one_step_allowance_covers_the_whole_walk() {
let fs = || {
let mut schema = W::default();
// For (1000) { empty block } on the file structure tree.
schema.uint(1).uint(FILE_STRUCTURE_TREE).uint(5);
for t in [15, 26, 1000, 19, 21] {
schema.uint(t);
}
let globals = globals_after(schema, &[RED], &[], &[], &[]);
Fs { globals, tree: tree(&[plate(S_RED)], &[Occ { part: 1, ..Occ::default() }]), tess: two_face_square() }
};
let one = PrcFile::parse(&prc(&[fs()], &model(&[(0, 1)]))).unwrap();
let two = PrcFile::parse(&prc(&[fs(), fs()], &model(&[(0, 1), (1, 1)]))).unwrap();
let walk = |f: &PrcFile| f.walk_within(StyleAlpha::default(), EntityOverrides::default(), Budget { steps: 3000, ..Budget::FULL });
assert_eq!(walk(&one).map(|w| w.0.len()), Ok(1));
assert!(matches!(walk(&two), Err(PrcError::Malformed(m)) if m.contains("step ceiling")));
assert_eq!(two.placements().map(|p| p.len()), Ok(2));
}
/// Globals that cannot be read are reported, not replaced by empty ones, when an entity
/// reference names one of their coordinate systems.
#[test]
fn unreadable_globals_are_an_error_when_a_reference_names_a_coordinate_system() {
let file = |local| {
let mut fonts = W::default();
fonts.uint(0).uint(303);
base(&mut fonts, None);
fonts.uint(0).double(0.).double(0.).string(None).uint(1);
let occs = [Occ { part: 1, refs: vec![ref_entity_at(7, PLAIN, local)], ..Occ::default() }];
let fs = Fs { globals: fonts, tree: tree(&[plate(S_RED)], &occs), tess: two_face_square() };
PrcFile::parse(&prc(&[fs], &model(&[(0, 1)]))).unwrap()
};
assert_eq!(file(0).placements().map(|p| p.len()), Ok(1));
assert!(matches!(file(1).placements(), Err(PrcError::Unsupported("PRC global fonts"))));
}
#[test]
fn an_exponential_walk_is_cut_off() {
// Occurrence k has occurrence k + 1 twice as sons: 2^21 visits.