forked from 1b-agent/opencadstudio-upstream
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.
This commit is contained in:
1 parent
b1f4047f9c
commit
d0bfa48037
2 files changed
+59
-2
No files matched your search
@@ -2149,6 +2149,7 @@ impl OpenCADStudio {
|
||||
Some(v) => v
|
||||
.parse::<i16>()
|
||||
.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::<i16>()
|
||||
.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"] {
|
||||
|
||||
+35
-2
@@ -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)]
|
||||
|
||||
Reference in new issue
Block a user