forked from 1b-agent/opencadstudio-upstream
web: harden plot preview after adversarial review
Three reviewers (security, correctness, performance) converged on:
- Download PDF died on the second click: Closure::once_into_js throws
when invoked twice. The handler is now a plain FnMut kept for the
overlay's lifetime.
- Rasterisation was bounded by page count, not pixels: 12 A0 sheets at
150 DPI peaked near 1.8 GB of RGBA plus ~1.7 GB of decoded images,
and a drawing declaring absurd paper sizes made the rasteriser
saturate its u16 dimensions into a ~17 GB allocation that aborts the
tab. Preview now computes a uniform DPI under a 64-Mpx total budget
(floor 24 DPI) from the page tree before allocating, with a per-page
4096-px edge clamp.
- Overlays stacked when previewing twice and their blob URLs were only
revoked via Close. The overlay is now single-instance (id-tagged);
a replacement or Close revokes every URL through a shared registry.
- emit_image resampled before probing the XObject cache, so a title
block placed on 12 layouts ran 12 Lanczos passes to embed once.
The cache is now probed with the target dimensions first.
- Preview downloads are named {stem}_preview.pdf instead of a constant
plot_preview.pdf, and the page label says 'first N of M' when the
preview is capped while the PDF keeps every page.
This commit is contained in:
1 parent
6306359390
commit
2a05479d69
5 files changed
+218
-25
No files matched your search
+20
-4
@@ -3266,8 +3266,9 @@ impl OpenCADStudio {
|
||||
// PDF and open it in a browser tab for the browser print dialog.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
{
|
||||
let stem = self.active_stem("_layouts");
|
||||
let work = move || {
|
||||
crate::io::print_to_printer::preview_pages_pdf(&pages)
|
||||
crate::io::print_to_printer::preview_pages_pdf(&stem, &pages)
|
||||
.map(|_| crate::t!("Opened plot preview.").into_owned())
|
||||
.map_err(|e| crate::tf!("Preview failed: {e}").into_owned())
|
||||
};
|
||||
@@ -3523,9 +3524,10 @@ impl OpenCADStudio {
|
||||
// in a browser tab, where the browser's own print dialog takes over.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
{
|
||||
let stem = self.active_stem("");
|
||||
background_task(
|
||||
move || {
|
||||
crate::io::print_to_printer::preview_page_pdf(&page)
|
||||
crate::io::print_to_printer::preview_page_pdf(&stem, &page)
|
||||
.map(|_| crate::t!("Opened plot preview.").into_owned())
|
||||
.map_err(|e| crate::tf!("Preview failed: {e}").into_owned())
|
||||
},
|
||||
@@ -3631,6 +3633,18 @@ impl OpenCADStudio {
|
||||
dir.join(format!("{stem}_{ts}.pdf"))
|
||||
}
|
||||
|
||||
/// File stem of the active drawing for default output names, with an
|
||||
/// optional suffix (e.g. "_layouts" for print-all). Falls back to
|
||||
/// "drawing" when the tab has never been saved.
|
||||
fn active_stem(&self, suffix: &str) -> String {
|
||||
self.tabs[self.active_tab]
|
||||
.current_path
|
||||
.as_deref()
|
||||
.and_then(|path| path.file_stem())
|
||||
.map(|stem| format!("{}{}", stem.to_string_lossy(), suffix))
|
||||
.unwrap_or_else(|| format!("drawing{suffix}"))
|
||||
}
|
||||
|
||||
/// Open the full Plot / Print dialog, seeding its state from the active
|
||||
/// layout's plot settings and the printers found on the system.
|
||||
pub(super) fn on_plot_dialog_open(&mut self) -> Task<Message> {
|
||||
@@ -4454,8 +4468,9 @@ impl OpenCADStudio {
|
||||
// Preview renders to a temp PDF and opens it — never a save dialog,
|
||||
// whatever the output target is.
|
||||
if preview {
|
||||
let stem = self.active_stem("");
|
||||
let work = move || {
|
||||
crate::io::print_to_printer::preview_page_pdf(&page)
|
||||
crate::io::print_to_printer::preview_page_pdf(&stem, &page)
|
||||
.map(|_| crate::t!("Opened plot preview.").into_owned())
|
||||
.map_err(|e| crate::tf!("Preview failed: {e}").into_owned())
|
||||
};
|
||||
@@ -4479,8 +4494,9 @@ impl OpenCADStudio {
|
||||
let page = self.layout_plot_page();
|
||||
|
||||
if preview {
|
||||
let stem = self.active_stem("");
|
||||
let work = move || {
|
||||
crate::io::print_to_printer::preview_page_pdf(&page)
|
||||
crate::io::print_to_printer::preview_page_pdf(&stem, &page)
|
||||
.map(|_| crate::t!("Opened plot preview.").into_owned())
|
||||
.map_err(|e| crate::tf!("Preview failed: {e}").into_owned())
|
||||
};
|
||||
|
||||
+10
-1
@@ -305,7 +305,13 @@ fn emit_image(
|
||||
// One shared factor from the denser axis preserves the pixel aspect ratio.
|
||||
let (w, h) = downsample_target(width, height, dpi_x.max(dpi_y), target);
|
||||
// Skip token reductions (< 5%): a Lanczos pass costs more than it saves.
|
||||
if (w != width || h != height) && w as f64 / (width as f64) < 0.95 {
|
||||
let needs_resample = (w != width || h != height) && w as f64 / (width as f64) < 0.95;
|
||||
// The same Arc placed repeatedly (a title block on every layout)
|
||||
// resamples once: an existing XObject at the target dims means
|
||||
// only the draw transform below is new work.
|
||||
let already_embedded =
|
||||
resources.contains_key(&(std::sync::Arc::as_ptr(&image.pixels) as usize, w, h));
|
||||
if needs_resample && !already_embedded {
|
||||
if let Some(resized) = image::RgbaImage::from_raw(
|
||||
width,
|
||||
height,
|
||||
@@ -319,6 +325,9 @@ fn emit_image(
|
||||
height = h;
|
||||
pixels = std::borrow::Cow::Owned(resized);
|
||||
}
|
||||
} else if already_embedded {
|
||||
width = w;
|
||||
height = h;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -336,10 +336,15 @@ pub fn open_in_viewer(path: &std::path::Path) -> Result<(), String> {
|
||||
}
|
||||
|
||||
/// Render a page and show it the way each platform previews: the OS PDF
|
||||
/// viewer on desktop, an in-page rasterised overlay on the web.
|
||||
pub fn preview_page_pdf(page: &crate::io::pdf_export::PdfPageInput) -> Result<(), String> {
|
||||
/// viewer on desktop, an in-page rasterised overlay on the web. `stem` names
|
||||
/// the web download (`{stem}_preview.pdf`); the desktop temp file is unchanged.
|
||||
pub fn preview_page_pdf(
|
||||
stem: &str,
|
||||
page: &crate::io::pdf_export::PdfPageInput,
|
||||
) -> Result<(), String> {
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
{
|
||||
let _ = stem;
|
||||
let tmp = temp_pdf_path("preview");
|
||||
crate::io::pdf_export::export_pdf(page, &tmp)?;
|
||||
open_in_viewer(&tmp)
|
||||
@@ -347,26 +352,32 @@ pub fn preview_page_pdf(page: &crate::io::pdf_export::PdfPageInput) -> Result<()
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
{
|
||||
let bytes = crate::io::pdf_export::render_pdf_bytes(std::slice::from_ref(page), None)?;
|
||||
web_preview_overlay(&bytes)
|
||||
web_preview_overlay(stem, &bytes)
|
||||
}
|
||||
}
|
||||
|
||||
/// Web-only multi-page variant used by print-all: render every layout into
|
||||
/// one PDF and preview the first pages in the overlay.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
pub fn preview_pages_pdf(pages: &[crate::io::pdf_export::PdfPageInput]) -> Result<(), String> {
|
||||
pub fn preview_pages_pdf(
|
||||
stem: &str,
|
||||
pages: &[crate::io::pdf_export::PdfPageInput],
|
||||
) -> Result<(), String> {
|
||||
let bytes = crate::io::pdf_export::render_pdf_bytes(pages, None)?;
|
||||
web_preview_overlay(&bytes)
|
||||
web_preview_overlay(stem, &bytes)
|
||||
}
|
||||
|
||||
/// Web preview: rasterise the plotted PDF (the same hayro pipeline that draws
|
||||
/// PDF underlays) and show the pages in an in-page overlay. The browser gets
|
||||
/// plain PNGs, so neither popup permission nor a PDF viewer plugin is needed.
|
||||
/// Rasterisation is bounded by a total pixel budget so large sheets lower the
|
||||
/// preview DPI instead of the tab's memory ceiling.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
fn web_preview_overlay(bytes: &[u8]) -> Result<(), String> {
|
||||
fn web_preview_overlay(stem: &str, bytes: &[u8]) -> Result<(), String> {
|
||||
use crate::scene::model::pdf_raster;
|
||||
const PREVIEW_DPI: f32 = 150.0;
|
||||
const MAX_PREVIEW_PAGES: usize = 12;
|
||||
let pages = crate::scene::model::pdf_raster::rasterize_bytes_pages(
|
||||
let (pages, total_pages) = pdf_raster::rasterize_preview_pages(
|
||||
std::sync::Arc::new(bytes.to_vec()),
|
||||
PREVIEW_DPI,
|
||||
MAX_PREVIEW_PAGES,
|
||||
@@ -385,7 +396,7 @@ fn web_preview_overlay(bytes: &[u8]) -> Result<(), String> {
|
||||
pngs.push(png);
|
||||
}
|
||||
}
|
||||
crate::sys::show_pdf_preview("plot_preview.pdf", bytes, &pngs);
|
||||
crate::sys::show_pdf_preview(&format!("{stem}_preview.pdf"), bytes, &pngs, total_pages);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -130,6 +130,85 @@ pub fn rasterize_bytes_pages(bytes: Arc<Vec<u8>>, dpi: f32, max_pages: usize) ->
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Upper bound on one previewed page's pixel edge. Rasteriser dimensions
|
||||
/// saturate far above this, so the clamp must happen here: a page sized by a
|
||||
/// hostile (or merely mistyped) paper width would otherwise allocate until
|
||||
/// the tab aborts.
|
||||
pub const PREVIEW_MAX_PAGE_EDGE_PX: f64 = 4096.0;
|
||||
|
||||
/// Total pixel budget for one preview. Bounds both the wasm-heap RGBA pages
|
||||
/// and the browser's decoded-image memory behind the overlay. Memory scales
|
||||
/// with DPI², so a multipage preview lowers its DPI instead of its content.
|
||||
pub const PREVIEW_PIXEL_BUDGET: f64 = 64.0 * 1024.0 * 1024.0;
|
||||
|
||||
/// DPI that keeps every page edge within [`PREVIEW_MAX_PAGE_EDGE_PX`].
|
||||
fn page_edge_clamped_dpi(page_pt: (f32, f32), dpi: f32) -> f32 {
|
||||
let (w_pt, h_pt) = (page_pt.0 as f64, page_pt.1 as f64);
|
||||
let clamp = |edge_pt: f64| -> f64 {
|
||||
if edge_pt <= 0.0 {
|
||||
dpi as f64
|
||||
} else {
|
||||
(PREVIEW_MAX_PAGE_EDGE_PX * 72.0 / edge_pt).min(dpi as f64)
|
||||
}
|
||||
};
|
||||
clamp(w_pt).min(clamp(h_pt)).max(1.0) as f32
|
||||
}
|
||||
|
||||
/// Rasterise the first `max_pages` pages of freshly generated PDF bytes for
|
||||
/// preview under a total pixel budget: the DPI starts at `max_dpi` and drops
|
||||
/// uniformly (never below a legibility floor of 24) until the whole set fits,
|
||||
/// with a per-page edge clamp for absurd sheet sizes. Returns the pages and
|
||||
/// the PDF's true page count so the caller can say "first N of M".
|
||||
pub fn rasterize_preview_pages(
|
||||
bytes: Arc<Vec<u8>>,
|
||||
max_dpi: f32,
|
||||
max_pages: usize,
|
||||
) -> (Vec<Arc<PdfPage>>, usize) {
|
||||
let Ok(pdf) = Pdf::new(bytes) else {
|
||||
return (Vec::new(), 0);
|
||||
};
|
||||
let total = pdf.pages().len();
|
||||
let count = total.min(max_pages);
|
||||
// Point sizes come from the page tree — no allocation before the clamp.
|
||||
let sizes: Vec<(f32, f32)> = (0..count)
|
||||
.map(|index| {
|
||||
pdf.pages()
|
||||
.get(index)
|
||||
.map(|page| page.base_dimensions())
|
||||
.unwrap_or((595.0, 842.0))
|
||||
})
|
||||
.collect();
|
||||
let pixels_at_max: f64 = sizes
|
||||
.iter()
|
||||
.map(|&(w_pt, h_pt)| {
|
||||
let dpi = page_edge_clamped_dpi((w_pt, h_pt), max_dpi) as f64;
|
||||
let w = (w_pt as f64 / 72.0 * dpi).max(1.0);
|
||||
let h = (h_pt as f64 / 72.0 * dpi).max(1.0);
|
||||
w * h
|
||||
})
|
||||
.sum();
|
||||
let dpi = if pixels_at_max <= PREVIEW_PIXEL_BUDGET {
|
||||
max_dpi
|
||||
} else {
|
||||
((max_dpi as f64) * (PREVIEW_PIXEL_BUDGET / pixels_at_max).sqrt())
|
||||
.clamp(24.0, max_dpi as f64) as f32
|
||||
};
|
||||
let pages = sizes
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter_map(|(index, &size)| {
|
||||
let page = raster_page(&pdf, index, page_edge_clamped_dpi(size, dpi))?;
|
||||
Some(Arc::new(PdfPage {
|
||||
pixels: page.pixels,
|
||||
width: page.width,
|
||||
height: page.height,
|
||||
dpi,
|
||||
}))
|
||||
})
|
||||
.collect();
|
||||
(pages, total)
|
||||
}
|
||||
|
||||
fn source_bytes(path: &str) -> Option<Arc<Vec<u8>>> {
|
||||
if let Some(bytes) = source_cache()
|
||||
.lock()
|
||||
@@ -190,4 +269,39 @@ mod tests {
|
||||
let capped = rasterize_bytes_pages(bytes, 72.0, 2);
|
||||
assert_eq!(capped.len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preview_pages_keep_full_dpi_for_small_sets() {
|
||||
let mut document = PdfDocument::new("small set");
|
||||
for _ in 0..3 {
|
||||
document
|
||||
.pages
|
||||
.push(OutputPage::new(Mm(50.8), Mm(25.4), Vec::new()));
|
||||
}
|
||||
let bytes = Arc::new(document.save(&PdfSaveOptions::default(), &mut Vec::new()));
|
||||
|
||||
let (pages, total) = rasterize_preview_pages(bytes, 72.0, 12);
|
||||
assert_eq!((total, pages.len()), (3, 3));
|
||||
assert_eq!((pages[0].width, pages[0].height), (144, 72));
|
||||
assert_eq!(pages[0].dpi, 72.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preview_pages_clamp_absurd_sheet_sizes_before_allocating() {
|
||||
// 2 m × 2 m sheet: naive 150 DPI would be ~139 Mpx for one page; the
|
||||
// edge clamp bounds each side instead of letting the rasteriser
|
||||
// allocate it (which saturates its u16 dims and aborts the tab).
|
||||
let mut document = PdfDocument::new("huge sheet");
|
||||
document
|
||||
.pages
|
||||
.push(OutputPage::new(Mm(2000.0), Mm(2000.0), Vec::new()));
|
||||
let bytes = Arc::new(document.save(&PdfSaveOptions::default(), &mut Vec::new()));
|
||||
|
||||
let (pages, total) = rasterize_preview_pages(bytes, 150.0, 12);
|
||||
assert_eq!((total, pages.len()), (1, 1));
|
||||
assert!(pages[0].width <= 4096 && pages[0].height <= 4096);
|
||||
assert!(pages[0].width >= 4000 && pages[0].height >= 4000);
|
||||
let pixels = pages[0].width as f64 * pages[0].height as f64;
|
||||
assert!(pixels <= 4096.0 * 4096.0, "clamped, not {pixels} px");
|
||||
}
|
||||
}
|
||||
+55
-12
@@ -377,7 +377,7 @@ pub fn download_bytes(name: &str, bytes: &[u8]) {
|
||||
/// browser hosts routinely withhold. A Download button still hands out the
|
||||
/// vector PDF, Close drops the overlay. Web only.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>]) {
|
||||
pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>], total_pages: usize) {
|
||||
use wasm_bindgen::JsCast;
|
||||
let Some(window) = web_sys::window() else {
|
||||
return;
|
||||
@@ -385,6 +385,20 @@ pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>]) {
|
||||
let Some(document) = window.document() else {
|
||||
return;
|
||||
};
|
||||
// One preview at a time: a replotted preview replaces its predecessor and
|
||||
// revokes its blob URLs, so repeated previews cannot stack overlays or
|
||||
// leak the images underneath.
|
||||
const OVERLAY_ID: &str = "ocs-plot-preview";
|
||||
if let Some(previous) = document.get_element_by_id(OVERLAY_ID) {
|
||||
previous.remove();
|
||||
}
|
||||
for url in preview_urls()
|
||||
.lock()
|
||||
.unwrap_or_else(|poisoned| poisoned.into_inner())
|
||||
.drain(..)
|
||||
{
|
||||
let _ = web_sys::Url::revoke_object_url(&url);
|
||||
}
|
||||
if page_pngs.is_empty() {
|
||||
// Rasterisation produced nothing: deliver the PDF itself rather than
|
||||
// show an empty frame.
|
||||
@@ -396,6 +410,7 @@ pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>]) {
|
||||
return;
|
||||
};
|
||||
let backdrop: web_sys::HtmlElement = el.unchecked_into();
|
||||
backdrop.set_id(OVERLAY_ID);
|
||||
backdrop
|
||||
.set_attribute(
|
||||
"style",
|
||||
@@ -433,6 +448,10 @@ pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>]) {
|
||||
download_bytes(name, pdf_bytes);
|
||||
return;
|
||||
}
|
||||
preview_urls()
|
||||
.lock()
|
||||
.unwrap_or_else(|poisoned| poisoned.into_inner())
|
||||
.extend(urls.iter().cloned());
|
||||
|
||||
// Sticky action bar so Download/Close stay reachable while pages scroll.
|
||||
let bar: web_sys::HtmlElement = document
|
||||
@@ -451,11 +470,19 @@ pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>]) {
|
||||
.unwrap_or_else(|| backdrop.clone());
|
||||
if let Ok(el) = document.create_element("span") {
|
||||
let label: web_sys::HtmlElement = el.unchecked_into();
|
||||
label.set_inner_text(&format!(
|
||||
"Plot preview — {} page{}",
|
||||
urls.len(),
|
||||
if urls.len() == 1 { "" } else { "s" }
|
||||
));
|
||||
label.set_inner_text(&if total_pages > urls.len() {
|
||||
format!(
|
||||
"Plot preview — first {} of {} pages",
|
||||
urls.len(),
|
||||
total_pages
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"Plot preview — {} page{}",
|
||||
urls.len(),
|
||||
if urls.len() == 1 { "" } else { "s" }
|
||||
)
|
||||
});
|
||||
bar.append_child(&label).ok();
|
||||
}
|
||||
let button = |text: &str| -> Option<web_sys::HtmlElement> {
|
||||
@@ -473,18 +500,26 @@ pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>]) {
|
||||
if let Some(dl) = button("Download PDF") {
|
||||
let name = name.to_string();
|
||||
let bytes = pdf_bytes.to_vec();
|
||||
let click = wasm_bindgen::closure::Closure::once_into_js(move || {
|
||||
download_bytes(&name, &bytes);
|
||||
});
|
||||
let click: js_sys::Function = click.unchecked_into();
|
||||
// A plain FnMut stays usable for every click; the one-shot variant
|
||||
// throws on the second click, and users click Download twice.
|
||||
let closure: wasm_bindgen::closure::Closure<dyn FnMut()> =
|
||||
wasm_bindgen::closure::Closure::new(move || {
|
||||
download_bytes(&name, &bytes);
|
||||
});
|
||||
let click: js_sys::Function = closure.as_ref().unchecked_ref::<js_sys::Function>().clone();
|
||||
closure.forget();
|
||||
dl.set_onclick(Some(&click));
|
||||
bar.append_child(&dl).ok();
|
||||
}
|
||||
if let Some(close) = button("Close") {
|
||||
let backdrop = backdrop.clone();
|
||||
let click = wasm_bindgen::closure::Closure::once_into_js(move || {
|
||||
for url in &urls {
|
||||
let _ = web_sys::Url::revoke_object_url(url);
|
||||
for url in preview_urls()
|
||||
.lock()
|
||||
.unwrap_or_else(|poisoned| poisoned.into_inner())
|
||||
.drain(..)
|
||||
{
|
||||
let _ = web_sys::Url::revoke_object_url(&url);
|
||||
}
|
||||
backdrop.remove();
|
||||
});
|
||||
@@ -500,6 +535,14 @@ pub fn show_pdf_preview(name: &str, pdf_bytes: &[u8], page_pngs: &[Vec<u8>]) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Blob URLs owned by the live preview overlay, revoked when it is closed or
|
||||
/// replaced. They live outside the wasm heap and are not GC'd on their own.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
fn preview_urls() -> &'static std::sync::Mutex<Vec<String>> {
|
||||
static URLS: std::sync::OnceLock<std::sync::Mutex<Vec<String>>> = std::sync::OnceLock::new();
|
||||
URLS.get_or_init(|| std::sync::Mutex::new(Vec::new()))
|
||||
}
|
||||
|
||||
/// Short platform string for bug reports: OS + architecture on the desktop,
|
||||
/// the browser user-agent on the web.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
|
||||
Reference in new issue
Block a user