From d0bfa480370c8d2dbd6c308db8c7e425c2c1f907 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Wed, 30 Sep 2026 08:02:31 +0100 Subject: [PATCH] fix(units): clamp precision vars and their formatters Bug: SETVAR LUPREC and SETVAR AUPREC stored any parseable i16 verbatim, and the header value flows through UnitContext into the per-call formatters, which applied only a lower bound (.max(0)) with no upper bound: format_length and format_angle rendered {:.*} with the raw precision. SETVAR LUPREC 30000 therefore made format_length emit 30003-character strings on every properties and annotation formatting call - a ~30 KB allocation per formatted value - while format_area in the same file had long since capped itself with .min(15) and the units dialog only ever offers 0-8 places. Negative values were also stored verbatim. Reproduce: 1. Command line: SETVAR LUPREC 30000 (or AUPREC 30000), then select something so the properties palette formats lengths/angles. 2. Before: the header stores 30000 and each formatted value is ~30 KB (the test measured 30003 chars for one length); -3 is stored as-is. 3. After: the setter stores 8 (0-8 range, echoed truthfully), -3 stores 0, 5 stores 5 - and even a file carrying LUPREC 30000 in its header formats short output, because the formatters cap themselves. Fix: Two layers. The formatters get the cap format_area already uses: format_signed_length and format_signed_angle now clamp with .max(0).min(15), so every source - SETVAR, the units dialog, and values carried in DWG/DXF files - produces bounded output on the hot paths (15 matches the point past which f64 decimal output is meaningless, which is why format_area was written that way). The setters store x.clamp(0, 8) with the clamped value echoed back, the same stay-inside-the-app's-own-UI-range rule SETVAR ISOLINES follows against the Options slider. Architectural and fractional formats ignore precision (fixed 1/64-inch resolution) and are unaffected. Regression tests mirror the existing patterns: one forces hostile precision through the formatters while saving and restoring the thread-local unit context, one drives SETVAR through the command line and checks the stored header values. --- src/app/commands/styleprops.rs | 24 ++++++++++++++++++++++ src/entities/common.rs | 37 ++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/app/commands/styleprops.rs b/src/app/commands/styleprops.rs index c54d29b57..19842378d 100644 --- a/src/app/commands/styleprops.rs +++ b/src/app/commands/styleprops.rs @@ -2149,6 +2149,7 @@ impl OpenCADStudio { Some(v) => v .parse::() .map(|x| { + let x = x.clamp(0, 8); h.linear_unit_precision = x; (format!("LUPREC = {x}"), true) }) @@ -2171,6 +2172,7 @@ impl OpenCADStudio { Some(v) => v .parse::() .map(|x| { + let x = x.clamp(0, 8); h.angular_unit_precision = x; (format!("AUPREC = {x}"), true) }) @@ -3445,6 +3447,28 @@ mod tests { assert_eq!(app.tabs[i].scene.document.header.isolines, 0); } + /// `SETVAR LUPREC` / `AUPREC` stored any parseable `i16` verbatim. The + /// units dialog only ever offers 0-8 places, and the formatters read the + /// header on every properties/annotation call, so an unclamped value + /// (positive or negative) fed unbounded strings into those hot paths. + #[test] + fn setvar_precision_stays_within_the_range_the_units_dialog_offers() { + let mut app = fresh_app(); + let i = app.active_tab; + + let _ = app.run_command_line("SETVAR LUPREC 30000"); + assert_eq!(app.tabs[i].scene.document.header.linear_unit_precision, 8); + + let _ = app.run_command_line("SETVAR AUPREC 30000"); + assert_eq!(app.tabs[i].scene.document.header.angular_unit_precision, 8); + + let _ = app.run_command_line("SETVAR LUPREC -3"); + assert_eq!(app.tabs[i].scene.document.header.linear_unit_precision, 0); + + let _ = app.run_command_line("SETVAR LUPREC 5"); + assert_eq!(app.tabs[i].scene.document.header.linear_unit_precision, 5); + } + #[test] fn selection_commands_gather_instead_of_using_the_whole_drawing() { for command in ["SETBYLAYER", "OVERKILL"] { diff --git a/src/entities/common.rs b/src/entities/common.rs index 949eabf84..ab2e35674 100644 --- a/src/entities/common.rs +++ b/src/entities/common.rs @@ -122,7 +122,7 @@ pub fn without_negative_zero(text: String) -> String { fn format_signed_length(value: f64) -> String { let ctx = unit_context(); - let prec = ctx.luprec.max(0) as usize; + let prec = ctx.luprec.max(0).min(15) as usize; match ctx.lunits { 1 => format!("{:.*e}", prec, value), 3 => { @@ -196,7 +196,7 @@ pub fn format_angle(value_rad: f64) -> String { fn format_signed_angle(value_rad: f64) -> String { let ctx = unit_context(); - let prec = ctx.auprec.max(0) as usize; + let prec = ctx.auprec.max(0).min(15) as usize; match ctx.aunits { 1 => dms(value_rad.to_degrees(), prec), 2 => { @@ -1265,6 +1265,39 @@ mod length_format_tests { } } } + + /// LUPREC / AUPREC reach the formatters straight from the header, where + /// `SETVAR` or a carried file can hold any `i16`. `format_area` already + /// caps itself at 15; `format_length` and `format_angle` printed the raw + /// count on every call, so precision 30_000 bloated each formatted value + /// to ~30 KB on the properties hot path. + #[test] + fn huge_precision_cannot_bloat_formatting_output() { + let original = unit_context(); + let length = with_units(2, 30_000, 12.3456); + let mut ctx = unit_context(); + ctx.aunits = 0; + ctx.auprec = 30_000; + set_unit_context(ctx); + let angle = format_angle(1.0); + let area = format_area(12.3456); + set_unit_context(original); + assert!( + length.len() < 64, + "format_length produced {} chars", + length.len() + ); + assert!( + angle.len() < 64, + "format_angle produced {} chars", + angle.len() + ); + assert!( + area.len() < 64, + "format_area produced {} chars", + area.len() + ); + } } #[cfg(test)]