diff --git a/crates/ocs_doc_api/src/backend.rs b/crates/ocs_doc_api/src/backend.rs index 0e004d6e3..5f5d5962a 100644 --- a/crates/ocs_doc_api/src/backend.rs +++ b/crates/ocs_doc_api/src/backend.rs @@ -19,9 +19,22 @@ pub type KernelBody = cadkernel::brep::Body; pub trait DocApiBackend { // ── identity / solid lookup ──────────────────────────────────────────── /// Resolve a live B-rep body for `id` (solid cache; re-lift from AcisData on - /// miss). `UnknownId` if `id` is not a live solid. + /// miss). `UnknownId` if `id` is not a live solid. NOTE: returns an OWNED body + /// (a clone of the cache entry); use `with_body` for read-only queries. fn resolve_body(&mut self, id: ObjectId) -> ApiResult; + /// Borrow the cached body read-only for a query, avoiding the `resolve_body` + /// deep clone. Default: falls back to `resolve_body` (clone). The host overrides + /// this to borrow the `solid_models` cache in place (O(1), no copy). + fn with_body( + &mut self, + id: ObjectId, + f: &mut dyn FnMut(&KernelBody) -> ApiResult, + ) -> ApiResult { + let body = self.resolve_body(id)?; + f(&body) + } + /// Store a new solid body as a `Solid3D` entity; returns the fresh `ObjectId`. /// Mirrors host `add_solid_model` (edge wires + `solid_to_sat` + /// `set_sat_document` + `register_solid_model`). diff --git a/crates/ocs_doc_api/src/executor.rs b/crates/ocs_doc_api/src/executor.rs index e6c626e42..a6e198e28 100644 --- a/crates/ocs_doc_api/src/executor.rs +++ b/crates/ocs_doc_api/src/executor.rs @@ -103,14 +103,19 @@ pub fn apply_op(b: &mut B, op: Operation) -> ApiResult { Prepared::Solid(crate::geom::make_solid(p).map_err(|e| at_index(name, i, e))?) } - EntitySpec::Curve(c) => Prepared::Curve(c.clone()), + EntitySpec::Curve(c) => { + validate_curve_spec(c).map_err(|e| at_index(name, i, e))?; + Prepared::Curve(c.clone()) + } }; prepared.push(prep); } @@ -254,6 +259,11 @@ pub fn apply_op(b: &mut B, op: Operation) -> ApiResult { + // Bound the section count like the bulk ops (one envelope can carry many + // ids; an uncapped loft would be an unbounded kernel call on the UI thread). + if profiles.len() > BULK_ITEM_CAP { + return Err(over_cap(name, profiles.len())); + } // Resolve every profile to its curve set BEFORE push_undo (all-or-nothing). if profiles.len() < 2 { return Err(ApiError::validation(name, "loft needs >= 2 profiles")); @@ -336,24 +346,25 @@ pub fn apply_queries(b: &mut B, queries: Vec) -> ApiRes } let mut results = Vec::with_capacity(queries.len()); for q in &queries { + let qname = q.query_name(); let r = match q { - Query::GetEntity { id } => QueryResult::Entity(b.get_entity(*id)?), - Query::GetBounds { id } => QueryResult::Bounds(b.bounds(*id)?), - Query::GetCentroid { id } => QueryResult::Centroid(b.centroid(*id)?), - Query::GetVolume { id } => QueryResult::Volume(b.volume(*id)?), + Query::GetEntity { id } => QueryResult::Entity(b.get_entity(*id).map_err(|e| label(qname, e))?), + Query::GetBounds { id } => QueryResult::Bounds(b.bounds(*id).map_err(|e| label(qname, e))?), + Query::GetCentroid { id } => QueryResult::Centroid(b.centroid(*id).map_err(|e| label(qname, e))?), + Query::GetVolume { id } => QueryResult::Volume(b.volume(*id).map_err(|e| label(qname, e))?), Query::GetIntersects { a, b: bid } => { - let ba = b.bounds(*a)?; - let bb = b.bounds(*bid)?; + let ba = b.bounds(*a).map_err(|e| label(qname, e))?; + let bb = b.bounds(*bid).map_err(|e| label(qname, e))?; QueryResult::Intersects(ba.overlaps(&bb)) } Query::GetGeometryRevision => QueryResult::Revision(b.revision()), - Query::GetTextContent { id } => QueryResult::TextContent(b.text_content(*id)?), - Query::GetHatchBoundary { id } => QueryResult::HatchBoundary(b.hatch_boundary(*id)?), - Query::GetDimensionMeasurement { id } => QueryResult::DimensionMeasurement(b.dimension_measurement(*id)?), - Query::GetAttributes { id } => QueryResult::Attributes(b.attributes(*id)?), - Query::GetBlockEntities { block_name } => QueryResult::BlockEntities(b.block_entities(block_name)?), + Query::GetTextContent { id } => QueryResult::TextContent(b.text_content(*id).map_err(|e| label(qname, e))?), + Query::GetHatchBoundary { id } => QueryResult::HatchBoundary(b.hatch_boundary(*id).map_err(|e| label(qname, e))?), + Query::GetDimensionMeasurement { id } => QueryResult::DimensionMeasurement(b.dimension_measurement(*id).map_err(|e| label(qname, e))?), + Query::GetAttributes { id } => QueryResult::Attributes(b.attributes(*id).map_err(|e| label(qname, e))?), + Query::GetBlockEntities { block_name } => QueryResult::BlockEntities(b.block_entities(block_name).map_err(|e| label(qname, e))?), Query::GetViewportView { id } => { - let (target, height) = b.viewport_view(*id)?; + let (target, height) = b.viewport_view(*id).map_err(|e| label(qname, e))?; QueryResult::ViewportView { target, height } } }; @@ -366,6 +377,15 @@ pub fn apply_queries(b: &mut B, queries: Vec) -> ApiRes }) } +/// Attach the query name to an error for better diagnostics (uses `query_name`). +fn label(query: &'static str, e: ApiError) -> ApiError { + match e { + ApiError::Validation { reason, .. } => ApiError::validation(query, reason), + ApiError::UnknownId(id) => ApiError::validation(query, format!("unknown ObjectId {id:?}")), + other => other, + } +} + fn profile_curves( b: &B, profile: ObjectId, @@ -377,6 +397,21 @@ fn profile_curves( b.profile_curves(profile) } +/// Validate a `Curve2Spec`'s fallible conditions BEFORE mutation (the same rules +/// the host's `curve_spec_to_entity` enforces), so `CreateMany` stays all-or-nothing. +/// Currently: ellipse minor/major ratio must be in (0, 1]. +fn validate_curve_spec(spec: &crate::ops::Curve2Spec) -> ApiResult<()> { + if let crate::ops::Curve2Spec::Ellipse { ratio, .. } = spec { + if !(*ratio > 0.0 && *ratio <= 1.0) { + return Err(ApiError::validation( + "CreateMany", + format!("ellipse minor_axis_ratio must be in (0, 1], got {ratio}"), + )); + } + } + Ok(()) +} + fn require_exists(b: &B, id: ObjectId, op: &'static str) -> ApiResult<()> { if b.entity_exists(id) { Ok(()) diff --git a/crates/ocs_doc_api/src/id.rs b/crates/ocs_doc_api/src/id.rs index 447761fae..c19e6fc33 100644 --- a/crates/ocs_doc_api/src/id.rs +++ b/crates/ocs_doc_api/src/id.rs @@ -17,10 +17,6 @@ impl ObjectId { self.0 } - pub const fn is_null(self) -> bool { - self.0 == 0 - } - /// Build from a raw u64 (e.g. an `acadrust::Handle`'s value). pub const fn from_u64(v: u64) -> Self { Self(v) diff --git a/crates/ocs_doc_api/src/transport/inproc.rs b/crates/ocs_doc_api/src/transport/inproc.rs index 5cdca1310..e8328e0a7 100644 --- a/crates/ocs_doc_api/src/transport/inproc.rs +++ b/crates/ocs_doc_api/src/transport/inproc.rs @@ -19,10 +19,6 @@ impl InProcess { pub fn new(backend: B) -> Self { Self { backend: Mutex::new(backend) } } - /// Borrow the backend mutably (test/introspection helper). - pub fn backend_mut(&mut self) -> &mut B { - self.backend.get_mut().expect("in-process backend poisoned") - } /// Borrow the backend immutably (test/introspection helper). pub fn backend(&self) -> std::sync::MutexGuard<'_, B> { self.backend.lock().expect("in-process backend poisoned") diff --git a/crates/ocs_doc_api/tests/mock_backend.rs b/crates/ocs_doc_api/tests/mock_backend.rs index 82909cd86..43b7c6f14 100644 --- a/crates/ocs_doc_api/tests/mock_backend.rs +++ b/crates/ocs_doc_api/tests/mock_backend.rs @@ -12,6 +12,38 @@ use ocs_doc_api::{ HasId, ObjectId, PlacementSpec, }; +/// Coarse bounds for a stored 2D curve spec (mirrors the host's entity_bounds and +/// the example mock — all three must answer curve bounds identically). +fn curve_bounds(spec: &Curve2Spec) -> Aabb { + match spec { + Curve2Spec::Line { start, end } => Aabb { + min: [start[0].min(end[0]), start[1].min(end[1]), start[2].min(end[2])], + max: [start[0].max(end[0]), start[1].max(end[1]), start[2].max(end[2])], + }, + Curve2Spec::Circle { centre, radius } | Curve2Spec::Arc { centre, radius, .. } => Aabb { + min: [centre[0] - radius, centre[1] - radius, centre[2]], + max: [centre[0] + radius, centre[1] + radius, centre[2]], + }, + Curve2Spec::Point { position } => Aabb { min: *position, max: *position }, + Curve2Spec::Polyline { points, .. } | Curve2Spec::Spline { control_points: points, .. } => { + let mut min = [f64::INFINITY; 3]; + let mut max = [f64::NEG_INFINITY; 3]; + for p in points { + for i in 0..3 { + min[i] = min[i].min(p[i]); + max[i] = max[i].max(p[i]); + } + } + Aabb { min, max } + } + Curve2Spec::Ellipse { centre, major_axis, .. } => { + let r = (major_axis[0] * major_axis[0] + major_axis[1] * major_axis[1] + major_axis[2] * major_axis[2]).sqrt(); + Aabb { min: [centre[0] - r, centre[1] - r, centre[2] - r], max: [centre[0] + r, centre[1] + r, centre[2] + r] } + } + Curve2Spec::Ray { origin, .. } | Curve2Spec::XLine { origin, .. } => Aabb { min: *origin, max: *origin }, + } +} + /// In-memory mock backend: HashMap + a tiny entity table. /// Tracks undo steps + revision bumps to assert per-op semantics. #[derive(Default)] @@ -29,6 +61,9 @@ struct MockBackend { dimension_measurements: HashMap, /// Stored insert attributes. attributes_store: HashMap>, + /// Stored 2D curve specs so `bounds()` works on curve entities (aligned with + /// the example mock — both mocks must answer curve bounds the same way). + curves: HashMap, /// Stored viewport views (target, height). viewport_views: HashMap, } @@ -72,7 +107,9 @@ impl DocApiBackend for MockBackend { Curve2Spec::Ray { .. } => "Ray", Curve2Spec::XLine { .. } => "XLine", }; - Ok(self.alloc(kind)) + let id = self.alloc(kind); + self.curves.insert(id, spec.clone()); + Ok(id) } fn add_insert(&mut self, spec: &ocs_doc_api::ops::InsertSpec) -> ApiResult { if spec.block_name.is_empty() { @@ -237,6 +274,11 @@ impl DocApiBackend for MockBackend { Err(ApiError::Unsupported("profiles not mocked".into())) } fn bounds(&mut self, id: ObjectId) -> ApiResult { + // Solids via the kernel body; 2D curves from their stored spec (aligned + // with the example mock — both mocks answer curve bounds identically). + if let Some(spec) = self.curves.get(&id) { + return Ok(curve_bounds(spec)); + } let body = self.body(id)?; let bb = cadkernel::brep::body_bounds(body) .ok_or_else(|| ApiError::geometry(GeometryErrorKind::Empty, "no bounds"))?; diff --git a/crates/ocs_plugin_api/src/manifest.rs b/crates/ocs_plugin_api/src/manifest.rs index c0113e126..b72648a72 100644 --- a/crates/ocs_plugin_api/src/manifest.rs +++ b/crates/ocs_plugin_api/src/manifest.rs @@ -19,7 +19,14 @@ /// vtable so API v2 plugins keep working. v4 adds full-duplex notifications on /// a multiplexed socket while leaving the V2/V3 ABI and protocol untouched. v5 /// adds the `BuiltinPlugin::on_load` lifecycle callback. -pub const API_VERSION: u32 = 5; +/// Host ABI major version. Bumped to 6 when the DocApi v2 surface +/// (`PluginRequest::DocApiRequest` / `PluginResponse::DocApiResponse` + +/// `HostApi::doc_api_dispatch`) was added: plugins that call DocApi require an +/// API-6 host and are rejected at handshake by older (API ≤ 5) hosts, instead of +/// failing at runtime with a bincode-decode transport error. The DocApi wire +/// changes are append-only and `doc_api_dispatch` is a defaulted method, so +/// older V2–V5 plugins still load unchanged on an API-6 host. +pub const API_VERSION: u32 = 6; /// Oldest plugin API major the current host still loads. This keeps previously /// compiled cdylibs usable as long as their vtable layout is a prefix of the diff --git a/src/app/doc_api.rs b/src/app/doc_api.rs index b34d75f6e..028cb9df1 100644 --- a/src/app/doc_api.rs +++ b/src/app/doc_api.rs @@ -19,8 +19,17 @@ use super::plugin_host::HostSession; /// Entry point called by the `HostApi::doc_api_dispatch` override (below) and by // the in-process path. Deserializes the envelope, runs the crate executor, // serializes the `Receipt` (or `ApiError`) back to bytes. -pub fn execute_doc_api(host: &mut HostSession<'_>, _tab_id: u64, bytes: &[u8]) -> Result, String> { +pub fn execute_doc_api(host: &mut HostSession<'_>, tab_id: u64, bytes: &[u8]) -> Result, String> { use ocs_doc_api::{DocApiEnvelope, EnvelopeBody}; + // Authorization: the request must name the SAME tab the HostSession is bound to. + // The HostSession is built per-tab by the dispatch pump; a plugin naming a + // different tab_id would otherwise reach a tab it isn't bound to (confused deputy). + if tab_id != host.tab_id() { + return Err(format!( + "DocApi tab mismatch: request names tab {tab_id} but the bound tab is {}", + host.tab_id() + )); + } let envelope: DocApiEnvelope = bincode::deserialize(bytes) .map_err(|e| format!("DocApiEnvelope deserialize: {e}"))?; let result: ApiResult = match envelope.body { @@ -30,11 +39,13 @@ pub fn execute_doc_api(host: &mut HostSession<'_>, _tab_id: u64, bytes: &[u8]) - bincode::serialize(&result).map_err(|e| format!("Receipt serialize: {e}")) } +// ObjectId <-> acadrust::Handle converters delegate to the crate's helpers +// (single source of truth; no local duplicates). fn obj_to_handle(id: ObjectId) -> Handle { - Handle::new(id.as_u64()) + id.to_handle() } fn handle_to_obj(h: Handle) -> ObjectId { - ObjectId::from_u64(h.value()) + ObjectId::from_handle(h) } impl DocApiBackend for HostSession<'_> { @@ -49,6 +60,18 @@ impl DocApiBackend for HostSession<'_> { .ok_or(ApiError::UnknownId(id)) } + fn with_body( + &mut self, + id: ObjectId, + f: &mut dyn FnMut(&KernelBody) -> ApiResult, + ) -> ApiResult { + let handle = obj_to_handle(id); + // Lift-on-miss, then borrow the cache entry in place — O(1), no deep clone. + self.scene_mut().restore_solid_models(&[handle]); + let body = self.scene().solid_models.get(&handle).ok_or(ApiError::UnknownId(id))?; + f(body) + } + fn store_solid(&mut self, body: &KernelBody) -> ApiResult { // Pre-flight the display prep BEFORE committing the entity: if the mesh // cannot be produced, fail now — no entity, no rollback, no revision bump @@ -409,6 +432,10 @@ impl DocApiBackend for HostSession<'_> { fn add_raster_image(&mut self, spec: &ocs_doc_api::ops::RasterImageSpec) -> ApiResult { use acadrust::entities::RasterImage; use acadrust::types::{Vector2, Vector3}; + // Validate the plugin-supplied path before embedding it in the document: + // non-empty, a known image extension, and not a UNC/network path or + // parent-traversal (which could point the host at unintended resources). + validate_image_path(&spec.file_path)?; let mut img = RasterImage::default(); img.file_path = spec.file_path.clone(); img.insertion_point = Vector3::new(spec.insertion_point[0], spec.insertion_point[1], spec.insertion_point[2]); @@ -574,15 +601,15 @@ impl DocApiBackend for HostSession<'_> { fn bounds(&mut self, id: ObjectId) -> ApiResult { let handle = obj_to_handle(id); - // Lift-on-miss for solids (consistent with volume/centroid): resolve_body - // repopulates a cold solid_models cache from SAT. A solid whose cache entry - // was dropped (e.g. by a host-side update_entity) no longer misreports - // `Unsupported` (plan review). + // Lift-on-miss for solids (consistent with volume/centroid); borrow the cache + // read-only via with_body (O(1), no B-rep deep clone). if matches!(self.document().get_entity(handle), Some(EntityType::Solid3D(_))) { - let body = self.resolve_body(id)?; - let bb = cadkernel::brep::body_bounds(&body) - .ok_or_else(|| ApiError::geometry(GeometryErrorKind::Empty, "no bounds"))?; - return Ok(Aabb { min: bb.min, max: bb.max }); + let mut f = |body: &KernelBody| -> ApiResult { + let bb = cadkernel::brep::body_bounds(body) + .ok_or_else(|| ApiError::geometry(GeometryErrorKind::Empty, "no bounds"))?; + Ok(Aabb { min: bb.min, max: bb.max }) + }; + return self.with_body(id, &mut f); } crate::app::doc_api_convert::entity_bounds(self.document().get_entity(handle), id) } @@ -626,7 +653,7 @@ impl HostSession<'_> { fn mass_properties(&mut self, id: ObjectId) -> ApiResult<(f64, [f64; 3])> { let handle = obj_to_handle(id); let epoch = self.scene().geometry_epoch; - if let Some(&(cached_epoch, v, c)) = self.mass_cache.get(&handle) { + if let Some(&(cached_epoch, v, c)) = self.scene().doc_api_mass_cache.get(&handle) { if cached_epoch == epoch { return Ok((v, c)); } @@ -635,8 +662,10 @@ impl HostSession<'_> { // converge to near-analytic (the render-mesh metrics cache is the coarse // display LOD, NOT for query accuracy). Bounded by the cold-tess budget; // memoized per (handle, epoch) so repeated queries on the same solid are O(1). + // The budget + cache live on the per-tab Scene so they persist across + // DocApi dispatches (not reset per request). const COLD_TESS_BUDGET: usize = 256; - if self.cold_tess_used >= COLD_TESS_BUDGET { + if self.scene().doc_api_cold_tess_used >= COLD_TESS_BUDGET { // Budget exhausted: serve a documented-approximate value from the render // mesh metrics cache rather than failing (large distinct-solid batches // degrade gracefully instead of erroring). Accuracy-first up to the budget. @@ -646,18 +675,47 @@ impl HostSession<'_> { } } return Err(ApiError::Unsupported( - "volume/centroid cold-tessellation budget exceeded for this session; query fewer distinct solids".into(), + "volume/centroid cold-tessellation budget exceeded for this tab; query fewer distinct solids".into(), )); } - self.cold_tess_used += 1; + self.scene_mut().doc_api_cold_tess_used += 1; let body = self.resolve_body(id)?; let mesh = cadkernel::brep::mesh_body(&body, 0.1, 1e-4); let (v, c) = mesh_volume_centroid(&mesh); - self.mass_cache.insert(handle, (epoch, v, c)); + self.scene_mut().doc_api_mass_cache.insert(handle, (epoch, v, c)); Ok((v, c)) } } +/// Validate a raster-image file path before embedding it in a document. +/// Rejects empty paths, UNC/network paths, parent-traversal, and non-image extensions. +fn validate_image_path(path: &str) -> ApiResult<()> { + let p = path.trim(); + if p.is_empty() { + return Err(ApiError::validation("CreateRasterImage", "empty image file path")); + } + // UNC / network / device paths. + if p.starts_with("\\\\") || p.starts_with("//") { + return Err(ApiError::Unsupported("network/UNC image paths are not allowed".into())); + } + // Parent-traversal anywhere in the path. + if p.split(['/', '\\']).any(|seg| seg == "..") { + return Err(ApiError::Unsupported("parent-traversal ('..') in image path is not allowed".into())); + } + // Known image extension (last component). + let ext_ok = p.rsplit('.').next().map(|ext| { + matches!(ext.to_ascii_lowercase().as_str(), + "png" | "jpg" | "jpeg" | "bmp" | "gif" | "tif" | "tiff" | "webp" | "tga") + }).unwrap_or(false); + if !ext_ok { + return Err(ApiError::validation( + "CreateRasterImage", + format!("unsupported image file extension in {path:?}"), + )); + } + Ok(()) +} + fn kernel_placement(p: &PlacementSpec) -> cadkernel::brep::Placement { cadkernel::brep::Placement { x_axis: p.x_axis, @@ -687,7 +745,8 @@ mod tests { fn dispatch(host: &mut HostSession<'_>, env: DocApiEnvelope) -> ApiResult { let bytes = bincode::serialize(&env).unwrap(); - let out = execute_doc_api(host, 0, &bytes).expect("dispatch failed"); + let tab_id = host.tab_id(); + let out = execute_doc_api(host, tab_id, &bytes).expect("dispatch failed"); bincode::deserialize(&out).expect("receipt deserialize") } @@ -1156,6 +1215,25 @@ mod tests { if let cadkernel::geom2d::Curve::Arc(arc) = &curves[0] { assert!((arc.radius - 5.0).abs() < 0.01, "arc radius {} (semicircle)", arc.radius); } + + // Negative (CW) bulge: the center must be on the CW side (mirror of CCW). + // A single 90° bulge segment start=(0,0), end=(1,0), bulge=-0.4142 (~90° CW): + // correct center is BELOW the chord; the buggy double-sign put it above. + let neg = crate::app::doc_api_convert::bulge_arc_segment([0.0, 0.0], [1.0, 0.0], -0.4142) + .expect("negative bulge arc"); + if let cadkernel::geom2d::Curve::Arc(arc) = &neg { + // 90° arc, chord 1: radius = 0.5/sin(45°) ≈ 0.7071; center y must be negative. + assert!((arc.radius - 0.7071).abs() < 0.01, "cw arc radius {}", arc.radius); + assert!(arc.centre[1] < 0.0, "CW arc center y must be below the chord, got {:?}", arc.centre); + } else { + panic!("expected an arc for negative bulge"); + } + // And the CCW mirror: center above. + let pos = crate::app::doc_api_convert::bulge_arc_segment([0.0, 0.0], [1.0, 0.0], 0.4142) + .expect("positive bulge arc"); + if let cadkernel::geom2d::Curve::Arc(arc) = &pos { + assert!(arc.centre[1] > 0.0, "CCW arc center y must be above the chord, got {:?}", arc.centre); + } } // ── Phase 2c-ii: dimension sub-types (radius/diameter/angular) ────────── @@ -1309,6 +1387,75 @@ mod tests { } } + // ── Review-fix regression tests (Loft cap, CreateMany validation, Ellipse + // rotation, raster path validation) ───────────────────────────────────── + + #[test] + fn fix_loft_over_cap_is_rejected() { + let mut app = OpenCADStudio::new_for_test(); + let mut host = HostSession::new(&mut app, 0); + let over = ocs_doc_api::ops::BULK_ITEM_CAP + 1; + let profiles: Vec = (0..over).map(|i| ObjectId::from_u64(i as u64)).collect(); + let err = dispatch(&mut host, DocApiEnvelope::op(Operation::Loft { profiles })).unwrap_err(); + assert!(matches!(err, ApiError::Validation { .. }), "{err:?}"); + } + + #[test] + fn fix_create_many_invalid_curve_is_all_or_nothing() { + let mut app = OpenCADStudio::new_for_test(); + let mut host = HostSession::new(&mut app, 0); + let rev_before = host.scene().geometry_epoch; + // A valid point + an invalid ellipse (ratio > 1) in one CreateMany batch. + let specs = vec![ + ocs_doc_api::ops::EntitySpec::Curve(Curve2Spec::Point { position: [0.0; 3] }), + ocs_doc_api::ops::EntitySpec::Curve(Curve2Spec::Ellipse { + centre: [0.0; 3], major_axis: [1.0; 3], ratio: 2.0, start: 0.0, end: std::f64::consts::TAU, + }), + ]; + let err = dispatch(&mut host, DocApiEnvelope::op(Operation::CreateMany(specs))).unwrap_err(); + assert!(matches!(err, ApiError::Validation { .. }), "{err:?}"); + // All-or-nothing: NO entity was created (the point would have been added first). + assert_eq!(host.scene().geometry_epoch, rev_before, "no mutation on rejected CreateMany"); + } + + #[test] + fn fix_partial_ellipse_rotates_sweep() { + let mut app = OpenCADStudio::new_for_test(); + let mut host = HostSession::new(&mut app, 0); + // Partial ellipse arc start=0,end=pi/2; rotate by +pi/2 -> sweep becomes pi/2..pi. + let ell = new_id(&dispatch(&mut host, DocApiEnvelope::op(Operation::CreateCurve( + Curve2Spec::Ellipse { centre: [0.0; 3], major_axis: [4.0, 0.0, 0.0], ratio: 0.5, start: 0.0, end: std::f64::consts::FRAC_PI_2 }))).unwrap()); + let rot90 = ocs_doc_api::PlacementSpec { + x_axis: [0.0, 1.0, 0.0], y_axis: [-1.0, 0.0, 0.0], z_axis: [0.0, 0.0, 1.0], origin: [0.0; 3], + }; + dispatch(&mut host, DocApiEnvelope::op(Operation::Transform { id: ell, placement: rot90 })).unwrap(); + let Some(EntityType::Ellipse(e)) = host.document().get_entity(obj_to_handle(ell)) else { panic!("ellipse not found") }; + let half_pi = std::f64::consts::FRAC_PI_2; + assert!((e.start_parameter - half_pi).abs() < 1e-6, "start {} (expected {})", e.start_parameter, half_pi); + assert!((e.end_parameter - std::f64::consts::PI).abs() < 1e-6, "end {} (expected pi)", e.end_parameter); + // major_axis rotated from +X to +Y. + assert!((e.major_axis.y - 4.0).abs() < 1e-6, "major_axis {:?}", e.major_axis); + } + + #[test] + fn fix_raster_image_path_validation() { + let mut app = OpenCADStudio::new_for_test(); + let mut host = HostSession::new(&mut app, 0); + let mk = |path: &str| Operation::CreateRasterImage(ocs_doc_api::ops::RasterImageSpec { + file_path: path.into(), insertion_point: [0.0; 3], u_vector: [0.5, 0.0, 0.0], v_vector: [0.0, 0.5, 0.0], size: [10.0, 10.0], + }); + // Valid path succeeds. + assert!(dispatch(&mut host, DocApiEnvelope::op(mk("C:/img/photo.png"))).is_ok()); + // UNC path rejected. + assert!(dispatch(&mut host, DocApiEnvelope::op(mk("\\\\server\\share\\x.png"))).is_err()); + // Parent-traversal rejected. + assert!(dispatch(&mut host, DocApiEnvelope::op(mk("C:/img/../../etc/passwd.png"))).is_err()); + // Non-image extension rejected. + assert!(dispatch(&mut host, DocApiEnvelope::op(mk("C:/img/evil.exe"))).is_err()); + // Empty path rejected. + assert!(dispatch(&mut host, DocApiEnvelope::op(mk(""))).is_err()); + } + // ── Read-mostly family bounds (Leader/Mesh/Face3D/MLine/Helix) ────────── #[test] @@ -1603,6 +1750,18 @@ mod tests { assert_eq!(host.scene().geometry_epoch, before, "no entity on unknown block"); } + #[test] + fn doc_api_tab_mismatch_is_rejected() { + let mut app = OpenCADStudio::new_for_test(); + let mut host = HostSession::new(&mut app, 0); + // A request naming a DIFFERENT tab than the bound one is rejected. + let env = DocApiEnvelope::op(Operation::CreateCurve(Curve2Spec::Point { position: [0.0; 3] })); + let bytes = bincode::serialize(&env).unwrap(); + let wrong_tab = host.tab_id().wrapping_add(999); + let err = execute_doc_api(&mut host, wrong_tab, &bytes).unwrap_err(); + assert!(err.contains("tab mismatch"), "{err}"); + } + #[test] fn doc_api_unknown_id_surfaces_structured_error() { let mut app = OpenCADStudio::new_for_test(); diff --git a/src/app/doc_api_convert.rs b/src/app/doc_api_convert.rs index 9ccfb1aa0..f4dd1c424 100644 --- a/src/app/doc_api_convert.rs +++ b/src/app/doc_api_convert.rs @@ -115,6 +115,9 @@ pub fn transform_entity_geometry( e.center = apply(e.center); // The major-axis is a direction+length vector: rotate+scale it (no origin). e.major_axis = apply_dir(e.major_axis); + // A Z-rotation also shifts the partial-arc sweep (eccentric-anomaly params). + e.start_parameter += rot_z; + e.end_parameter += rot_z; EntityType::Ellipse(e) } EntityType::Spline(sp) => { @@ -191,7 +194,7 @@ fn v3(p: [f64; 3]) -> Vector3 { /// Convert a polyline bulge segment (start → end, bulge = tan(θ/4)) to a /// `geom2d::Curve::Arc`. Positive bulge = counter-clockwise arc; negative = clockwise. -fn bulge_arc_segment(start: [f64; 2], end: [f64; 2], bulge: f64) -> ApiResult { +pub(crate) fn bulge_arc_segment(start: [f64; 2], end: [f64; 2], bulge: f64) -> ApiResult { use cadkernel::geom2d::{Arc as KArc, Curve as KCurve}; let chord_x = end[0] - start[0]; let chord_y = end[1] - start[1]; @@ -207,11 +210,13 @@ fn bulge_arc_segment(start: [f64; 2], end: [f64; 2], bulge: f64) -> ApiResult= 0.0 { 1.0 } else { -1.0 }; let perp_x = -chord_y / chord_len * sign; let perp_y = chord_x / chord_len * sign; @@ -392,15 +397,7 @@ pub fn entity_bounds(entity: Option<&EntityType>, id: ObjectId) -> ApiResult { - let mut min = [f64::INFINITY; 3]; - let mut max = [f64::NEG_INFINITY; 3]; - for p in &s.control_points { - min = [min[0].min(p.x), min[1].min(p.y), min[2].min(p.z)]; - max = [max[0].max(p.x), max[1].max(p.y), max[2].max(p.z)]; - } - Aabb { min, max } - } + EntityType::Spline(s) => points_aabb(s.control_points.iter().collect(), id)?, EntityType::Viewport(vp) => Aabb { min: [ vp.center.x - vp.width / 2.0, @@ -443,16 +440,10 @@ pub fn entity_bounds(entity: Option<&EntityType>, id: ObjectId) -> ApiResult { - let pts = [d.first_point, d.second_point, d.definition_point]; - let mut min = [f64::INFINITY; 3]; - let mut max = [f64::NEG_INFINITY; 3]; - for p in pts { - min = [min[0].min(p.x), min[1].min(p.y), min[2].min(p.z)]; - max = [max[0].max(p.x), max[1].max(p.y), max[2].max(p.z)]; - } - Aabb { min, max } - } + EntityType::Dimension(acadrust::entities::Dimension::Linear(d)) => points_aabb( + [&d.first_point, &d.second_point, &d.definition_point].into_iter().collect(), + id, + )?, EntityType::Hatch(h) => { let mut min = [f64::INFINITY; 3]; let mut max = [f64::NEG_INFINITY; 3]; diff --git a/src/app/plugin_host.rs b/src/app/plugin_host.rs index dca04abb2..3e5977de2 100644 --- a/src/app/plugin_host.rs +++ b/src/app/plugin_host.rs @@ -18,10 +18,6 @@ pub(crate) struct HostSession<'a> { doc_store: Option>, /// In-flight DocApi v2 undo delta (between `push_undo` and `finalize_op`). doc_api_pending: Option, - /// Memoized volume/centroid per (handle, geometry_epoch) for DocApi mass queries. - pub(crate) mass_cache: std::collections::HashMap, - /// Per-session count of cold-cache mass tessellations (budget for query DoS). - pub(crate) cold_tess_used: usize, } impl<'a> HostSession<'a> { @@ -31,8 +27,6 @@ impl<'a> HostSession<'a> { tab, doc_store: None, doc_api_pending: None, - mass_cache: std::collections::HashMap::new(), - cold_tess_used: 0, } } diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 9642523f1..3fb4226c7 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1804,6 +1804,14 @@ pub struct Scene { pub block_meshes: HashMap, /// Kernel B-reps used by solid operations and exact-geometry saves. pub solid_models: HashMap, + /// Memoized DocApi volume/centroid per (handle, geometry_epoch) so repeated + /// mass queries on the same solid are O(1). Persists across DocApi dispatches. + #[cfg(not(target_arch = "wasm32"))] + pub doc_api_mass_cache: std::collections::HashMap, + /// Per-tab cold-tessellation budget for expensive DocApi volume/centroid + /// computations (bounded DoS surface). Persists across DocApi dispatches. + #[cfg(not(target_arch = "wasm32"))] + pub doc_api_cold_tess_used: usize, /// GPU render data for raster images (RasterImage entities), keyed by handle. pub images: HashMap, /// The viewport that is currently "entered" (MSPACE mode). @@ -2067,6 +2075,10 @@ impl Scene { material_base_dir: None, block_meshes: HashMap::default(), solid_models: HashMap::default(), + #[cfg(not(target_arch = "wasm32"))] + doc_api_mass_cache: std::collections::HashMap::default(), + #[cfg(not(target_arch = "wasm32"))] + doc_api_cold_tess_used: 0, images: HashMap::default(), active_viewport: None, bg_color: [33.0 / 255.0, 40.0 / 255.0, 48.0 / 255.0, 1.0],