Skip to content

Return an error for out-of-range indices and non-finite coordinates in Manifold::new - #18

Merged
komietty merged 1 commit into
komietty:mainfrom
Erry91:fix/validate-input-indices-and-coordinates
Oct 27, 2026
Merged

komietty merged 1 commit into
komietty:mainfrom
Erry91:fix/validate-input-indices-and-coordinates

Conversation

@Erry91

@Erry91 Erry91 commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Manifold::new / new_with_values return Result, but two kinds of malformed input still panic or are silently misread. This turns both into an Err, like the existing checks for boundary edges and buffer lengths.

1. Indices outside the vertex range

The remap table rmap was sized by pos.len() (three entries per vertex) rather than by the vertex count, so:

  • an index >= pos.len() indexes past the end of rmap and panics (src/manifold/mod.rs:64);
  • an index between the vertex count and pos.len() is silently read as vertex 0, because those rmap entries are never written.
// unit tetrahedron, 4 vertices, pos.len() == 12
let pos = [0., 0., 0., 1., 0., 0., 0., 1., 0., 0., 0., 1.];
let mut idx = [0, 2, 1, 0, 1, 3, 0, 3, 2, 1, 2, 3];
idx[0] = 99; Manifold::new(&pos, &idx); // panics: index out of bounds
idx[0] = 5;  Manifold::new(&pos, &idx); // Ok, vertex 5 taken as vertex 0

Now: Err("idx must refer to vertices in pos"), and rmap is sized by the vertex count.

2. NaN and infinite coordinates

A NaN coordinate was accepted by new and made the next compute_boolean panic. Non-finite coordinates (checked after the cast to Real, so this also catches f64 values that overflow f32 under the f32 feature) now give Err("pos must not contain NaN or infinite coordinates").

Tests

Four tests in a new test_input_validation module at the end of src/tests.rs, with the other tests (placed after the last module, so it does not conflict with #16, which adds to test_mesh_cleanup): a valid tetrahedron is accepted; index 99 and index 5 are errors; NaN, +∞ and −∞ are errors. The three error tests fail on main and pass with the change. cargo test passes for every feature set in CI ("", rayon, f32, f32,rayon), and cargo clippy reports the same warnings as on main.

Relation to #16

Complementary: #16 handles edges shared by more than two faces, this handles indices and coordinates. The two merge cleanly. With both, every malformed input in the probe I ran returns an Err: out-of-range index, NaN, ±∞, empty mesh, a single open triangle, a degenerate triangle, a flipped triangle, a duplicate face, the two tetrahedra from #14, and two cubes touching on an edge. No input panics.

Context: we feed boolmesh meshes imported from building models, which can be corrupt; an Err lets the caller report and skip the element instead of losing the process.

…n Manifold::new

An index at or past the vertex count either indexed past the end of the
remap table (index >= pos.len(): panic) or was silently read as vertex 0
(vertex count <= index < pos.len(), since the table was sized by pos.len()).
A NaN coordinate was accepted and made the following boolean panic.

new_with_values now rejects both with an Err, like the other malformed
inputs, and sizes the remap table by the vertex count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Erry91
Erry91 force-pushed the fix/validate-input-indices-and-coordinates branch from 6727991 to 2a63480 Compare October 17, 2026 10:16
@komietty

Copy link
Copy Markdown
Owner

LGTM. Thank you for your PR.

@komietty
komietty merged commit 5a65a2b into komietty:main Oct 27, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants