diff --git a/Cargo.lock b/Cargo.lock index b77485ae0..c3a087e84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1096,7 +1096,7 @@ dependencies = [ [[package]] name = "craft-vectorcraft-format" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=9be9a63782298ab59cd9b7a42ee9cba4f42a5ccb#9be9a63782298ab59cd9b7a42ee9cba4f42a5ccb" +source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" dependencies = [ "flate2", "serde", diff --git a/Cargo.toml b/Cargo.toml index 9b4b99c92..2d53acdc3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ craft-proof = { git = "https://github.com/storytold/craft-libs", rev = "9be9a637 # desktop. The `egui` feature adds `craft_window::decorate`. (craft-libs#217.) craft-window = { git = "https://github.com/storytold/craft-libs", rev = "9be9a63782298ab59cd9b7a42ee9cba4f42a5ccb", features = ["egui"] } # The `.vectorcraft` envelope: its members, version, embedded images and the data after the JSON. -craft-vectorcraft-format = { git = "https://github.com/storytold/craft-libs", rev = "9be9a63782298ab59cd9b7a42ee9cba4f42a5ccb" } +craft-vectorcraft-format = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d" } # craft-update: app-specific pretend versions apply to startup and manual checks. craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "1b34eba41e62138fc2438d2734523b302840b45d", features = ["egui"] } # internal diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 5d7aabd07..1da653311 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -510,20 +510,32 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result // at `MAX_READ_ANCHORS` anchors together, the document at `MAX_READ_NODES` objects, and the // fields read before an object's type at `MAX_BUFFERED_VALUES` values. The preview and the // PDF are skipped without being kept. - let read = || { - vectorcraft_doc::node::with_node_budget(max_nodes, || { - vectorcraft_doc::node::with_buffered_value_budget(max_values, || Contents::::read(file.json())) + fn within_budgets(max_nodes: usize, max_values: usize, read: impl FnOnce() -> T) -> T { + vectorcraft_doc::geom::path::with_anchor_budget(vectorcraft_doc::geom::path::MAX_READ_ANCHORS, || { + vectorcraft_doc::node::with_node_budget(max_nodes, || vectorcraft_doc::node::with_buffered_value_budget(max_values, read)) }) - }; - let mut f = vectorcraft_doc::geom::path::with_anchor_budget(vectorcraft_doc::geom::path::MAX_READ_ANCHORS, read).map_err(|e| match e { + } + let read_error = |e| match e { ReadError::Json(e) => { let why = if e.to_string().contains("recursion limit") { format!("its objects are nested too deep ({e})") } else { e.to_string() }; FormatError::NotVectorcraft(why) } ReadError::Envelope(e) => e.into(), e => FormatError::NotVectorcraft(e.to_string()), - })?; - let pdf_compatible = pdf_member(file.json()).map_err(|e| FormatError::NotVectorcraft(e.to_string()))?; + }; + // The document and the saved PDF preference come from one pass over the JSON. + let (mut f, pdf_compatible) = match within_budgets(max_nodes, max_values, || Contents::::read_with_pdf_preference(file.json())) { + Ok(read) => read, + // That pass refuses a `pdfCompatible` that is not a boolean, and any other data error. + // The file is read again without the preference, and only then is its JSON scanned for + // the preference a second time, so such a file opens as before. + Err(ReadError::Json(e)) if e.is_data() => { + let f = within_budgets(max_nodes, max_values, || Contents::::read(file.json())).map_err(read_error)?; + let pdf_compatible = pdf_member(file.json()).map_err(|e| FormatError::NotVectorcraft(e.to_string()))?; + (f, pdf_compatible) + } + Err(e) => return Err(read_error(e)), + }; let info = FileInfo { version: f.version, legacy: f.format == LEGACY_EXTENSION, pdf_compatible }; // A version 1 document saved before VectorCraft 0.21.0, or for an app before it (an older file // version): its Twist angles turned the other way. The app that wrote it is read from the JSON @@ -615,9 +627,17 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result Ok(NativeFile { doc, depth_notes, info, profiles }) } +#[cfg(test)] +thread_local! { + /// Tests: how many times a file's JSON was scanned again for its PDF preference on this thread. + static PDF_MEMBER_SCANS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// The saved PDF preference, falling back to the presence of its payload without retaining or /// decoding it. The array form has neither member. Repeated members use their last value. fn pdf_member(json: &[u8]) -> Result { + #[cfg(test)] + PDF_MEMBER_SCANS.with(|n| n.set(n.get() + 1)); struct PdfMember; impl<'de> Visitor<'de> for PdfMember { type Value = bool; @@ -801,6 +821,32 @@ mod tests { } } + #[test] + fn only_a_preference_that_is_not_a_boolean_scans_the_json_again() { + let scans_for = |bytes: &[u8]| { + PDF_MEMBER_SCANS.with(|n| n.set(0)); + let compatible = load_info(bytes).unwrap().1.pdf_compatible; + (compatible, PDF_MEMBER_SCANS.with(|n| n.get())) + }; + let d = Document::new(10.0, 10.0); + assert_eq!(scans_for(&save(&d, false)), (false, 0)); + assert_eq!(scans_for(&save(&d, true)), (false, 0)); + for compatible in [false, true] { + let bytes = save_with(&d, &SaveOptions { pdf_compatible: Some(compatible), ..SaveOptions::default() }).unwrap(); + assert_eq!(scans_for(&bytes), (compatible, 0)); + } + let with_pdf = save_with(&d, &SaveOptions { pdf: Some(vec![7; 1024]), ..SaveOptions::default() }).unwrap(); + assert_eq!(scans_for(&with_pdf), (true, 0)); + let mut value: serde_json::Value = serde_json::from_slice(&save(&d, false)).unwrap(); + for odd in [serde_json::json!("yes"), serde_json::json!(1), serde_json::json!([true])] { + value["pdfCompatible"] = odd.clone(); + assert_eq!(scans_for(&serde_json::to_vec(&value).unwrap()), (false, 1), "{odd}"); + } + // A document that can't be read still fails, after the second read. + value["document"] = serde_json::json!("not a document"); + assert!(load_info(&serde_json::to_vec(&value).unwrap()).is_err()); + } + #[test] fn file_info_keeps_the_pdf_preference_without_artboards() { let mut d = Document::new(10.0, 10.0);