diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..711e095 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,151 @@ +# Working on smart-package-tracker + +Orientation for developers and AI agents. Read this before changing code. +`CLAUDE.md` points here; this file is the single source of truth. + +## What this crate is + +A `no_std` + `alloc` library that mints package tracking IDs (`PKG-9ED9285C`) +and renders them as Code 128 or QR barcodes, to PNG or SVG. No network I/O, no +async, no carrier integrations — those are deliberately out of scope (see +"Boundaries"). + +## The one diagram that explains the layout + +```text +TrackingId ──▶ Symbology::encode ──▶ Symbol ──▶ Renderer::render ──▶ bytes + src/id/ src/symbology/ (BitMatrix) src/render/ +``` + +The `BitMatrix` in the middle is the seam. Encoders never know about pixels; +renderers never know about symbologies. A linear barcode is a 1-row matrix, a +QR code is a square one — that is the *only* difference the renderers see. + +**If a change makes a renderer match on `SymbologyKind`, the seam has been +broken. Push the difference into `Symbol`/`SymbologyKind` metadata instead.** + +## Repo map + +| Path | What lives there | +|------|------------------| +| `src/lib.rs` | Crate docs, feature table, re-exports. Public API surface starts here. | +| `src/error.rs` | The single `Error` enum. `#[non_exhaustive]` — add variants, never repurpose them. | +| `src/id/mod.rs` | `TrackingId` newtype + parsing/validation. `MAX_LEN`, the charset rule. | +| `src/id/generator.rs` | `IdGenerator`/builder: prefix, entropy width, checksum policy. | +| `src/id/checksum.rs` | ISO/IEC 7064 MOD 37,36 check character. | +| `src/symbology/mod.rs` | `BitMatrix`, `Symbol`, the `Symbology` trait, `SymbologyKind` metadata. | +| `src/symbology/code128.rs` | Code 128 encode + decode, wrapping the `code128` crate. | +| `src/symbology/qr.rs` | QR encode, wrapping the `qrcode` crate. | +| `src/render/mod.rs` | `RenderOptions`, `Length`, `Color`, `QuietZone`, and **`Layout`** — the shared geometry calculation. | +| `src/render/png.rs` | Rasterises `Layout` into RGBA, wraps it in a PNG with a `pHYs` chunk. | +| `src/render/svg.rs` | Emits the same `Layout` as `` runs. | +| `src/render/hri.rs` | Embedded 5x7 bitmap font for the human-readable text line. | +| `tests/integration.rs` | Consumer-level end-to-end tests (Code 128). | +| `tests/qr.rs` | QR output decoded back with `rqrr`, an independent decoder. | +| `examples/label.rs` | Runnable end-to-end demo: `cargo run --example label`. | + +## Invariants — do not break these silently + +1. **Quiet zones are conformant by default.** Code 128 needs 10 modules per + side, QR needs 4 on all four sides (`SymbologyKind::required_quiet_zone`). + Nothing — including the human-readable text line — may be drawn inside them. + This is the number one cause of "looks fine, will not scan". +2. **Modules snap to whole pixels.** A bar straddling a pixel boundary renders + as a grey edge and costs the contrast a scanner needs. `Layout` is integer + pixels throughout. +3. **PNG and SVG describe identical geometry.** Both consume the same `Layout`. + Any geometry change belongs in `Layout::compute`, never in one renderer. +4. **Fallible operations return `Result`, they do not panic.** The README + promises this. Accessors tolerate out-of-range input rather than panicking, + and the handful of constructors that do assert say so in a `# Panics` + section. Keep it that way. +5. **Output is byte-for-byte reproducible.** No timestamps, no hash-map + iteration order, no system fonts. That is why the HRI font is embedded. +6. **`no_std` + `alloc` core.** Only `std`, `png`, and `qr` may assume `std`. + Use `alloc::` imports, not `std::`, outside `#[cfg(feature = "std")]`. +7. **`#![forbid(unsafe_code)]`.** Non-negotiable. + +## Commands + +```sh +cargo test --all-features # everything, incl. doctests +cargo clippy --all-features --all-targets -- -D warnings +cargo fmt --all --check +cargo run --example label # writes label*.png / label*.svg + +# no_std still builds +cargo build --target thumbv7em-none-eabihf --no-default-features --features code128,svg + +# feature combinations compile in isolation (CI covers a subset) +for f in "" std serde os-rng code128 svg qr png; do + cargo check --no-default-features --features "$f" --quiet || echo "FAIL [$f]" +done +``` + +MSRV is **1.85** (set by `getrandom` 0.4 / edition 2024) and is tested in CI. +Raising it is a breaking change. + +## How to add things + +* **A new symbology** — implement `Symbology` in `src/symbology/`, add a + `SymbologyKind` variant with its `required_quiet_zone()` and `is_linear()`, + gate it behind a feature. Touch no rendering code. QR was added this way. +* **A new renderer** — implement `Renderer` in `src/render/`, consume `Layout`, + compute no geometry of your own. +* **A new error case** — add a variant to `src/error.rs` with a lowercase, + actionable `#[error(...)]` message. The enum is `#[non_exhaustive]`, so this + is not a breaking change. +* **A new public item** — it needs a doc comment (`missing_docs` is warned on), + an `# Errors` section if fallible, and a `# Panics` section if it can panic. + +## Test conventions + +* Unit tests live in a `#[cfg(test)] mod tests` at the bottom of their file and + gate on features (`#[cfg(all(test, feature = "code128"))]`). +* Integration tests assert on *decoded output*, not on internals — PNG bytes are + decoded back to pixels, QR output is decoded by `rqrr`. +* Prefer a test that would fail for the real-world reason (a scanner cannot read + this) over one that asserts an implementation detail. Note that a decoder + handed a perfectly cropped image is far more forgiving than a scanner pointed + at a parcel — see the comment in `tests/qr.rs`. + +## Boundaries + +Out of scope on purpose: image scanning, shipment events, carrier APIs, GS1 +identifier issuance. They bring network I/O, async runtimes, and vendor licence +terms into a dependency graph that is currently 16 permissively-licensed crates +with no copyleft (`cargo deny check` enforces this). + +## Release — irreversible + +Pushing a `v*.*.*` tag runs `.github/workflows/release.yml`, which publishes to +crates.io behind a `crates-io` environment approval. **A published version can +be yanked but never removed or replaced.** Do not create version tags unless +publishing is the explicit intent. `workflow_dispatch` with `dry_run: true` +verifies and packages without publishing. + +## Where the bugs have actually come from + +Every defect found in this crate so far has been one of four shapes. They are +worth checking for by name in any change: + +1. **A rule applied to one symbology but not the other.** The human-readable + text line was placed a fixed one module below the symbol — correct for + Code 128, which has no vertical quiet zone, and a quiet-zone violation for + QR, which needs four clear modules underneath. +2. **Two limits chosen independently that have to agree.** `MAX_ENTROPY_BITS` + allowed a body longer than `MAX_LEN` permitted, so the generator could mint + IDs its own parser refused. Tie limits like these together with a + `const _: () = assert!(...)` so they cannot drift. +3. **A validated type reachable through a back door.** Deriving + `Deserialize` on `TrackingId` let any string in, bypassing `parse` + completely. Anything that constructs a validated type must go through its + validator. +4. **A guard that does not guard what its comment claims.** The image size + limit was per-axis while the allocation is the product of both. + +And one about tests: a test that passes because a decoder is lenient proves +nothing about a scanner. Assert on the rendered pixels. + +Fixed instances of all four are described in +[CHANGELOG.md](CHANGELOG.md#unreleased). diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b6c547..bc580a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,66 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Human-readable text no longer intrudes into a QR code's quiet zone.** The + HRI line was placed one module below the symbol, which is right for Code 128 + — a linear symbology has no vertical quiet zone — and wrong for QR, which + requires four clear modules on every side. At default settings 708 pixels of + text ink landed inside the band a scanner needs to find the symbol. Text is + now placed below the quiet zone. Linear output is unchanged. +- **`IdGenerator` can no longer mint IDs that `TrackingId::parse` rejects.** + `MAX_ENTROPY_BITS` (512, so a 128-character body) and the format's overall + length limit (128) had been chosen independently, so a 512-bit policy + produced a 132-character ID that failed to parse on the way back out of + storage. The length limit is now 160, derived from the widest policy the + generator accepts, and a static assertion fails the build if the two ever + drift apart again. IDs that parsed before still parse. +- **`serde` deserialisation validates.** `TrackingId` derived `Deserialize` + with `#[serde(transparent)]`, so any string at all — `""`, + `"pkg-lowercase"`, `"total garbage!!"` — became a `TrackingId` without going + through `parse`. Worse, `"no-separator-here"` was accepted and then reported + `prefix() == "no"` and `body() == "separator"`, silently dropping the rest. + Deserialisation now runs the same validation as `parse` and fails with the + same message. Serialisation is unchanged. +- **`BitMatrix::row` no longer panics on an out-of-range row.** It returns an + empty slice, matching `get` and `set`, which already tolerated out-of-range + coordinates. `BitMatrix::from_row` now documents the panic it has always had. +- **The image size limit bounds the allocation, not just each axis.** Two + dimensions individually under the 20,000 px cap multiplied out to 400 + megapixels — a 1.6 GiB single allocation in the PNG rasteriser, which is + exactly what the limit exists to prevent. Geometry over 64 megapixels is now + rejected with an actionable message. +- `IdGenerator::validate` now checks that a body's entropy characters are + uppercase hexadecimal. `PKG-ZZZZZZZZ` used to pass under the default policy + despite the generator only ever emitting `0-9A-F`. The check character is + still drawn from the wider 36-character alphabet and verified separately. +- A prefix containing multi-byte characters is now reported as a charset + error rather than as being too long — the length check counts bytes, and ran + first. +- The Code 128 bar blitter clamps both ends of its slice. An upstream bar + reported at or past the trailing quiet zone would have sliced backwards and + panicked; no released version of `code128` emits one, but a version bump no + longer risks turning that into a crash. + +### Added + +- `AGENTS.md`, an orientation guide for contributors and coding agents: the + repo map, the invariants a change must not break, where to add a new + symbology or renderer, and the shapes the bugs above kept taking. `CLAUDE.md` + points at it. +- README section mapping every `Error` variant to its cause and its fix. +- Tests covering each fix above, including a QR quiet-zone assertion that + renders with human-readable text enabled and inspects the pixels around the + symbol, rather than relying on a decoder that is far more forgiving than a + scanner. + +### Changed + +- Editor and agent tooling files (`.claude/`, `.entire/`, `CLAUDE.md`, + `hello-entire.md`) are excluded from the published crate. Publishing is + irreversible, so the tarball should carry only what a consumer needs. + ## [0.2.0] - 2026-08-22 ### Added diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/Cargo.toml b/Cargo.toml index df63500..71a5469 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,10 @@ documentation = "https://docs.rs/smart-package-tracker" readme = "README.md" keywords = ["barcode", "code128", "tracking", "shipping", "logistics"] categories = ["encoding", "multimedia::images", "no-std"] -exclude = ["/.github", "/target"] +# Editor, agent and tooling files mean nothing to a consumer of the crate, +# and publishing is irreversible. AGENTS.md and CHANGELOG.md stay in: they +# are about the crate rather than about one tool. +exclude = ["/.github", "/target", "/.claude", "/.entire", "/CLAUDE.md", "/hello-entire.md"] [dependencies] code128 = { version = "0.2.1", default-features = false, optional = true } diff --git a/README.md b/README.md index 78cbf7a..643ea8a 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,8 @@ the PNG prints. * **Set C compression.** Digit runs encode two per symbol, which roughly halves the width of numeric carrier tracking numbers. * **Reproducible output.** Identical inputs produce byte-identical files. -* **No panics on bad input.** Every fallible operation returns `Result`. +* **No panics on bad input.** Every fallible operation returns `Result`, and + out-of-range accessors return empty rather than panicking. ## QR codes @@ -110,6 +111,35 @@ generator.validate(&id)?; Randomness alone never guarantees uniqueness. Keep a unique constraint at the storage layer and retry on conflict. +## When something goes wrong + +Every fallible call returns `Result<_, Error>`. `Error` is `#[non_exhaustive]`, +so match with a `_` arm. What each variant means in practice: + +| Variant | Typical cause | What to do | +|---------|---------------|------------| +| `InvalidTrackingId` | A string from storage or user input is not `PREFIX-BODY`, or is not `A-Z`/`0-9` | The message names the first violation; IDs are uppercase-only by design | +| `IdPolicyMismatch` | The ID is well-formed but was not minted by this `IdGenerator` — wrong prefix, wrong body length, or a failed check character | Compare against the generator that issued it; a failed check character usually means a misread or typo | +| `InvalidIdConfig` | `IdGenerator::builder()` was given a bad prefix or entropy width | Prefix: 1–16 chars of `A-Z0-9`. Entropy: a multiple of 4, between 16 and 512 | +| `Entropy` | The OS randomness source is unavailable | The crate never falls back to a weaker source. Use `generate_from_entropy` with your own bytes | +| `InsufficientEntropy` | `generate_from_entropy` was handed too few bytes | Supply at least `generator.entropy_bytes()` | +| `Unencodable` | Code 128: the payload is outside Latin-1. QR: it does not fit the pinned version, or the version is not 1–40 | Use QR for non-Latin-1 payloads; raise the version or lower the ECC level | +| `EmptyPayload` | An empty string was passed to an encoder | Barcodes cannot encode nothing | +| `InvalidRenderOptions` | Non-positive DPI/width/height, or geometry over 20,000 px on an axis | The message names the computed size; reduce `module_width`, `height`, or `dpi` | +| `Render` | The PNG encoder failed | Rare; the message carries the underlying error | +| `Decode` | `Barcode::decode()` was called on a non-Code 128 symbol, or on an invalid pattern | There is no QR decoder in this crate | +| `Io` | A `*_to_file` helper could not write | Standard `std::io::Error`; only present with the `std` feature | + +Two failure modes deserve naming because they produce a file that *looks* +correct and does not scan: + +* **Quiet zones.** `QuietZone::None` and small `QuietZone::Modules(n)` values + are honoured exactly as asked. `QuietZone::Standard` is the only setting that + guarantees conformance. +* **Human-readable text.** The embedded font covers `0-9`, `A-Z`, space, and + `- . / * + $ % :`. Anything else prints as a hollow box. Check with + `hri_supports(text)` before it reaches a label, or disable HRI. + ## Feature flags | Feature | Default | Effect | @@ -185,10 +215,16 @@ See [CHANGELOG.md](CHANGELOG.md). ```sh cargo test --all-features -cargo clippy --all-features -- -D warnings -cargo run --example label +cargo clippy --all-features --all-targets -- -D warnings +cargo fmt --all --check +cargo run --example label # writes label*.png and label*.svg ``` +[AGENTS.md](AGENTS.md) is the orientation guide for contributors and coding +agents: the repo map, the invariants a change must not break, where to add a new +symbology or renderer, and the release procedure. `CLAUDE.md` points at it. + + ## License Licensed under either of [Apache License 2.0](LICENSE-APACHE) or diff --git a/src/id/generator.rs b/src/id/generator.rs index 6b75b0d..5c381c0 100644 --- a/src/id/generator.rs +++ b/src/id/generator.rs @@ -14,6 +14,20 @@ const MIN_ENTROPY_BITS: u16 = 16; /// Above this the barcode gets impractically wide for no benefit. const MAX_ENTROPY_BITS: u16 = 512; +/// A generator must never mint an ID that [`TrackingId::parse`] would reject. +/// The widest policy is the longest prefix, the separator, every entropy +/// character and a check character; if that ever outgrows the format's length +/// limit, this fails the build rather than the round trip. +const _: () = { + let separator = 1; + let check_character = 1; + let widest = MAX_PREFIX_LEN + separator + (MAX_ENTROPY_BITS as usize) / 4 + check_character; + assert!( + widest <= super::MAX_LEN, + "the widest IdGenerator policy must still parse as a TrackingId" + ); +}; + /// Which check character, if any, to append to generated IDs. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -190,9 +204,10 @@ impl IdGenerator { /// Check that `id` was produced by this generator's policy. /// - /// Verifies the prefix, the body length, and the check character. Note - /// that this cannot prove provenance — it only rules out IDs that this - /// policy could never have produced. + /// Verifies the prefix, the body length, that the entropy characters are + /// uppercase hexadecimal, and the check character. Note that this cannot + /// prove provenance — it only rules out IDs that this policy could never + /// have produced. /// /// # Errors /// @@ -219,6 +234,18 @@ impl IdGenerator { }); } + // The generator only ever emits `0-9A-F` here, so anything else could + // not have come from this policy. The check character is drawn from the + // wider 36-character alphabet and is verified separately below. + if let Some(bad) = body[..self.entropy_chars()] + .chars() + .find(|c| !c.is_ascii_digit() && !matches!(c, 'A'..='F')) + { + return Err(Error::IdPolicyMismatch { + reason: alloc::format!("body must be uppercase hexadecimal, found `{bad}`"), + }); + } + if self.checksum == Checksum::Iso7064Mod37_36 && !checksum::verify(body) { return Err(Error::IdPolicyMismatch { reason: "check character does not match the body".to_string(), @@ -285,15 +312,18 @@ impl IdGeneratorBuilder { "prefix must not be empty".to_string(), )); } - if self.prefix.len() > MAX_PREFIX_LEN { + // Charset first: `len()` counts bytes, so a multi-byte prefix would + // otherwise be reported as too long rather than as containing a + // character that is not allowed at all. + if let Some(bad) = self.prefix.chars().find(|c| !super::is_body_char(*c)) { return Err(Error::InvalidIdConfig(alloc::format!( - "prefix must be at most {MAX_PREFIX_LEN} characters, got {}", - self.prefix.len() + "prefix must consist of `A-Z` and `0-9`, found `{bad}`" ))); } - if let Some(bad) = self.prefix.chars().find(|c| !super::is_body_char(*c)) { + if self.prefix.len() > MAX_PREFIX_LEN { return Err(Error::InvalidIdConfig(alloc::format!( - "prefix must consist of `A-Z` and `0-9`, found `{bad}`" + "prefix must be at most {MAX_PREFIX_LEN} characters, got {}", + self.prefix.len() ))); } if self.entropy_bits % 4 != 0 { @@ -367,6 +397,52 @@ mod tests { assert!(g.validate(&corrupted).is_err()); } + #[test] + fn validate_rejects_a_body_the_policy_could_not_have_produced() { + // Right prefix, right length, but the generator only ever emits + // `0-9A-F` — a body of `Z`s can only be corruption or a forgery. + let g = IdGenerator::default(); + let err = g + .validate(&TrackingId::parse("PKG-ZZZZZZZZ").unwrap()) + .unwrap_err(); + assert!(matches!(err, Error::IdPolicyMismatch { .. }), "got {err:?}"); + + g.validate(&TrackingId::parse("PKG-9ED9285C").unwrap()) + .expect("hexadecimal bodies are still accepted"); + } + + #[test] + fn validate_still_accepts_a_non_hex_check_character() { + // The check character comes from the 36-character alphabet, so it may + // fall outside `0-9A-F` while the entropy before it does not. + let g = IdGenerator::builder() + .checksum(Checksum::Iso7064Mod37_36) + .build() + .unwrap(); + for seed in 0u8..32 { + let id = g.generate_from_entropy(&[seed; 4]).unwrap(); + g.validate(&id) + .unwrap_or_else(|e| panic!("{id} should validate: {e}")); + } + } + + #[test] + fn a_multibyte_prefix_is_reported_as_a_charset_error() { + // Six characters, but 18 bytes: the length check counts bytes, so + // running it first would report "at most 16 characters, got 18" for a + // prefix whose real problem is that none of it is allowed. + let prefix = "\u{4e2d}\u{6587}\u{4e2d}\u{6587}\u{4e2d}\u{6587}"; + assert_eq!(prefix.chars().count(), 6); + assert_eq!(prefix.len(), 18); + + let err = IdGenerator::builder().prefix(prefix).build().unwrap_err(); + let message = alloc::format!("{err}"); + assert!( + message.contains("`A-Z` and `0-9`"), + "expected a charset error, got: {message}" + ); + } + #[test] fn validate_rejects_a_foreign_prefix() { let g = IdGenerator::default(); @@ -409,6 +485,40 @@ mod tests { assert!(IdGenerator::builder().entropy_bits(1024).build().is_err()); } + #[test] + fn the_widest_policy_still_parses_as_a_tracking_id() { + // The generator's limits and the format's length limit were chosen + // independently; at 512 bits the ID used to come out longer than + // `TrackingId::parse` would accept, so an ID could be minted and then + // rejected on the way back out of storage. + let g = IdGenerator::builder() + .prefix("ABCDEFGHIJKLMNOP") // MAX_PREFIX_LEN + .entropy_bits(MAX_ENTROPY_BITS) + .checksum(Checksum::Iso7064Mod37_36) + .build() + .expect("the widest policy must be buildable"); + + let id = g.generate_from_entropy(&[0xab; 64]).unwrap(); + assert_eq!(id.body().len(), 129); // 128 hex characters + check character + + let round_tripped = TrackingId::parse(id.as_str()) + .unwrap_or_else(|e| panic!("the widest policy must round trip: {e}")); + assert_eq!(id, round_tripped); + g.validate(&round_tripped).unwrap(); + } + + #[test] + #[cfg(feature = "os-rng")] + fn every_supported_entropy_width_round_trips() { + for bits in [MIN_ENTROPY_BITS, 32, 64, 128, 256, 480, MAX_ENTROPY_BITS] { + let g = IdGenerator::builder().entropy_bits(bits).build().unwrap(); + let id = g.generate().unwrap(); + let parsed = TrackingId::parse(id.as_str()) + .unwrap_or_else(|e| panic!("{bits} bits produced an unparseable id: {e}")); + assert_eq!(id, parsed); + } + } + #[test] #[cfg(feature = "os-rng")] fn os_entropy_produces_distinct_well_formed_ids() { diff --git a/src/id/mod.rs b/src/id/mod.rs index d7bdb57..f7587f3 100644 --- a/src/id/mod.rs +++ b/src/id/mod.rs @@ -18,7 +18,13 @@ use crate::error::{Error, Result}; /// Character separating the prefix from the body. const SEPARATOR: char = '-'; /// Upper bound on a whole ID, to keep parsing and barcode widths bounded. -const MAX_LEN: usize = 128; +/// +/// It has to cover the widest policy [`IdGenerator`] can be built with — a +/// 16-character prefix, the separator, 128 hexadecimal characters (512 bits of +/// entropy) and a check character, which comes to 146 — or the generator could +/// mint IDs that [`TrackingId::parse`] refuses to read back. A static assertion +/// in `generator` holds the two limits together. +pub(crate) const MAX_LEN: usize = 160; /// Characters permitted in a prefix or body. /// @@ -35,6 +41,11 @@ fn is_body_char(c: char) -> bool { /// [`IdGenerator::generate`] (custom policy), or [`TrackingId::parse`] when /// reading an ID back from storage or user input. /// +/// With the `serde` feature an ID serialises as a plain string, and +/// deserialising runs the same validation as [`TrackingId::parse`] — a +/// `TrackingId` that arrived over the wire is as trustworthy as one built by +/// hand. +/// /// # Examples /// /// ``` @@ -54,10 +65,27 @@ fn is_body_char(c: char) -> bool { /// # fn main() {} /// ``` #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(transparent))] pub struct TrackingId(pub(crate) String); +#[cfg(feature = "serde")] +impl<'de> serde::Deserialize<'de> for TrackingId { + /// Deserialise and validate. + /// + /// Deriving this would accept any string at all, which would let a value + /// that [`TrackingId::parse`] rejects — a lowercase ID, or one carrying a + /// second separator whose tail is then silently ignored by + /// [`body`](TrackingId::body) — enter the type through the back door. + fn deserialize(deserializer: D) -> core::result::Result + where + D: serde::Deserializer<'de>, + { + let raw = ::deserialize(deserializer)?; + Self::parse(&raw).map_err(serde::de::Error::custom) + } +} + impl TrackingId { /// Generate an ID with the default policy: `PKG-` plus 32 bits of entropy. /// @@ -237,3 +265,42 @@ mod tests { assert_eq!(b, c); } } + +#[cfg(all(test, feature = "serde"))] +mod serde_tests { + use super::*; + use serde::de::value::{Error as ValueError, StrDeserializer}; + use serde::de::IntoDeserializer; + use serde::Deserialize; + + fn from_str(raw: &str) -> Result { + let de: StrDeserializer<'_, ValueError> = raw.into_deserializer(); + TrackingId::deserialize(de).map_err(|e| Error::InvalidTrackingId { + reason: alloc::string::ToString::to_string(&e), + }) + } + + #[test] + fn deserialising_validates_like_parse() { + for raw in [ + "", // empty + "PKG9ED9285C", // no separator + "pkg-lowercase", // lowercase + "no-separator-here", // two separators: `body()` would drop the tail + "total garbage!!", // illegal characters + ] { + assert!( + from_str(raw).is_err(), + "`{raw}` must not deserialise into a TrackingId" + ); + } + } + + #[test] + fn well_formed_ids_still_deserialise() { + let id = from_str("PKG-9ED9285C").unwrap(); + assert_eq!(id, TrackingId::parse("PKG-9ED9285C").unwrap()); + assert_eq!(id.prefix(), "PKG"); + assert_eq!(id.body(), "9ED9285C"); + } +} diff --git a/src/render/mod.rs b/src/render/mod.rs index 0d26fca..c9ee317 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -27,6 +27,15 @@ use crate::symbology::Symbol; /// mistake, and failing loudly beats attempting a multi-gigabyte allocation. const MAX_DIMENSION_PX: u32 = 20_000; +/// Refuse to allocate images with more pixels than this in total. +/// +/// The per-axis limit alone does not bound memory: two dimensions each within +/// it multiply out to 400 megapixels, which the PNG renderer would rasterise +/// into a 1.6 GiB RGBA buffer in a single allocation. 64 megapixels is +/// 256 MiB — still far beyond any real label, which at 600 dpi on a 4x6 inch +/// stock is under 9 megapixels. +const MAX_PIXELS: u64 = 64_000_000; + /// A physical or device length. /// /// Barcode geometry is specified in physical units — the X-dimension of a @@ -243,7 +252,7 @@ impl RenderOptions { /// # Errors /// /// Returns [`Error::InvalidRenderOptions`] if the result would exceed - /// 20,000 pixels on either axis. + /// 20,000 pixels on either axis, or 64 megapixels in total. pub fn layout(&self, symbol: &Symbol) -> Result { Layout::compute(symbol, self) } @@ -413,9 +422,17 @@ impl Layout { // Centre the symbol and the text independently within the content box. let symbol_x_px = quiet_x_px + (content_w_px - symbol_w_px) / 2; let (hri_x_px, hri_y_px) = if draw_hri { + // The text goes *below* the symbol's quiet zone, never inside it. + // A linear symbology has no vertical quiet zone, so this is the + // familiar one-module gap; a matrix symbology such as QR requires + // four clear modules underneath, and text drawn into them costs + // exactly the margin a scanner uses to find the symbol. ( quiet_x_px + (content_w_px - text_w_px) / 2, - quiet_y_px + symbol_h_px + module_px, + quiet_y_px + .saturating_mul(2) + .saturating_add(symbol_h_px) + .saturating_add(module_px), ) } else { (0, 0) @@ -432,6 +449,13 @@ impl Layout { reduce module_width, height, or dpi" ))); } + if u64::from(width_px) * u64::from(height_px) > MAX_PIXELS { + return Err(Error::InvalidRenderOptions(alloc::format!( + "computed image is {width_px}x{height_px} px, over the {} megapixel limit; \ + reduce module_width, height, or dpi", + MAX_PIXELS / 1_000_000 + ))); + } Ok(Self { module_px, @@ -651,6 +675,39 @@ mod tests { )); } + #[test] + #[cfg(feature = "qr")] + fn a_huge_but_within_axis_limits_image_is_still_rejected() { + // Both axes under MAX_DIMENSION_PX multiply out to an allocation the + // per-axis check never sees: this geometry used to be accepted and + // then rasterised into a 1.27 GiB buffer. + use crate::symbology::{Qr, QrVersion}; + + let big = Qr::new() + .version(QrVersion::Fixed(40)) + .encode("PKG-9ED9285C") + .unwrap(); + let opts = RenderOptions::builder() + .module_width(Length::Px(100.0)) + .human_readable(false) + .build() + .unwrap(); + + let err = opts.layout(&big).unwrap_err(); + assert!(matches!(err, Error::InvalidRenderOptions(_)), "got {err:?}"); + assert!( + alloc::format!("{err}").contains("megapixel"), + "the message should name the limit that was hit: {err}" + ); + + // A label-sized geometry is still comfortably allowed. + let sane = RenderOptions::builder() + .module_width(Length::Px(8.0)) + .build() + .unwrap(); + assert!(sane.layout(&big).is_ok()); + } + #[test] fn colors_format_as_css_hex() { assert_eq!(Color::BLACK.to_hex(), "#000000"); diff --git a/src/symbology/code128.rs b/src/symbology/code128.rs index 8b47a4d..f5b5fa9 100644 --- a/src/symbology/code128.rs +++ b/src/symbology/code128.rs @@ -64,6 +64,13 @@ impl Symbology for Code128 { let mut row = vec![false; width as usize]; for bar in code.bar_coordinates() { let start = bar.x.saturating_sub(UPSTREAM_QUIET_ZONE) as usize; + // Clamp both ends, not just the far one: a bar reported at or past + // the trailing quiet zone would otherwise slice backwards and + // panic. Upstream does not emit one today; this keeps a version + // bump from turning that into a crash. + if start >= row.len() { + continue; + } let end = (start + bar.width as usize).min(row.len()); row[start..end].fill(true); } diff --git a/src/symbology/mod.rs b/src/symbology/mod.rs index b14839c..64a36a4 100644 --- a/src/symbology/mod.rs +++ b/src/symbology/mod.rs @@ -110,6 +110,11 @@ impl BitMatrix { } /// Build a single-row matrix from a run of modules. + /// + /// # Panics + /// + /// Panics if `row` is empty, which no symbology should ever produce. Use + /// [`from_vec`](Self::from_vec) to get an [`Option`] instead. pub fn from_row(row: Vec) -> Self { assert!(!row.is_empty(), "a symbol must have a positive size"); Self { @@ -164,8 +169,13 @@ impl BitMatrix { self.bits[(y as usize) * w + (x as usize)] = dark; } - /// One row of modules. + /// One row of modules. An out-of-bounds row reads as empty, matching the + /// way [`get`](Self::get) and [`set`](Self::set) tolerate out-of-bounds + /// coordinates. pub fn row(&self, y: u32) -> &[bool] { + if y >= self.height { + return &[]; + } let w = self.width as usize; let start = (y as usize) * w; &self.bits[start..start + w] @@ -268,6 +278,18 @@ mod tests { assert!(!m.get(9, 9)); } + #[test] + fn matrix_reads_out_of_bounds_rows_as_empty() { + // `get` and `set` tolerate out-of-range coordinates, so `row` must too + // rather than panicking on a slice range — this is public API, and the + // crate promises not to panic on bad input. + let m = BitMatrix::new(3, 2); + assert_eq!(m.row(0).len(), 3); + assert_eq!(m.row(1).len(), 3); + assert!(m.row(2).is_empty()); + assert!(m.row(u32::MAX).is_empty()); + } + #[test] fn debug_renders_ascii_art() { let m = BitMatrix::from_row(vec![true, false, true]); diff --git a/tests/qr.rs b/tests/qr.rs index 74cf01e..58097e4 100644 --- a/tests/qr.rs +++ b/tests/qr.rs @@ -206,3 +206,53 @@ fn long_payload_text_widens_the_image_instead_of_overflowing() { layout.width_px ); } + +#[test] +fn human_readable_text_stays_out_of_the_quiet_zone() { + // ISO/IEC 18004 wants four clear modules on every side of the symbol. The + // human-readable line is drawn below the symbol, and it must clear that + // band rather than eat into it: the test above proves a cropped image + // decodes, which says nothing about a scanner finding the symbol on a + // parcel. Assert on the pixels around the symbol, with HRI enabled. + let barcode = Barcode::qr("PKG-9ED9285C").unwrap(); + let options = RenderOptions::builder() + .module_width(Length::Px(8.0)) + .human_readable(true) + .build() + .unwrap(); + let layout = options.layout(barcode.symbol()).unwrap(); + let (w, h, luma) = render_greyscale(&barcode, &options); + + let quiet = layout.quiet_x_px; + assert_eq!(quiet, 4 * layout.module_px); + + // The band around the symbol box, out to the required quiet zone. + let left = layout.symbol_x_px - quiet; + let top = layout.symbol_y_px - quiet; + let right = layout.symbol_x_px + layout.symbol_w_px + quiet; + let bottom = layout.symbol_y_px + layout.symbol_h_px + quiet; + assert!( + right as usize <= w && bottom as usize <= h, + "quiet zone is clipped by the image" + ); + + for y in top..bottom { + for x in left..right { + let inside_symbol = x >= layout.symbol_x_px + && x < layout.symbol_x_px + layout.symbol_w_px + && y >= layout.symbol_y_px + && y < layout.symbol_y_px + layout.symbol_h_px; + if !inside_symbol { + assert_eq!( + luma[(y as usize) * w + (x as usize)], + 255, + "quiet zone pixel at ({x}, {y}) is not blank" + ); + } + } + } + + // And the text is still there, below the quiet zone. + let text_rows = (bottom as usize..h).any(|y| (0..w).any(|x| luma[y * w + x] < 128)); + assert!(text_rows, "no human-readable text was drawn"); +}