M12: review fixes from the renderer PR's review of the reader

- Huffman leaf tables where one code begins another are malformed; the
  shorter leaf would end the longer code early and misalign the payload.
- A model-file end past the input is malformed rather than clipped.
- Each schema entry costs one element, so empty programs are not free.
- tessellations() carries the globals schema's remaining allowance into
  the tessellation reader: one stream, one budget.
- A face behaviour wider than 16 bits is malformed, not zeroed.
- Coordinate arrays that are not whole points are malformed; a partial
  last normal drops the stored normals, as an unusable normal slot does.
- Triangles a TESS_3D's faces emit are charged to the element budget as
  they are made, since faces may reuse one index range.
- TriangleMesh::transformed drops stored normals under a projective
  matrix (last row not [0, 0, 0, w]).
- Placement::tessellation's doc says placements() does not check it
  against the tessellation section.

Each fix has a test, seen failing with the fix reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9

crates/prc/src/arrays.rs

@@ -126,6 +126,12 @@ fn huffman_leaves(b: &mut Lsb<'_>, nbits: u32, signed: bool) -> Result<HashMap<(
             return Err(malformed("duplicate Huffman code"));
         }
     }
+    // A leaf whose code begins another's would end that code early, leaving its unread bits
+    // in front of the next element.
+    let shadowed = codes.keys().any(|&(len, code)| (1..len).any(|l| codes.contains_key(&(l, code >> (len - l)))));
+    if shadowed {
+        return Err(malformed("a Huffman code that is a prefix of another"));
+    }
     Ok(codes)
 }
 
@@ -282,6 +288,16 @@ mod tests {
         assert!(matches!(huffman(&mut BitReader::new(&bytes), 2, false), Err(PrcError::Truncated("a Huffman element count"))));
     }
 
+    #[test]
+    fn a_huffman_code_that_begins_another_is_malformed() {
+        // `10` would end every `100` one bit early.
+        let mut w = W::default();
+        w.huffman(2, 2, &[(1, 0b10, 2), (2, 0b100, 3)], &[2, 1]);
+        let bytes = w.bytes();
+        let r = huffman(&mut BitReader::new(&bytes), 2, false);
+        assert!(malformed_by(r, "a Huffman code that is a prefix of another"));
+    }
+
     #[test]
     fn huffman_widths_out_of_range_are_errors() {
         // Well-formed payloads at each width, which only the width check refuses.

crates/prc/src/container.rs

@@ -314,13 +314,16 @@ impl PrcFile {
         if mf_end < mf_start {
             return Err(PrcError::Malformed("model file ends before it starts".into()));
         }
+        if mf_end > data.len() {
+            return Err(PrcError::Malformed(format!("model file ends at {mf_end}, past the {}-byte stream", data.len())));
+        }
 
         let mut budget = Budget { left: limit, limit };
         // prc-rs: the last section runs to the model file's start.
         let last = mf_start.min(data.len());
         let file_structures =
             descriptions.iter().map(|(id, offs)| file_structure(data, *id, &Extent { offs, last }, &mut budget)).collect::<Result<Vec<_>, _>>()?;
-        let model_file = budget.inflate(data, (mf_start, mf_end.min(data.len())), "model file")?;
+        let model_file = budget.inflate(data, (mf_start, mf_end), "model file")?;
         Ok(PrcFile { header, file_structures, model_file, uncompressed_files })
     }
 }

crates/prc/src/geometry.rs

