diff --git a/crates/polymodel-ldraw-core/src/cache.rs b/crates/polymodel-ldraw-core/src/cache.rs index 5d544d0..b969fe3 100644 --- a/crates/polymodel-ldraw-core/src/cache.rs +++ b/crates/polymodel-ldraw-core/src/cache.rs @@ -8,8 +8,24 @@ use std::fmt; pub struct NormalizedPath(String); impl NormalizedPath { pub fn new(path: &str) -> Result { + let normalized_separators = path.replace('\\', "/"); + let bytes = normalized_separators.as_bytes(); + let drive_qualified = bytes.len() >= 2 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':'; + if normalized_separators.starts_with('/') + || normalized_separators.starts_with("//") + || drive_qualified + { + return Err(ParseError::Diagnostic(Diagnostic { + code: DiagnosticCode::PathForbiddenSyntax, + severity: Severity::Error, + message: "absolute include paths are forbidden".into(), + span: None, + })); + } let mut parts = Vec::new(); - for part in path.replace('\\', "/").split('/') { + for part in normalized_separators.split('/') { if part.is_empty() || part == "." { continue; } diff --git a/crates/polymodel-ldraw-core/src/lib.rs b/crates/polymodel-ldraw-core/src/lib.rs index 11247a1..035f6e6 100644 --- a/crates/polymodel-ldraw-core/src/lib.rs +++ b/crates/polymodel-ldraw-core/src/lib.rs @@ -33,6 +33,33 @@ mod tests { fn options<'a>(ledger: &'a ReservationLedger) -> ParseOptions<'a> { ParseOptions::new(ReservationOwner::preview(1, 1, 1), ledger) } + #[test] + fn normalized_paths_reject_rooted_include_forms() { + for path in ["/parts/a.dat", "\\\\server\\share\\a.dat", "C:/parts/a.dat", "z:\\parts\\a.dat"] { + assert!(matches!( + NormalizedPath::new(path), + Err(LdrawError::Diagnostic(Diagnostic { + code: DiagnosticCode::PathForbiddenSyntax, + .. + })) + ), "path should be rejected: {path}"); + } + assert_eq!(NormalizedPath::new("parts/./a.dat").unwrap().as_str(), "parts/a.dat"); + } + + #[test] + fn cancellation_zero_targets_next_checkpoint_and_default_is_disabled() { + let cancellation = CancellationPolicy::default(); + assert!(!cancellation.checkpoint()); + cancellation.cancel_after_checkpoints(0); + assert!(cancellation.checkpoint()); + + let cancellation = CancellationPolicy::default(); + cancellation.cancel_after_checkpoints(2); + assert!(!cancellation.checkpoint()); + assert!(cancellation.checkpoint()); + } + #[test] fn scanner_preserves_case_quotes_and_endings() { let lines = scan_lines( diff --git a/crates/polymodel-ldraw-core/src/types.rs b/crates/polymodel-ldraw-core/src/types.rs index 9506d1b..adf7862 100644 --- a/crates/polymodel-ldraw-core/src/types.rs +++ b/crates/polymodel-ldraw-core/src/types.rs @@ -341,13 +341,25 @@ impl LdrawLimits { /// The parser checks this signal at every phase boundary; it never snapshots a /// boolean before scanning. `AtomicBool` is available on native and WASM targets /// and does not require a host runtime or an async executor. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct CancellationPolicy { cancelled: Arc, checkpoints: Arc, + /// `u64::MAX` is the disabled sentinel; every other value is a 1-based + /// checkpoint threshold, including zero for the next checkpoint. cancel_after: Arc, } +impl Default for CancellationPolicy { + fn default() -> Self { + Self { + cancelled: Arc::new(AtomicBool::new(false)), + checkpoints: Arc::new(AtomicU64::new(0)), + cancel_after: Arc::new(AtomicU64::new(u64::MAX)), + } + } +} + impl CancellationPolicy { pub fn new() -> Self { Self::default() @@ -361,9 +373,11 @@ impl CancellationPolicy { /// /// This is intentionally a host-test and deterministic orchestration seam: /// callers can cancel a deep traversal without another thread or a timing - /// dependency. A value of zero cancels at the next checkpoint. + /// dependency. A value of zero cancels at the next checkpoint; the default + /// policy has no checkpoint threshold. pub fn cancel_after_checkpoints(&self, checkpoints: u64) { - self.cancel_after.store(checkpoints, Ordering::Release); + let threshold = checkpoints.max(1); + self.cancel_after.store(threshold, Ordering::Release); } pub(crate) fn checkpoint(&self) -> bool { @@ -371,9 +385,8 @@ impl CancellationPolicy { .checkpoints .fetch_add(1, Ordering::AcqRel) .saturating_add(1); - self.is_cancelled() - || (self.cancel_after.load(Ordering::Acquire) != 0 - && count >= self.cancel_after.load(Ordering::Acquire)) + let threshold = self.cancel_after.load(Ordering::Acquire); + self.is_cancelled() || (threshold != u64::MAX && count >= threshold) } pub fn is_cancelled(&self) -> bool { diff --git a/tools/ldraw-compat-harness/src/main.rs b/tools/ldraw-compat-harness/src/main.rs index 1aea3d6..ea35e35 100644 --- a/tools/ldraw-compat-harness/src/main.rs +++ b/tools/ldraw-compat-harness/src/main.rs @@ -877,15 +877,20 @@ fn compare_candidate_to_oracle( )); } let mut oracle_step_count = 0usize; + let mut consumed_candidate_indices = std::collections::BTreeSet::new(); for (model_index, oracle_model) in oracle.models.iter().enumerate() { if oracle_model.model_index as usize != model_index { return Err(format!("{} oracle model order/index mismatch at {model_index}", fixture.id)); } - let candidate_model = parsed + let matching_candidate_indices = parsed .result .models .iter() - .find(|candidate| { + .enumerate() + .filter_map(|(candidate_index, candidate)| { + if consumed_candidate_indices.contains(&candidate_index) { + return None; + } let candidate_name = Path::new(&candidate.path) .file_name() .and_then(|name| name.to_str()) @@ -894,13 +899,29 @@ fn compare_candidate_to_oracle( .file_stem() .and_then(|name| name.to_str()) .unwrap_or_default(); - candidate_name.eq_ignore_ascii_case(&oracle_model.name) + (candidate_name.eq_ignore_ascii_case(&oracle_model.name) || (model_index == 0 - && oracle_model.name == format!("pm77-{candidate_stem}")) + && oracle_model.name == format!("pm77-{candidate_stem}"))) + .then_some(candidate_index) }) - .ok_or_else(|| { - format!("{} missing candidate model[{}] {}", fixture.id, model_index, oracle_model.name) - })?; + .collect::>(); + let candidate_index = match matching_candidate_indices.as_slice() { + [] => { + return Err(format!( + "{} missing candidate model[{}] {}", + fixture.id, model_index, oracle_model.name + )); + } + [candidate_index] => *candidate_index, + indices => { + return Err(format!( + "{} ambiguous candidate model[{}] {} matched indices {:?}", + fixture.id, model_index, oracle_model.name, indices + )); + } + }; + consumed_candidate_indices.insert(candidate_index); + let candidate_model = &parsed.result.models[candidate_index]; let candidate_name = Path::new(&candidate_model.path) .file_name() .and_then(|name| name.to_str()) diff --git a/tools/ldraw-oracle-generator/src/main.rs b/tools/ldraw-oracle-generator/src/main.rs index a5e7369..41275d7 100644 --- a/tools/ldraw-oracle-generator/src/main.rs +++ b/tools/ldraw-oracle-generator/src/main.rs @@ -1183,12 +1183,16 @@ fn validate_u64_array(value: Option<&Value>, path: &str) -> Result<(), String> { Ok(()) } +fn finite_number(value: &Value) -> bool { + value.as_f64().is_some_and(f64::is_finite) +} + fn validate_position(value: Option<&Value>, path: &str) -> Result<(), String> { let values = value .and_then(Value::as_array) .ok_or_else(|| format!("{path} must be an array"))?; - if values.len() != 3 || values.iter().any(|value| !(value.is_number() || value.is_null())) { - return Err(format!("{path} must contain exactly three numbers/nulls")); + if values.len() != 3 || values.iter().any(|value| !finite_number(value)) { + return Err(format!("{path} must contain exactly three finite numbers")); } Ok(()) } @@ -1197,8 +1201,8 @@ fn validate_matrix(value: Option<&Value>, path: &str) -> Result<(), String> { let values = value .and_then(Value::as_array) .ok_or_else(|| format!("{path} must be an array"))?; - if values.len() != 12 || values.iter().any(|value| !(value.is_number() || value.is_null())) { - return Err(format!("{path} must contain exactly twelve numbers/nulls")); + if values.len() != 12 || values.iter().any(|value| !finite_number(value)) { + return Err(format!("{path} must contain exactly twelve finite numbers")); } Ok(()) } @@ -1580,6 +1584,41 @@ mod tests { assert!(validate_document(&value).is_err()); } + #[test] + fn successful_document_rejects_null_numeric_values_recursively() { + let paths = [ + ("models[0].mesh.positions", serde_json::json!([[null, 0, 0]])), + ("models[0].mesh.normals", serde_json::json!([[0, null, 0]])), + ("tokenized_geometry.triangle_positions", serde_json::json!([[[0, 0, null], [0, 0, 0], [0, 0, 0]]])), + ("models[0].children[0].transform.translation", serde_json::json!([0, null, 0])), + ("models[0].children[0].transform.matrix", serde_json::json!([1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, null])), + ]; + for (path, replacement) in paths { + let mut value = valid_success_document(); + match path { + "models[0].mesh.positions" => value["models"][0]["mesh"]["positions"] = replacement, + "models[0].mesh.normals" => value["models"][0]["mesh"]["normals"] = replacement, + "tokenized_geometry.triangle_positions" => value["tokenized_geometry"]["triangle_positions"] = replacement, + "models[0].children[0].transform.translation" => { + value["models"][0]["children"] = serde_json::json!([{ + "index": 0, "bfc": "off", + "transform": {"translation": replacement, "matrix": [1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0]}, + "child_name": null, "child_path": null + }]); + } + "models[0].children[0].transform.matrix" => { + value["models"][0]["children"] = serde_json::json!([{ + "index": 0, "bfc": "off", + "transform": {"translation": [0, 0, 0], "matrix": replacement}, + "child_name": null, "child_path": null + }]); + } + _ => unreachable!(), + } + assert!(validate_document(&value).is_err(), "invalid numeric path accepted: {path}"); + } + } + fn valid_success_document() -> Value { serde_json::json!({ "schema_version": ORACLE_SCHEMA, diff --git a/tools/ldraw-oracle/oracle.cpp b/tools/ldraw-oracle/oracle.cpp index c08fcc1..9d85b42 100644 --- a/tools/ldraw-oracle/oracle.cpp +++ b/tools/ldraw-oracle/oracle.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -178,27 +179,23 @@ std::optional tokenize_file(const std::filesystem::path& p return models; } -void resolve_includes(const std::string& ldrawDir, const LDParse::ModelStream& models, - const std::set& modelNames, std::set& visited, - ResolvedCounts& counts, LDParse::ErrF error_handler) { - for (const auto& [modelName, lineStream] : models) { - (void)modelName; - for (const auto& [lineText, tokens] : lineStream) { - const auto name = include_name(tokens, lineText); - if (!name) continue; - - const auto key = lowercase(*name); - if (modelNames.count(key)) continue; - if (!visited.insert(key).second) continue; - const auto path = resolve_part(ldrawDir, *name); - if (!path) continue; - const auto partModels = tokenize_file(*path, error_handler); - if (!partModels) continue; - - add_counts(counts, count_geometry(*partModels)); - resolve_includes(ldrawDir, *partModels, modelNames, visited, counts, error_handler); - } +using ModelCounts = std::map; + +void resolve_model_occurrences(const LDParse::Model& model, const ModelCounts& source_counts, + std::set& active, + ResolvedCounts& counts) { + const auto key = lowercase(model.getPath()); + if (const auto found = source_counts.find(key); found != source_counts.end()) { + add_counts(counts, found->second); + } + if (!active.insert(&model).second) return; + for (const auto& [index, bfc, transform, child] : model.children()) { + (void)index; + (void)bfc; + (void)transform; + if (child) resolve_model_occurrences(*child, source_counts, active, counts); } + active.erase(&model); } std::string certification(boost::logic::tribool value) { @@ -588,13 +585,14 @@ int run_one(const std::string& path) { return 0; } - ResolvedCounts resolved{t_tri, t_quad, t_line, t_cond, t_incl}; - if (const char* ldraw_dir = std::getenv("LDRAWDIR"); ldraw_dir && *ldraw_dir) { - std::set model_names; - for (const auto& model : models) model_names.insert(lowercase(model.first)); - std::set visited; - resolve_includes(ldraw_dir, models, model_names, visited, resolved, boundary_error); + ModelCounts source_counts; + for (const auto& [model_name, line_stream] : models) { + LDParse::ModelStream one_model{{model_name, line_stream}}; + source_counts.emplace(lowercase(model_name), count_geometry(one_model)); } + ResolvedCounts resolved; + std::set active; + resolve_model_occurrences(*root, source_counts, active, resolved); std::cout << success_json(path, names, *root, models, diagnostics, t_tri, t_quad, t_line, t_cond, t_incl, resolved, tri_positions) << '\n';