diff --git a/Cargo.lock b/Cargo.lock index 40e2fa48..54a6228f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -526,7 +526,7 @@ dependencies = [ "http 0.2.12", "http 1.5.0", "http-body 1.1.0", - "lru", + "lru 0.18.2", "percent-encoding", "regex-lite", "sha2 0.11.0", @@ -2140,6 +2140,8 @@ checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" [[package]] name = "dpp-aas" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af4ec4a1cc09b41cc32c1d5c40e595c541c0c6c6e52cec2670810524a7548123" dependencies = [ "dpp-domain", "dpp-vocab", @@ -2150,6 +2152,8 @@ dependencies = [ [[package]] name = "dpp-calc" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1f5a7897583c4fb6c43056506d83acfc71c1eb14a7d239dc71c154b2ce8d12d" dependencies = [ "chrono", "dpp-rules", @@ -2215,6 +2219,8 @@ dependencies = [ [[package]] name = "dpp-crypto" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1747716351031fbd56464d367ebfa5c2bc3b21d6bdbef10f6705a52a6bae2149" dependencies = [ "aes-gcm", "anyhow", @@ -2263,6 +2269,8 @@ dependencies = [ [[package]] name = "dpp-digital-link" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee158b22c553f128c436121d011c8c9f0f6af67a5a33a6594828e837c2024863" dependencies = [ "dpp-domain", "serde", @@ -2273,6 +2281,8 @@ dependencies = [ [[package]] name = "dpp-domain" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a19209f530263d1bfb75a86cc3b2cd54c058b87b57ff57e1b898072e4a7f452" dependencies = [ "async-trait", "chrono", @@ -2442,6 +2452,8 @@ dependencies = [ [[package]] name = "dpp-plugin-traits" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3379fa5d91c4f7d179bb92dd08cfebc605ba3d699d9cb3729f0910aeaf8888d" dependencies = [ "semver", "serde", @@ -2452,6 +2464,8 @@ dependencies = [ [[package]] name = "dpp-registry" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b626175826e18ef802f2e9de5da9ea449a202e1d34823e5771880fedefaf85b6" dependencies = [ "chrono", "dpp-domain", @@ -2500,6 +2514,7 @@ dependencies = [ "rand 0.10.2", "redis", "reqwest", + "rqrr", "serde", "serde_json", "thiserror 2.0.19", @@ -2514,6 +2529,8 @@ dependencies = [ [[package]] name = "dpp-rules" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8bbedb7cce0dad9d136793248444eba53c305a2811168d1bf3a7f229d61b02ce" dependencies = [ "base64 0.23.1", "chrono", @@ -2617,6 +2634,8 @@ dependencies = [ [[package]] name = "dpp-vc" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8feb107d2fe5a7a4391e3dfb46cfaa8335e2b9ef678bec9774a1669e1fa58a1" dependencies = [ "anyhow", "async-trait", @@ -2637,6 +2656,8 @@ dependencies = [ [[package]] name = "dpp-vocab" version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73c5b4713451f179e3f7bbfa09f19c94cfb3713709a43116328875ef44a03735" dependencies = [ "serde", "serde_json", @@ -3143,6 +3164,34 @@ dependencies = [ "thread_local", ] +[[package]] +name = "g2gen" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c5a7e0eb46f83a20260b850117d204366674e85d3a908d90865c78df9a6b1dfc" +dependencies = [ + "g2poly", + "proc-macro2", + "quote", + "syn 2.0.119", +] + +[[package]] +name = "g2p" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "539e2644c030d3bf4cd208cb842d2ce2f80e82e6e8472390bcef83ceba0d80ad" +dependencies = [ + "g2gen", + "g2poly", +] + +[[package]] +name = "g2poly" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "312d2295c7302019c395cfb90dacd00a82a2eabd700429bba9c7a3f38dbbe11b" + [[package]] name = "generator" version = "0.8.9" @@ -4063,6 +4112,15 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "lru" +version = "0.16.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f66e8d5d03f609abc3a39e6f08e4164ebf1447a732906d39eb9b99b7919ef39" +dependencies = [ + "hashbrown 0.16.1", +] + [[package]] name = "lru" version = "0.18.2" @@ -4853,7 +4911,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b570b25f7617e43d59005d0990ccb79e950a423952cea19671b7a876da390adf" dependencies = [ "anyhow", - "itertools 0.13.0", + "itertools 0.14.0", "proc-macro2", "quote", "syn 2.0.119", @@ -5398,6 +5456,17 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rqrr" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ffbe87d9e8db95652c25ded2418150e00b08c2fde09e23ec15896d2c470c6631" +dependencies = [ + "g2p", + "image 0.24.9", + "lru 0.16.4", +] + [[package]] name = "rustc-demangle" version = "0.1.28" @@ -6325,7 +6394,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.3", "once_cell", "rustix", "windows-sys 0.61.2", diff --git a/crates/dpp-render/src/carrier.rs b/crates/dpp-render/src/carrier.rs index 90f180c7..5fdda90f 100644 --- a/crates/dpp-render/src/carrier.rs +++ b/crates/dpp-render/src/carrier.rs @@ -2,6 +2,35 @@ use serde_json::Value; +/// The quiet zone every rendering of a carrier must leave around the symbol, +/// in modules. +/// +/// ISO/IEC 18004 requires four modules of blank margin on all four sides of a +/// QR symbol. It is not decoration: the quiet zone is what lets a scanner find +/// the symbol's edge, and a code printed flush to other artwork — or to the +/// edge of a label — is out of spec whether or not any particular decoder is +/// lenient enough to read it anyway. +/// +/// # Why this is a shared constant and not a number in each renderer +/// +/// It was a number in each renderer, and they disagreed. The SVG rendered for +/// the passport page applied four modules; the PNG served by the resolver's +/// `/qr` route applied **none**, sizing its image at exactly `width * scale`. +/// The divergence ran the wrong way round, too — the screen rendering, which a +/// browser surrounds with white page anyway, was the compliant one, while the +/// downloadable PNG an operator would actually print onto a label was the +/// symbol with no margin at all. +/// +/// Software decoders mostly tolerate a missing quiet zone, which is exactly why +/// nothing caught it: a round-trip test that only asks "does this decode?" is +/// satisfied by a symbol no hand scanner would read against a busy background. +/// So the geometry is asserted directly, per renderer, against this constant. +/// +/// The two renderers stay separate — they are split by output class, and +/// `lib.rs` records the intent to revisit that — but the property neither is +/// allowed to get wrong now has one home. +pub const QR_QUIET_ZONE_MODULES: u32 = 4; + /// Build the GS1 Digital Link URI a carrier (QR/Data Matrix) for this /// passport should encode. /// diff --git a/crates/dpp-render/src/lib.rs b/crates/dpp-render/src/lib.rs index 005f5cdf..02fe4698 100644 --- a/crates/dpp-render/src/lib.rs +++ b/crates/dpp-render/src/lib.rs @@ -25,5 +25,5 @@ mod page; mod remainder; mod sections; -pub use carrier::carrier_uri; +pub use carrier::{QR_QUIET_ZONE_MODULES, carrier_uri}; pub use page::{SnapshotNotice, build_qr_svg, render_page}; diff --git a/crates/dpp-render/src/page.rs b/crates/dpp-render/src/page.rs index 020ba8c7..a88298a5 100644 --- a/crates/dpp-render/src/page.rs +++ b/crates/dpp-render/src/page.rs @@ -175,7 +175,7 @@ pub fn build_qr_svg(carrier_uri: &str) -> String { let width = code.width(); let colors = code.to_colors(); let module_size = 4u32; - let quiet = 4u32; // quiet zone in modules + let quiet = crate::carrier::QR_QUIET_ZONE_MODULES; let total = (width as u32 + quiet * 2) * module_size; let mut rects = String::with_capacity(colors.len() * 48); @@ -337,13 +337,65 @@ mod tests { assert!(html.contains("<script>")); } + /// Renamed from `build_qr_svg_encodes_the_carrier_uri`, which claimed more + /// than it checked: the URI it found was the one in the ``, not + /// anything about the modules. What the symbol encodes is covered by the + /// resolver's PNG round-trip; both renderings come from the same + /// `QrCode::new`, so the encoding is exercised once rather than twice. #[test] - fn build_qr_svg_encodes_the_carrier_uri() { + fn build_qr_svg_names_the_carrier_uri_in_its_title() { let svg = build_qr_svg("https://id.odal-node.io/01/09506000134352/21/abc"); assert!(svg.starts_with("<svg")); assert!(svg.contains("id.odal-node.io")); } + /// The SVG reserves the four-module quiet zone on all sides. + /// + /// Read off the geometry: the `viewBox` must be wider than the symbol by + /// exactly two quiet zones, and no drawn module may fall inside the margin. + /// The PNG renderer had this wrong while this one had it right, which is + /// why the value is now a shared constant and why both sides assert it. + #[test] + fn build_qr_svg_reserves_the_quiet_zone() { + let uri = "https://id.odal-node.io/01/09506000134352/21/abc"; + let svg = build_qr_svg(uri); + let module_size = 4u32; + let quiet_px = crate::carrier::QR_QUIET_ZONE_MODULES * module_size; + + let total: u32 = svg + .split("viewBox=\"0 0 ") + .nth(1) + .and_then(|s| s.split(' ').next()) + .and_then(|s| s.parse().ok()) + .expect("the svg must declare a square viewBox"); + + let symbol_px = QrCode::new(uri.as_bytes()).unwrap().width() as u32 * module_size; + assert_eq!( + total, + symbol_px + quiet_px * 2, + "the viewBox must reserve a quiet zone on both sides" + ); + + // No module is drawn inside the margin. + for rect in svg.split("<rect x=\"").skip(1) { + let x: u32 = rect.split('"').next().unwrap().parse().unwrap(); + let y: u32 = rect + .split("y=\"") + .nth(1) + .and_then(|s| s.split('"').next()) + .and_then(|s| s.parse().ok()) + .expect("every module rect carries a y"); + assert!( + x >= quiet_px && y >= quiet_px, + "module at ({x},{y}) intrudes into the {quiet_px}px quiet zone" + ); + assert!( + x + module_size <= total - quiet_px && y + module_size <= total - quiet_px, + "module at ({x},{y}) crosses the far quiet-zone edge" + ); + } + } + #[test] fn build_qr_svg_escapes_the_title() { let svg = build_qr_svg("https://id.odal-node.io/\"><script>alert(1)</script>"); diff --git a/crates/dpp-resolver/Cargo.toml b/crates/dpp-resolver/Cargo.toml index 7a023625..65589d2a 100644 --- a/crates/dpp-resolver/Cargo.toml +++ b/crates/dpp-resolver/Cargo.toml @@ -54,3 +54,8 @@ metrics-exporter-prometheus = { workspace = true } tower = { version = "0.5", features = ["util"] } ed25519-dalek = { workspace = true } rand = { workspace = true } +# Test-only QR decoder. The round-trip it enables is the only thing that can +# tell "this endpoint returned a PNG" apart from "this endpoint returned a PNG +# encoding the right URL" — the distinction the previous tests could not make, +# and the one a `qrcode` or `image` bump can silently change. +rqrr = "0.10" diff --git a/crates/dpp-resolver/src/handlers/resolve_qr.rs b/crates/dpp-resolver/src/handlers/resolve_qr.rs index 44f1e2e2..594a8b55 100644 --- a/crates/dpp-resolver/src/handlers/resolve_qr.rs +++ b/crates/dpp-resolver/src/handlers/resolve_qr.rs @@ -105,10 +105,10 @@ pub async fn resolve_qr_handler( .into_response(); }; - let code = match QrCode::new(carrier_uri.as_bytes()) { - Ok(c) => c, + let png_bytes = match qr_png(&carrier_uri) { + Ok(bytes) => bytes, Err(e) => { - tracing::error!(dpp_id = %dpp_id, error = %e, "QR code generation failed"); + tracing::error!(dpp_id = %dpp_id, error = %e, "QR PNG generation failed"); return ( StatusCode::INTERNAL_SERVER_ERROR, [(header::CONTENT_TYPE, "image/png")], @@ -118,39 +118,6 @@ pub async fn resolve_qr_handler( } }; - // Build PNG from raw color matrix — avoids qrcode/image crate version conflict - let width = code.width() as u32; - let colors = code.to_colors(); - let scale: u32 = 8; - let img_size = width * scale; - let mut img = GrayImage::new(img_size, img_size); - for (i, color) in colors.iter().enumerate() { - let x = (i as u32) % width; - let y = (i as u32) / width; - let luma = if *color == qrcode::Color::Dark { - 0u8 - } else { - 255u8 - }; - for dy in 0..scale { - for dx in 0..scale { - img.put_pixel(x * scale + dx, y * scale + dy, Luma([luma])); - } - } - } - - let mut png_bytes: Vec<u8> = Vec::new(); - let dyn_img = DynamicImage::ImageLuma8(img); - if let Err(e) = dyn_img.write_to(&mut std::io::Cursor::new(&mut png_bytes), ImageFormat::Png) { - tracing::error!(dpp_id = %dpp_id, error = %e, "PNG encoding failed"); - return ( - StatusCode::INTERNAL_SERVER_ERROR, - [(header::CONTENT_TYPE, "image/png")], - Vec::new(), - ) - .into_response(); - } - // Cache as base64 string let b64 = base64::engine::general_purpose::STANDARD.encode(&png_bytes); state.cache.set(&cache_key, &b64).await; @@ -164,6 +131,58 @@ pub async fn resolve_qr_handler( .into_response() } +/// Pixels per QR module in the served PNG. +/// +/// Eight keeps a typical Digital Link symbol comfortably above the size a phone +/// camera resolves, without making the response large enough to be worth +/// tuning. +const MODULE_SCALE: u32 = 8; + +/// Render a carrier URI as PNG bytes. +/// +/// Extracted from the handler so the encoding can be tested for what it +/// *encodes* rather than only for having produced bytes. The handler's own path +/// reaches this only after fetching and verifying a passport, which a test +/// cannot exercise without a vault, a cache and a DID document — so for as long +/// as this lived inline, the only affordable assertion was `is_a_png`, and that +/// is what shipped. +/// +/// The quiet zone is [`dpp_render::QR_QUIET_ZONE_MODULES`], the same value the +/// SVG renderer uses. This function previously applied none: the image was +/// exactly `width * scale` pixels of symbol, which is out of spec under +/// ISO/IEC 18004 and is the rendering an operator prints onto a physical label. +fn qr_png(carrier_uri: &str) -> Result<Vec<u8>, String> { + let code = QrCode::new(carrier_uri.as_bytes()).map_err(|e| e.to_string())?; + + // Build the PNG from the raw colour matrix rather than `qrcode`'s own image + // renderer, which pins a different `image` major than this workspace. + let width = code.width() as u32; + let colors = code.to_colors(); + let quiet = dpp_render::QR_QUIET_ZONE_MODULES; + let img_size = (width + quiet * 2) * MODULE_SCALE; + + // Start from white so the quiet-zone border is blank without writing it. + let mut img = GrayImage::from_pixel(img_size, img_size, Luma([255u8])); + for (i, color) in colors.iter().enumerate() { + if *color != qrcode::Color::Dark { + continue; + } + let x = ((i as u32) % width + quiet) * MODULE_SCALE; + let y = ((i as u32) / width + quiet) * MODULE_SCALE; + for dy in 0..MODULE_SCALE { + for dx in 0..MODULE_SCALE { + img.put_pixel(x + dx, y + dy, Luma([0u8])); + } + } + } + + let mut png_bytes: Vec<u8> = Vec::new(); + DynamicImage::ImageLuma8(img) + .write_to(&mut std::io::Cursor::new(&mut png_bytes), ImageFormat::Png) + .map_err(|e| e.to_string())?; + Ok(png_bytes) +} + /// Record a QR-image render for telemetry, when telemetry is configured. This is /// label production, tracked separately from consumer scans and never summed /// into them. @@ -172,3 +191,87 @@ fn record_qr_render(state: &AppState, dpp_id: &str) { counter.record_qr_render(dpp_id); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// A GS1 Digital Link of the shape `carrier_uri` produces — the longest + /// form, with both AI 10 and AI 21, so the symbol is at the larger end of + /// what this route emits. + const CARRIER: &str = + "https://id.odal-node.io/01/09506000134352/10/BATCH-42/21/7abc8def0123456789ab"; + + /// Decode a PNG back to the single QR payload it carries. + fn decode(png: &[u8]) -> String { + let img = image::load_from_memory(png).expect("handler must emit a decodable PNG"); + let mut prepared = rqrr::PreparedImage::prepare(img.to_luma8()); + let grids = prepared.detect_grids(); + assert_eq!(grids.len(), 1, "exactly one QR symbol must be detectable"); + let (_meta, content) = grids[0].decode().expect("the symbol must decode"); + content + } + + /// The whole point: the image encodes the carrier URI, not merely *a* URI + /// and not merely some bytes. + /// + /// Everything before this asserted `image/png` and a non-empty body, which + /// a solid white square satisfies. A `qrcode` or `image` bump could have + /// changed what the label says and stayed green. + #[test] + fn the_png_encodes_the_carrier_uri_it_was_given() { + let png = qr_png(CARRIER).expect("a valid URI must render"); + assert_eq!(decode(&png), CARRIER); + } + + /// The symbol carries the four-module quiet zone ISO/IEC 18004 requires. + /// + /// Asserted geometrically rather than by decoding, and that is deliberate: + /// `rqrr` reads a symbol with no quiet zone perfectly well, so a round-trip + /// test alone is satisfied by an image no hand scanner would read against a + /// busy background. This is the assertion that would have caught the + /// zero-margin rendering this route used to serve. + /// + /// Checked as a blank border of the expected thickness on all four sides. + #[test] + fn the_symbol_keeps_its_quiet_zone_on_every_side() { + let png = qr_png(CARRIER).expect("a valid URI must render"); + let img = image::load_from_memory(&png).unwrap().to_luma8(); + let margin = dpp_render::QR_QUIET_ZONE_MODULES * MODULE_SCALE; + let (w, h) = img.dimensions(); + assert_eq!(w, h, "a QR symbol is square"); + + for y in 0..h { + for x in 0..w { + let in_border = x < margin || y < margin || x >= w - margin || y >= h - margin; + if in_border { + assert_eq!( + img.get_pixel(x, y)[0], + 255, + "({x},{y}) lies in the {margin}px quiet zone and must be blank" + ); + } + } + } + } + + /// The border is a margin around a symbol, not the whole image — a blank + /// PNG would satisfy the test above on its own. + #[test] + fn the_quiet_zone_check_cannot_pass_on_a_blank_image() { + let png = qr_png(CARRIER).expect("a valid URI must render"); + let img = image::load_from_memory(&png).unwrap().to_luma8(); + assert!( + img.pixels().any(|p| p[0] == 0), + "the image must contain dark modules" + ); + } + + /// A URI too long for any QR version fails as an error rather than + /// producing an image that encodes something other than what was asked. + #[test] + fn an_unencodable_uri_is_an_error_not_a_wrong_symbol() { + let too_long = "https://id.odal-node.io/01/".to_owned() + &"9".repeat(8000); + assert!(qr_png(&too_long).is_err()); + } +}