@@ -74,7 +74,8 @@ impl Matrix {
 impl TriangleMesh {
     /// This mesh with every position mapped through `m` and every stored normal through its
     /// inverse transpose (renormalised); a mirroring `m` (negative determinant) reverses each
-    /// triangle so the outside stays counter-clockwise.
+    /// triangle so the outside stays counter-clockwise. A projective `m` (a last row other than
+    /// `[0, 0, 0, w]`) bends the surface per point, so the stored normals are dropped.
     ///
     /// ```
     /// use pdfcraft_prc::{Matrix, TriangleMesh};
@@ -97,6 +98,10 @@ impl TriangleMesh {
         // and the result renormalised.
         let det = m.det3();
         let sign = if det < 0.0 { -1.0 } else { 1.0 };
+        if m.at(3, 0) != 0.0 || m.at(3, 1) != 0.0 || m.at(3, 2) != 0.0 {
+            out.normals.clear();
+            out.triangle_normals.clear();
+        }
         for n in &mut out.normals {
             let [x, y, z] = *n;
             let v = [0, 1, 2].map(|i| sign * (m.cofactor(i, 0) * x + m.cofactor(i, 1) * y + m.cofactor(i, 2) * z));
@@ -171,6 +176,25 @@ mod tests {
         assert_eq!(placed.normals, [[1.0, 0.0, 0.0]]);
     }
 
+    #[test]
+    fn a_projective_transform_drops_the_stored_normals() {
+        let mesh = TriangleMesh {
+            positions: vec![[0.0; 3], [1.0, 0.0, 0.0], [0.0, 0.0, 1.0]],
+            triangles: vec![[0, 1, 2]],
+            normals: vec![[1.0, 0.0, 1.0]],
+            triangle_normals: vec![[0, 0, 0]],
+            ..TriangleMesh::default()
+        };
+        let mut projective = Matrix::IDENTITY;
+        projective.0[3][0] = 1.0;
+        let placed = mesh.transformed(&projective);
+        assert!(placed.normals.is_empty() && placed.triangle_normals.is_empty());
+        // A uniform w keeps them.
+        let mut uniform = Matrix::IDENTITY;
+        uniform.0[3][3] = 2.0;
+        assert_eq!(mesh.transformed(&uniform).triangle_normals, [[0, 0, 0]]);
+    }
+
     impl Matrix {
         fn transformed_normal(&self, mesh: &TriangleMesh) -> [f64; 3] {
             mesh.transformed(self).normals.first().copied().unwrap_or_default()

crates/prc/src/schema.rs

@@ -41,7 +41,8 @@ impl Schema {
     ///
     /// # Errors
     /// [`PrcError::Truncated`] when a count runs past the data;
-    /// [`PrcError::Malformed`] for an entity type listed twice.
+    /// [`PrcError::Malformed`] for an entity type listed twice; [`PrcError::TooManyElements`]
+    /// past the reader's allowance, one element per entry and per token.
     pub fn read(r: &mut BitReader<'_>) -> Result<Self, PrcError> {
         let n = r.unsigned_integer()? as usize;
         // Every entry takes at least two bits.
@@ -50,6 +51,8 @@ impl Schema {
         }
         let mut programs = HashMap::new();
         for _ in 0..n {
+            // The entry itself costs one element, so empty programs are not free.
+            r.take_elements(1)?;
             let entity_type = r.unsigned_integer()?;
             let len = r.unsigned_integer()? as usize;
             if len > r.remaining() {
@@ -336,12 +339,16 @@ mod tests {
         w
     }
 
-    /// A schema program's tokens are charged against the reader's element allowance.
+    /// Each schema entry and its program's tokens are charged against the reader's element
+    /// allowance.
     #[test]
     fn program_tokens_are_charged_against_the_allowance() {
         let bytes = schema(&[(172, &[1, 2, 3])]).bytes();
-        assert!(matches!(Schema::read(&mut BitReader::with_elements(&bytes, 2)), Err(PrcError::TooManyElements { .. })));
-        assert!(Schema::read(&mut BitReader::with_elements(&bytes, 3)).is_ok());
+        assert!(matches!(Schema::read(&mut BitReader::with_elements(&bytes, 3)), Err(PrcError::TooManyElements { .. })));
+        assert!(Schema::read(&mut BitReader::with_elements(&bytes, 4)).is_ok());
+        let empty = schema(&[(172, &[]), (173, &[]), (174, &[])]).bytes();
+        assert!(matches!(Schema::read(&mut BitReader::with_elements(&empty, 2)), Err(PrcError::TooManyElements { .. })));
+        assert!(Schema::read(&mut BitReader::with_elements(&empty, 3)).is_ok());
     }
 
     fn run(entries: &[(u32, &[u32])], data: &W) -> (Result<(), PrcError>, usize) {

crates/prc/src/tess.rs

@@ -11,7 +11,7 @@
 mod triangulate;
 
 use crate::arrays;
-use crate::bits::BitReader;
+use crate::bits::{BitReader, Budget};
 use crate::entity::{Ctx, malformed};
 use crate::{FileStructure, PrcError, SectionKind};
 use triangulate::{Corners, Face};
@@ -109,8 +109,14 @@ impl FileStructure {
     /// [`PrcError::Malformed`] for a damaged section; [`PrcError::Unsupported`] for
     /// optimised vertex colours.
     pub fn tessellations(&self) -> Result<Vec<Tessellation>, PrcError> {
-        let schema = self.schema()?;
-        let r = BitReader::new(self.section(SectionKind::Tessellation));
+        self.tessellations_within(Budget::FULL)
+    }
+
+    /// As [`Self::tessellations`], the globals' schema and the section sharing `budget`.
+    pub(crate) fn tessellations_within(&self, budget: Budget) -> Result<Vec<Tessellation>, PrcError> {
+        let mut globals = BitReader::with_budget(self.section(SectionKind::Globals), budget);
+        let schema = crate::Schema::read(&mut globals)?;
+        let r = BitReader::with_budget(self.section(SectionKind::Tessellation), globals.budget());
         Ctx::new(r, &schema, self.authoring_version).file_structure_tessellation()
     }
 }
@@ -193,12 +199,14 @@ impl Ctx<'_, '_> {
         let texture = self.doubles("texture coordinates")?;
         self.schema.skip_added_fields(TESS_3D, &mut self.r)?;
 
-        let mut mesh = TriangleMesh { positions: points(&coords), normals_recalculated: recalc, ..TriangleMesh::default() };
+        let mut mesh = TriangleMesh { positions: points(&coords)?, normals_recalculated: recalc, ..TriangleMesh::default() };
         let styled = faces.iter().any(|f| !f.styles.is_empty());
         let mut corners = Corners::default();
         for f in &faces {
             let start = mesh.triangles.len();
             let entities = f.triangulate(&indices, !recalc, mesh.positions.len(), &mut mesh.triangles, &mut corners)?;
+            // Faces may reuse one index range, so the triangles are charged as they are made.
+            self.r.take_elements(mesh.triangles.len().saturating_sub(start))?;
             mesh.faces.push(start..mesh.triangles.len());
             if styled {
                 f.styles_into(&entities, &mut mesh.triangle_styles);
@@ -224,7 +232,8 @@ impl Ctx<'_, '_> {
             self.vertex_colors(n, false)?;
         }
         if !face.styles.is_empty() {
-            face.behaviour = u16::try_from(self.r.unsigned_integer()?).unwrap_or(0);
+            let bits = self.r.unsigned_integer()?;
+            face.behaviour = u16::try_from(bits).map_err(|_| malformed(format!("face behaviour {bits} is wider than 16 bits")))?;
         }
         self.schema.skip_added_fields(TESS_FACE, &mut self.r)?;
         Ok(face)
@@ -232,7 +241,7 @@ impl Ctx<'_, '_> {
 
     /// `TESS_3D_Wire` (175) after its type code [WD 7.8.7].
     fn wire(&mut self) -> Result<Vec<Vec<[f64; 3]>>, PrcError> {
-        let pts = points(&self.base_tess_data()?);
+        let pts = points(&self.base_tess_data()?)?;
         let words = self.uints("wire indices")?;
         let mut wires: Vec<Vec<[f64; 3]>> = Vec::new();
         let mut colours = 0usize;
@@ -377,7 +386,10 @@ impl Ctx<'_, '_> {
 /// The normal array and each triangle's corner normals, or both empty when any slot fails to
 /// name a normal: a mesh shades from its stored normals wholly or not at all.
 fn stored_normals(coords: &[f64], slots: &[[u32; 3]], triangles: usize) -> (Vec<[f64; 3]>, Vec<[u32; 3]>) {
-    let normals = points(coords);
+    // Like a slot naming no normal, a partial last normal drops the stored normals.
+    let Ok(normals) = points(coords) else {
+        return (Vec::new(), Vec::new());
+    };
     let named = |k: u32| k.is_multiple_of(3) && (k as usize / 3) < normals.len();
     if normals.is_empty() || slots.len() != triangles || !slots.iter().flatten().all(|&k| named(k)) {
         return (Vec::new(), Vec::new());
@@ -403,8 +415,13 @@ fn stored_uvs(coords: &[f64], sets: UvSets) -> (Vec<[f64; 2]>, UvSets) {
     (uvs, sets)
 }
 
-fn points(coords: &[f64]) -> Vec<[f64; 3]> {
-    coords.as_chunks::<3>().0.to_vec()
+/// `coords` as points; an array that does not divide into whole points is malformed.
+fn points(coords: &[f64]) -> Result<Vec<[f64; 3]>, PrcError> {
+    let (whole, rest) = coords.as_chunks::<3>();
+    if !rest.is_empty() {
+        return Err(malformed(format!("{} coordinates are not whole points", coords.len())));
+    }
+    Ok(whole.to_vec())
 }
 
 /// The point a `Double`-offset index names [WD 7.8.5.2: multiples of 3].

crates/prc/src/tess/tests.rs

@@ -24,8 +24,12 @@ fn uints(w: &mut W, v: &[u32]) {
 }
 
 fn square(w: &mut W, entity: u32) {
-    w.uint(entity).bit(false).uint(12);
-    for c in SQUARE {
+    coordinates(w, entity, &SQUARE);
+}
+
+fn coordinates(w: &mut W, entity: u32, coords: &[f64]) {
+    w.uint(entity).bit(false).uint(u32::try_from(coords.len()).unwrap());
+    for &c in coords {
         w.double(c);
     }
 }
@@ -37,11 +41,13 @@ struct Mesh<'a> {
     faces: &'a [(u32, u32, &'a [u32])],
     textures: u32,
     uvs: &'a [f64],
+    /// The coordinates; [`SQUARE`] when `None`.
+    coords: Option<&'a [f64]>,
 }
 
 /// A TESS_3D over [`SQUARE`] at authoring version `v`.
 fn tess_3d(w: &mut W, v: u32, m: &Mesh<'_>) {
-    square(w, 172);
+    coordinates(w, 172, m.coords.unwrap_or(&SQUARE));
     w.bit(true);
     if v >= 7039 {
         w.bit(false);
@@ -333,6 +339,11 @@ fn reused_vertex_colours_may_be_the_last_data() {
 /// carrying `attrs` and, when it has any, behaviour 0x30; then `colours` per-point colours
 /// and a trailing wire.
 fn styled_face(attrs: &[u32], colours: usize) -> Vec<Tessellation> {
+    decode(&styled_face_section(attrs, colours, 0x30), &Schema::default(), 8137).unwrap()
+}
+
+/// [`styled_face`]'s section, the face's behaviour `behaviour`.
+fn styled_face_section(attrs: &[u32], colours: usize, behaviour: u32) -> W {
     let mut b = W::default();
     square(&mut b, 172);
     b.bit(true).bit(false).bit(false).uint(0).uint(0);
@@ -349,11 +360,11 @@ fn styled_face(attrs: &[u32], colours: usize) -> Vec<Tessellation> {
         }
     }
     if !attrs.is_empty() {
-        b.uint(0x30);
+        b.uint(behaviour);
     }
     b.uint(0);
     wire(&mut b, false);
-    decode(&section(2, &b), &Schema::default(), 8137).unwrap()
+    section(2, &b)
 }
 
 #[test]
@@ -576,6 +587,13 @@ fn every_truncation_of_a_section_is_an_error_not_a_panic() {
 /// A one-file-structure PRC stream whose tessellation section is `tess`, with an empty
 /// globals section schema.
 fn prc_file(tess: &[u8]) -> Vec<u8> {
+    let mut globals = W::default();
+    globals.uint(0);
+    prc_file_with(&globals.bytes(), tess)
+}
+
+/// As [`prc_file`] with the globals section `globals`.
+fn prc_file_with(globals: &[u8], tess: &[u8]) -> Vec<u8> {
     use std::io::Write as _;
     const HEADER_LEN: usize = 107;
     let zlib = |data: &[u8]| {
@@ -584,9 +602,7 @@ fn prc_file(tess: &[u8]) -> Vec<u8> {
         e.finish().unwrap()
     };
     let u32s = |out: &mut Vec<u8>, vs: &[u32]| vs.iter().for_each(|v| out.extend_from_slice(&v.to_le_bytes()));
-    let mut globals = W::default();
-    globals.uint(0);
-    let sections: [&[u8]; 5] = [&globals.bytes(), b"", tess, b"", b""];
+    let sections: [&[u8]; 5] = [globals, b"", tess, b"", b""];
     let mut body = b"PRC".to_vec();
     u32s(&mut body, &[8137, 8137, 5, 6, 7, 8, 0, 0, 0, 0, 0]);
     let mut offs = vec![HEADER_LEN];
@@ -631,3 +647,58 @@ fn counted_entity_arrays_are_charged_against_the_allowance() {
     assert!(matches!(read(2), Err(PrcError::TooManyElements { .. })));
     assert_eq!(read(3).unwrap(), [1, 2, 3]);
 }
+
+#[test]
+fn a_face_behaviour_wider_than_16_bits_is_malformed() {
+    let r = decode(&styled_face_section(&[1], 0, 0x1_0030), &Schema::default(), 8137);
+    assert!(matches!(r, Err(PrcError::Malformed(m)) if m == "face behaviour 65584 is wider than 16 bits"));
+}
+
+#[test]
+fn coordinates_that_are_not_whole_points_are_malformed() {
+    let mut partial = SQUARE.to_vec();
+    partial.push(7.0);
+    let mut b = W::default();
+    let faces: &[(u32, u32, &[u32])] = &[(0x2, 0, &[1])];
+    tess_3d(&mut b, 8137, &Mesh { indices: &[0, 0, 0, 3, 0, 6], faces, coords: Some(&partial), ..Mesh::default() });
+    let r = decode(&section(1, &b), &Schema::default(), 8137);
+    assert!(matches!(r, Err(PrcError::Malformed(m)) if m == "13 coordinates are not whole points"));
+    // A partial last normal drops the stored normals, as a slot naming none does.
+    assert_eq!(stored_normals(&[0.0, 0.0, 1.0, 0.0], &[[0, 0, 0]], 1), (Vec::new(), Vec::new()));
+}
+
+/// Elements a tessellation section spends from a full allowance.
+fn spent(section: &W) -> usize {
+    let bytes = section.bytes();
+    let schema = Schema::default();
+    let mut ctx = Ctx::new(BitReader::new(&bytes), &schema, 8137);
+    ctx.file_structure_tessellation().unwrap();
+    crate::MAX_DECODED_ELEMENTS - ctx.r.elements_left()
+}
+
+#[test]
+fn faces_reusing_one_index_range_are_charged_for_each_triangle_they_make() {
+    const INDICES: [u32; 12] = [0, 0, 0, 3, 0, 6, 0, 0, 0, 6, 0, 9];
+    let square = |faces: &[(u32, u32, &[u32])]| {
+        let mut b = W::default();
+        tess_3d(&mut b, 8137, &Mesh { indices: &INDICES, faces, ..Mesh::default() });
+        spent(&section(1, &b))
+    };
+    let one = square(&[(0x2, 0, &[2])]);
+    let three = square(&[(0x2, 0, &[2]), (0x2, 0, &[2]), (0x2, 0, &[2])]);
+    // Per extra face: its count entry, its one data element and its two triangles.
+    assert_eq!(three - one, 2 * 4);
+}
+
+#[test]
+fn the_globals_schema_and_the_tessellations_share_one_allowance() {
+    // A schema of one entry with three tokens: four elements.
+    let mut globals = W::default();
+    globals.uint(1).uint(999).uint(3).uint(0).uint(0).uint(0);
+    let tess = textured_square();
+    let f = crate::PrcFile::parse(&prc_file_with(&globals.bytes(), &tess.bytes())).unwrap();
+    let need = spent(&tess) + 4;
+    let within = |elements| f.file_structures[0].tessellations_within(crate::bits::Budget { elements, ..crate::bits::Budget::FULL });
+    assert!(within(need).is_ok());
+    assert!(matches!(within(need - 1), Err(PrcError::TooManyElements { .. })));
+}

crates/prc/src/tree/mod.rs

@@ -112,7 +112,9 @@ type Palettes = Arc<[Arc<[Option<Paint>]>]>;
 pub struct Placement {
     /// Index into [`PrcFile::file_structures`].
     pub file_structure: usize,
-    /// Index into that file structure's [`tessellations`](crate::FileStructure::tessellations).
+    /// Index into that file structure's [`tessellations`](crate::FileStructure::tessellations),
+    /// as the tree stores it: [`PrcFile::placements`] does not decode that section, so an
+    /// index past its end is the caller's to refuse.
     pub tessellation: usize,
     /// The occurrence path's locations, then the representation item's coordinate systems.
     pub matrix: Matrix,
@@ -232,7 +234,7 @@ impl PrcFile {
     ///
     /// # Errors
     /// [`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,
+    /// coordinate system and the globals carry fonts or fill patterns; [`PrcError::Malformed`] for a tree 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> {

crates/prc/tests/container.rs

@@ -107,6 +107,11 @@ fn an_offset_or_count_past_the_data_is_an_error_not_a_panic() {
     bad[95..99].copy_from_slice(&u32::MAX.to_le_bytes());
     bad[99..103].copy_from_slice(&u32::MAX.to_le_bytes());
     assert!(PrcFile::parse(&bad).is_err());
+    // A model file ending one byte past the data, its zlib stream whole.
+    let mut bad = build(SECTIONS, b"m", 0);
+    let past = u32::try_from(bad.len() + 1).unwrap();
+    bad[99..103].copy_from_slice(&past.to_le_bytes());
+    assert!(matches!(PrcFile::parse(&bad), Err(PrcError::Malformed(_))));
     // A section count that claims more offsets than the data holds.
     let mut bad = build(SECTIONS, b"m", 0);
     bad[67..71].copy_from_slice(&u32::MAX.to_le_bytes());