diff --git a/Cargo.lock b/Cargo.lock index bd072da..ef05ce7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6404,7 +6404,6 @@ dependencies = [ "serde_json", "sha2 0.10.9", "thiserror 2.0.18", - "typed-arena", ] [[package]] @@ -9011,12 +9010,6 @@ version = "2.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ea3136b675547379c4bd395ca6b938e5ad3c3d20fad76e7fe85f9e0d011419c" -[[package]] -name = "typed-arena" -version = "2.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6af6ae20167a9ece4bcb41af5b80f8a1f1df981f6391189ce00fd257af04126a" - [[package]] name = "typed-path" version = "0.12.3" diff --git a/crates/polymodel-ldraw-core/Cargo.toml b/crates/polymodel-ldraw-core/Cargo.toml index 1915f88..cebfb6d 100644 --- a/crates/polymodel-ldraw-core/Cargo.toml +++ b/crates/polymodel-ldraw-core/Cargo.toml @@ -11,7 +11,6 @@ serde = { workspace = true } miette = { workspace = true } sha2 = "0.10" thiserror = { workspace = true } -typed-arena = "2" [dev-dependencies] serde_json = "1" diff --git a/crates/polymodel-ldraw-core/src/adapter.rs b/crates/polymodel-ldraw-core/src/adapter.rs index 66a542c..2820db2 100644 --- a/crates/polymodel-ldraw-core/src/adapter.rs +++ b/crates/polymodel-ldraw-core/src/adapter.rs @@ -1,4 +1,6 @@ -use crate::{LdrawParser, ParseError, ParseOptions, ParseResult, ParserProfile}; +use crate::{ + LdrawParser, Materialization, ParseError, ParseOptions, ParseResult, ParserProfile, RootId, +}; use polymodel_renderer_ledger::{ReservationLedger, ReservationOwner}; use std::borrow::Cow; @@ -13,6 +15,8 @@ pub struct AdapterRequest<'a> { pub ledger: &'a ReservationLedger, pub semantic_budget: Option, pub root_name: &'a str, + pub root: RootId, + pub materializations: &'a [Materialization<'a>], } impl InProcessRustAdapter { pub fn parse(request: AdapterRequest<'_>) -> Result, ParseError> { @@ -20,7 +24,9 @@ impl InProcessRustAdapter { options.profile = request.profile; options.semantic_budget = request.semantic_budget; options.root_name = request.root_name.into(); - let mut result = LdrawParser.parse_bytes(request.bytes, options, None)?; + options.resolved_root = request.root; + let mut result = + LdrawParser.parse_bytes(request.bytes, options, Some(request.materializations))?; result.provenance_id = Cow::Owned(request.provenance_id.to_owned()); let _ = request.fixture_id; Ok(result) @@ -41,6 +47,8 @@ impl InProcessRustAdapter { ledger, semantic_budget: Some(16 * 1024 * 1024), root_name: fixture_id, + root: RootId::UploadedManifest, + materializations: &[], }) } } diff --git a/crates/polymodel-ldraw-core/src/geom.rs b/crates/polymodel-ldraw-core/src/geom.rs index 52cd9db..c6ead2f 100644 --- a/crates/polymodel-ldraw-core/src/geom.rs +++ b/crates/polymodel-ldraw-core/src/geom.rs @@ -366,16 +366,6 @@ pub(crate) fn process_geometry<'src>( LineType::Four => (LimitKind::Triangles, 2), _ => unreachable!(), }; - counters - .add(line_kind, line_delta, limits) - .map_err(|name| limit_error(name, Some(line.span)))?; - for point in points.iter().copied() { - if let Some(bounds) = model.bounds.as_mut() { - bounds.add(point); - } else { - model.bounds = Some(Bounds3::from_point(point)); - } - } let Some(colour) = parse_geometry_colour(line.tokens.get(1)) else { model.bfc.invert_next = false; return add_diag( @@ -389,11 +379,23 @@ pub(crate) fn process_geometry<'src>( true, ); }; + counters + .add(line_kind, line_delta, limits) + .map_err(|name| limit_error(name, Some(line.span)))?; + let mut next_bounds = model.bounds; + for point in points.iter().copied() { + if let Some(bounds) = next_bounds.as_mut() { + bounds.add(point); + } else { + next_bounds = Some(Bounds3::from_point(point)); + } + } model.geometry.push(GeometryRecord { line_type: typ, colour, vertices: points, }); + model.bounds = next_bounds; match typ { LineType::Two => { model.lines = model diff --git a/crates/polymodel-ldraw-core/src/lib.rs b/crates/polymodel-ldraw-core/src/lib.rs index 26c6eee..d05061b 100644 --- a/crates/polymodel-ldraw-core/src/lib.rs +++ b/crates/polymodel-ldraw-core/src/lib.rs @@ -56,6 +56,75 @@ mod tests { assert_eq!(files.len(), 1); assert_eq!(files[0].lines.len(), 1); } + + #[test] + fn unterminated_quotes_are_rejected() { + let error = scan_lines(b"0 FILE \"root.ldr\n", &LdrawLimits::default()) + .expect_err("unterminated quote must not become a token"); + assert!(matches!( + error, + LdrawError::Diagnostic(Diagnostic { + message, + .. + }) if message.contains("unterminated quoted token") + )); + } + + #[test] + fn line_limit_is_per_physical_line() { + let mut limits = LdrawLimits::default(); + limits.line_bytes = 3; + assert!(scan_lines(b"abc\ndef", &limits).is_ok()); + assert!(scan_lines(b"abcd\n", &limits).is_err()); + } + + #[test] + fn invalid_geometry_colour_does_not_publish_geometry_or_counts() { + let ledger = ReservationLedger::new(); + let mut parse_options = options(&ledger); + parse_options.profile = ParserProfile::Compatibility; + let result = LdrawParser + .parse_bytes( + b"3 invalid 0 0 0 1 0 0 0 1 0\n3 16 0 0 0 1 0 0 0 1 0\n", + parse_options, + None, + ) + .expect("compatibility profile should continue after invalid colour"); + assert_eq!(result.models[0].geometry.len(), 1); + assert_eq!(result.models[0].triangles, 1); + assert_eq!(result.models[0].quads, 0); + } + #[test] + fn caller_materialization_controls_external_model_identity() { + let bytes = b"0 FILE root.ldr\n1 16 0 0 0 1 0 0 0 1 0 0 0 0 child.dat\n0 NOFILE\n"; + let child = b"3 16 0 0 0 1 0 0 0 1 0\n"; + let child_path = NormalizedPath::new("child.dat").unwrap(); + let materialization = Materialization { + path: child_path.clone(), + root: RootId::OfficialLibrary, + content_hash: CacheKey::new(child_path.clone(), RootId::OfficialLibrary, child) + .content_hash, + bytes: child, + available: true, + }; + let ledger = ReservationLedger::new(); + let result = InProcessRustAdapter::parse(AdapterRequest { + fixture_id: "materialized", + bytes, + profile: ParserProfile::Compatibility, + provenance_id: "caller-provenance", + owner: ReservationOwner::preview(1, 1, 1), + ledger: &ledger, + semantic_budget: Some(16 * 1024 * 1024), + root_name: "root.ldr", + root: RootId::UploadedManifest, + materializations: std::slice::from_ref(&materialization), + }) + .unwrap(); + assert!(result.models.iter().any(|model| model.path == "child.dat")); + assert_eq!(result.provenance_id, "caller-provenance"); + } + #[test] fn reservation_lives_until_result_drop() { let ledger = ReservationLedger::new(); @@ -75,11 +144,7 @@ mod tests { fn conditional_geometry_exposes_endpoints_and_controls() { let ledger = ReservationLedger::new(); let result = LdrawParser - .parse_bytes( - b"5 16 0 0 0 1 0 0 0 1 0 2 0 0\n", - options(&ledger), - None, - ) + .parse_bytes(b"5 16 0 0 0 1 0 0 0 1 0 2 0 0\n", options(&ledger), None) .unwrap(); let geometry = &result.models[0].geometry[0]; let (endpoints, controls) = geometry @@ -89,13 +154,15 @@ mod tests { assert_eq!(endpoints[1], Point3::new(1.0, 0.0, 0.0)); assert_eq!(controls[0], Point3::new(0.0, 1.0, 0.0)); assert_eq!(controls[1], Point3::new(2.0, 0.0, 0.0)); - assert!(GeometryRecord { - line_type: LineType::Two, - colour: ColourCode::from(16_u16), - vertices: Vec::new(), - } - .conditional_endpoints() - .is_none()); + assert!( + GeometryRecord { + line_type: LineType::Two, + colour: ColourCode::from(16_u16), + vertices: Vec::new(), + } + .conditional_endpoints() + .is_none() + ); } #[test] fn bfc_state_machine_certify_then_nocertify() { @@ -201,9 +268,9 @@ mod tests { .iter() .map(|diagnostic| diagnostic.code.to_string()) .collect::>(); - let error = result - .err() - .unwrap_or_else(|| panic!("{} rejected fixture unexpectedly published", fixture.id)); + let error = result.err().unwrap_or_else(|| { + panic!("{} rejected fixture unexpectedly published", fixture.id) + }); let actual_code = match error { LdrawError::Diagnostic(diagnostic) => diagnostic.code, other => panic!("{} rejected with unexpected error: {other}", fixture.id), @@ -213,7 +280,7 @@ mod tests { "{} rejected with {actual_code}, expected one of {expected_codes:?}", fixture.id ); - }, + } polymodel_ldraw_testkit::ExpectedOutcome::Accepted | polymodel_ldraw_testkit::ExpectedOutcome::Preserved | polymodel_ldraw_testkit::ExpectedOutcome::Limited @@ -227,10 +294,21 @@ mod tests { ); let expected = &fixture.expected.canonical; let projected = parsed.project(); - assert_eq!(projected.schema_version, polymodel_ldraw_testkit::SCHEMA_VERSION); - assert_eq!(projected.provenance_id, expected.provenance_id, "{} provenance", fixture.id); - assert!(!projected.syntax.is_empty() || projected.schema_version == polymodel_ldraw_testkit::SCHEMA_VERSION, - "{} accepted fixture should produce syntax records", fixture.id); + assert_eq!( + projected.schema_version, + polymodel_ldraw_testkit::SCHEMA_VERSION + ); + assert_eq!( + projected.provenance_id, expected.provenance_id, + "{} provenance", + fixture.id + ); + assert!( + !projected.syntax.is_empty() + || projected.schema_version == polymodel_ldraw_testkit::SCHEMA_VERSION, + "{} accepted fixture should produce syntax records", + fixture.id + ); if fixture.expected.outcome == polymodel_ldraw_testkit::ExpectedOutcome::Cancelled { @@ -410,6 +488,35 @@ mod tests { assert_sticker_geometry(&result); } + #[test] + fn literal_stop_is_not_rewritten_to_end() { + let ledger = ReservationLedger::new(); + let mut parse_options = options(&ledger); + parse_options.profile = ParserProfile::Compatibility; + let result = LdrawParser + .parse_bytes( + b"0 !TEXMAP START PLANAR 0 0 0 1 0 0 0 1 0 sticker.png\n0 !TEXMAP STOP\n", + parse_options, + None, + ) + .expect("compatibility profile should preserve unknown STOP"); + assert_eq!( + result + .semantic + .texmap_events + .iter() + .map(|event| event.kind) + .collect::>(), + vec![TexmapKind::Start] + ); + assert!( + result + .diagnostics + .iter() + .any(|diagnostic| diagnostic.code == DiagnosticCode::TexmapUnknownMetaStop) + ); + } + #[test] fn corpus_texmap_emits_ordered_events() { let fixture = corpus_fixture("texmap-events"); diff --git a/crates/polymodel-ldraw-core/src/parser.rs b/crates/polymodel-ldraw-core/src/parser.rs index 28df337..d05d862 100644 --- a/crates/polymodel-ldraw-core/src/parser.rs +++ b/crates/polymodel-ldraw-core/src/parser.rs @@ -11,12 +11,11 @@ use crate::texmap::{TexmapState, TextureDescriptor, texmap}; use crate::traversal::traverse; use crate::types::{ DEFAULT_COLOUR, DEFAULT_PROVENANCE, Diagnostic, DiagnosticCode, LdrawLimits, LimitCounters, - LimitKind, ParseError, ParseOptions, ParserProfile, Resolver, SemanticArenas, Span, add_diag, - limit_error, + LimitKind, Materialization, ParseError, ParseOptions, ParserProfile, SemanticArenas, Span, + add_diag, limit_error, }; use std::borrow::Cow; use std::collections::BTreeSet; -use typed_arena::Arena; pub struct LdrawParser; impl Default for LdrawParser { @@ -29,10 +28,9 @@ impl LdrawParser { &self, bytes: &'src [u8], options: ParseOptions<'a>, - resolver: Option<&dyn Resolver>, + materializations: Option<&[Materialization<'src>]>, ) -> Result, ParseError> { let mut counters = LimitCounters::default(); - let arena = Arena::>::new(); let input_len = u64::try_from(bytes.len()).map_err(|_| ParseError::Overflow("resource bytes"))?; counters @@ -53,9 +51,9 @@ impl LdrawParser { } let line_bytes = u64::try_from(line.raw.len()).map_err(|_| ParseError::Overflow("line bytes"))?; - counters - .add(LimitKind::LineBytes, line_bytes, &options.limits) - .map_err(|name| limit_error(name, Some(line.span)))?; + if line_bytes > options.limits.line_bytes { + return Err(limit_error(LimitKind::LineBytes, Some(line.span))); + } } let mut files = split_mpd(&scanned, &options.limits)?; let has_mpd_file = scanned.iter().any(|line| { @@ -106,84 +104,62 @@ impl LdrawParser { )?); } - if let Some(resolver) = resolver { - let mut visited_names = models - .iter() - .map(|model| model.path.as_str().to_owned()) - .collect::>(); - let mut model_index = 0; - while model_index < models.len() { - let include_names = models[model_index] - .includes - .iter() - .map(|include| include.name.to_string()) - .collect::>(); - for include_name in include_names { - let normalized_path = match NormalizedPath::new(&include_name) { - Ok(path) => path, - Err(_) => continue, - }; - let normalized_name = normalized_path.to_string(); - if models.iter().any(|model| model.path == normalized_path) - || !visited_names.insert(normalized_name.clone()) - { - continue; - } - counters - .add(LimitKind::Fetches, 1, &options.limits) - .map_err(|name| limit_error(name, None))?; - let Some(bytes) = resolver.resolve(&normalized_name) else { - continue; - }; - let part_bytes = arena.alloc(bytes.into_owned()).as_slice(); - counters - .add( - LimitKind::ResourceBytes, - u64::try_from(part_bytes.len()) - .map_err(|_| ParseError::Overflow("resource bytes"))?, - &options.limits, - ) - .map_err(|name| limit_error(name, None))?; - let resolved_scanned = scan_lines(part_bytes, &options.limits)?; - for line in &resolved_scanned { - counters - .add( - LimitKind::LineBytes, - u64::try_from(line.raw.len()) - .map_err(|_| ParseError::Overflow("line bytes"))?, - &options.limits, - ) - .map_err(|name| limit_error(name, Some(line.span)))?; - } - let file = VirtualFile { - id: u32::try_from(files.len()) - .map_err(|_| ParseError::Overflow("virtual file id"))?, - name: Cow::Owned(normalized_name), - lines: resolved_scanned, - source_span: Span { - start: 0, - end: u32::try_from(part_bytes.len()) - .map_err(|_| ParseError::Overflow("source span"))?, - line: 1, - column: 1, - }, - }; - counters - .add(LimitKind::Files, 1, &options.limits) - .map_err(|name| limit_error(name, None))?; - files.push(file.clone()); - models.push(parse_model( - &file, - &options, - &mut counters, - &mut diagnostics, - &mut arenas, - &mut colours, - &mut texture_ids, - )?); - } - model_index += 1; + let mut materialized_paths = BTreeSet::new(); + for materialization in materializations.into_iter().flatten() { + if !materialization.available { + continue; } + if materialization.bytes.is_empty() { + continue; + } + let normalized_path = materialization.path.clone(); + let normalized_name = normalized_path.to_string(); + if models.iter().any(|model| model.path == normalized_path) + || !materialized_paths.insert(materialization.path.clone()) + { + continue; + } + counters + .add( + LimitKind::ResourceBytes, + u64::try_from(materialization.bytes.len()) + .map_err(|_| ParseError::Overflow("resource bytes"))?, + &options.limits, + ) + .map_err(|name| limit_error(name, None))?; + let resolved_scanned = scan_lines(materialization.bytes, &options.limits)?; + let file = VirtualFile { + id: u32::try_from(files.len()) + .map_err(|_| ParseError::Overflow("virtual file id"))?, + name: Cow::Owned(normalized_name), + lines: resolved_scanned, + source_span: Span { + start: 0, + end: u32::try_from(materialization.bytes.len()) + .map_err(|_| ParseError::Overflow("source span"))?, + line: 1, + column: 1, + }, + }; + counters + .add(LimitKind::Files, 1, &options.limits) + .map_err(|name| limit_error(name, None))?; + files.push(file.clone()); + let mut model = parse_model( + &file, + &options, + &mut counters, + &mut diagnostics, + &mut arenas, + &mut colours, + &mut texture_ids, + )?; + model.key = CacheKey { + canonical_path: materialization.path.clone(), + resolved_root: materialization.root, + content_hash: materialization.content_hash.clone(), + }; + models.push(model); } let (scene, summaries) = traverse(&models, &options, &mut counters, &mut diagnostics)?; let root = models.first(); @@ -244,10 +220,12 @@ fn parse_model<'src>( let key = CacheKey::new_from_parts( path.clone(), options.resolved_root, - file.lines.iter().flat_map(|line| [line.raw, line.ending.as_str().as_bytes()]), + file.lines + .iter() + .flat_map(|line| [line.raw, line.ending.as_str().as_bytes()]), ); - let line_count = u64::try_from(file.lines.len()) - .map_err(|_| ParseError::Overflow("model line count"))?; + let line_count = + u64::try_from(file.lines.len()).map_err(|_| ParseError::Overflow("model line count"))?; let semantic_charge = line_count .checked_mul(256) .and_then(|charge| charge.checked_add(512)) diff --git a/crates/polymodel-ldraw-core/src/scanner.rs b/crates/polymodel-ldraw-core/src/scanner.rs index 44d6b8e..6a50fc2 100644 --- a/crates/polymodel-ldraw-core/src/scanner.rs +++ b/crates/polymodel-ldraw-core/src/scanner.rs @@ -282,10 +282,29 @@ fn scan_one<'src>( while i < bytes.len() && bytes[i] != b'"' { i += 1; } - let value = &text[inner..i]; - if i < bytes.len() { - i += 1; + if i >= bytes.len() { + return Err(ParseError::Diagnostic(Diagnostic { + code: DiagnosticCode::InvalidLineType, + severity: Severity::Error, + message: "unterminated quoted token".into(), + span: Some(Span { + start, + end: start + .checked_add( + u32::try_from(bytes.len()) + .map_err(|_| ParseError::Overflow("quoted token span end"))?, + ) + .ok_or(ParseError::Overflow("quoted token span end"))?, + line, + column: u32::try_from(begin) + .map_err(|_| ParseError::Overflow("quoted token column"))? + .checked_add(1) + .ok_or(ParseError::Overflow("quoted token column"))?, + }), + })); } + let value = &text[inner..i]; + i += 1; let begin_u32 = u32::try_from(begin).map_err(|_| ParseError::Overflow("token span start"))?; let end_u32 = u32::try_from(i).map_err(|_| ParseError::Overflow("token span end"))?; diff --git a/crates/polymodel-ldraw-core/src/texmap.rs b/crates/polymodel-ldraw-core/src/texmap.rs index eeb4a62..3d7e7bb 100644 --- a/crates/polymodel-ldraw-core/src/texmap.rs +++ b/crates/polymodel-ldraw-core/src/texmap.rs @@ -114,10 +114,7 @@ pub(crate) fn texmap<'src>( true, ); } - let kind = match line.tokens[2].kind { - TokenKind::Stop if model.texmap_state != TexmapState::Inactive => TokenKind::End, - kind => kind, - }; + let kind = line.tokens[2].kind; match kind { TokenKind::Next if line.tokens.len() == 3 => { if model.texmap_state != TexmapState::Active { diff --git a/crates/polymodel-ldraw-core/src/traversal.rs b/crates/polymodel-ldraw-core/src/traversal.rs index b98f025..6fba35b 100644 --- a/crates/polymodel-ldraw-core/src/traversal.rs +++ b/crates/polymodel-ldraw-core/src/traversal.rs @@ -156,7 +156,8 @@ pub(crate) fn traverse( counters .add(LimitKind::Instances, 1, &options.limits) .map_err(|name| limit_error(name, Some(include.span)))?; - let normalized_include = crate::cache::NormalizedPath::new(include.name.as_ref()); + let normalized_include = + crate::cache::NormalizedPath::new(include.name.as_ref()); let target = normalized_include .as_ref() .ok() @@ -168,12 +169,8 @@ pub(crate) fn traverse( .map(|path| path.to_string()) .unwrap_or_else(|_| include.name.to_string()) }); - let instance_id = format!( - "{}→{}#{}", - model.path, - child_name, - include_position + 1 - ); + let instance_id = + format!("{}→{}#{}", model.path, child_name, include_position + 1); let composed_transform = transform .compose(&include.transform) .ok_or(ParseError::Overflow("include transform arithmetic"))?; diff --git a/crates/polymodel-ldraw-core/src/types.rs b/crates/polymodel-ldraw-core/src/types.rs index 1e6f260..a86a539 100644 --- a/crates/polymodel-ldraw-core/src/types.rs +++ b/crates/polymodel-ldraw-core/src/types.rs @@ -1,5 +1,6 @@ -use miette::SourceSpan; +use crate::cache::NormalizedPath; use crate::geom::ColourCode; +use miette::SourceSpan; use polymodel_renderer_ledger::{ AdmissionError, RejectionReason, Reservation, ReservationLedger, ReservationOwner, ResourceClass, @@ -339,8 +340,15 @@ pub struct ParseOptions<'a> { pub cancellation: CancellationPolicy, } -pub trait Resolver { - fn resolve(&self, name: &str) -> Option>; +/// A caller-materialized target. The core never discovers or loads targets: a +/// native or network adapter supplies this table before parsing. +#[derive(Clone, Debug)] +pub struct Materialization<'a> { + pub path: NormalizedPath, + pub root: RootId, + pub content_hash: crate::cache::Sha256Hash, + pub bytes: &'a [u8], + pub available: bool, } /// Caller-owned storage for source buffers used during a parse. @@ -353,7 +361,9 @@ pub struct ByteBank { impl ByteBank { pub fn new() -> Self { - Self { buffers: Vec::new() } + Self { + buffers: Vec::new(), + } } pub fn add(&mut self, bytes: Vec) -> &[u8] { @@ -363,7 +373,6 @@ impl ByteBank { .map(Vec::as_slice) .expect("buffer was just added") } - } impl Default for ByteBank { @@ -371,33 +380,6 @@ impl Default for ByteBank { Self::new() } } - -pub struct LocalResolver { - base_dir: std::path::PathBuf, -} - -impl LocalResolver { - pub fn new>(base_dir: P) -> Self { - Self { base_dir: base_dir.into() } - } - - fn try_path(&self, name: &str, subdir: &str) -> Option> { - let path = self.base_dir.join(subdir).join(name); - match std::fs::read(&path) { - Ok(bytes) => Some(std::borrow::Cow::Owned(bytes)), - Err(_) => None, - } - } -} - -impl Resolver for LocalResolver { - fn resolve(&self, name: &str) -> Option> { - self.try_path(name, "parts") - .or_else(|| self.try_path(name, "p")) - .or_else(|| self.try_path(name, "parts/s")) - .or_else(|| self.try_path(name, "models")) - } -} impl<'a> ParseOptions<'a> { pub fn new(owner: ReservationOwner, ledger: &'a ReservationLedger) -> Self { Self {