mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-13 23:17:40 +00:00
main
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Tanish MankuandClaude
|
0a09864833 |
F1, F2: adapt to main and re-pin the Arabic layout golden
Rebase onto
|
||
|
Tanish MankuandClaude
|
ce9a1d1014 |
F2: read glyph names by the naming algorithm, and stop reading Symbol as Latin
Two gaps in code-to-Unicode, both found by probing the current behaviour rather than by reading the code. A font with no /Widths and no /ToUnicode fell back to StandardEncoding whatever it was, so /Symbol code 0x61 -- `alpha` in Symbol's own built-in encoding -- decoded as "a". ZapfDingbats and MacExpert were read as Latin the same way. That map is what `redact` matches patterns against, so a wrong character there is a silent redaction failure, and a wrong character is worse than no character. Symbol, ZapfDingbats and MacExpert now yield no mapping at all unless the font names a base encoding, which is always honoured as the deliberate statement it is. This keys on the base font *name*, not the /FontDescriptor symbolic flag. A great many ordinary Latin TrueType subsets set that flag and still rely on a Latin encoding, so suppressing on the flag would lose real text across a large share of real documents. A test pins that restraint so it is not "fixed" into a regression later. Glyph names now follow the published naming algorithm instead of two of its cases: a name is cut at its first `.` (`a.sc` is `a`), split on `_` into components (`f_i` is "fi"), and `uniXXXX` carries any number of four-digit values (`uni00660069` is "fi"). Names are resolved whole or not at all, so half a ligature is never returned. `glyph_text` returns a String for this; `glyph_unicode` stays as the single-character view. Hex parsing rejects the leading `+` that `from_str_radix` would otherwise accept, and a lone surrogate half. Both halves were verified by reverting each fix and watching the matching tests fail. The parity note on protect.redact-patterns now says Symbol-encoded text is skipped rather than mis-decoded. Still open, and an owner decision: Symbol and ZapfDingbats could be mapped properly with a glyph-name-to-Unicode table, but the vendored Adobe Glyph List is not on the AGENTS.md 1.1 closed list. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Tanish MankuandClaude
|
117430fade |
F1: measure an added item with the face it is written as
`Family::width` chose its standard-14 face from `bold` alone and passed `italic: false`, while `Family::base_font` writes the item out as the italic or oblique face. Every italic item was therefore laid out with upright metrics: Times-Italic `A` is 611 against Times-Roman's 722, so boxes, wrapping and centring were all sized from the wrong face. The measure now takes `italic` and the five call sites pass the item's own flag, which they already had in scope. The test walks all twelve style combinations and asserts the written face and the measured face are the same one, so the two can't drift apart again. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Tanish MankuandClaude Opus 5
|
aaa19d87af |
F1: put the generated table on the AGENTS.md 1.1 closed list
Isolated so it can be reverted on its own. AGENTS.md 1.1 keeps a closed list of Adobe-authored non-visual technical data, and adding to it needs the owner's explicit approval, which was given on 2026-10-08. The item itself -- the standard-14 font metrics and encoding tables from the PDF specification -- was already on that list; what is new is holding a copy first-party rather than only through hayro, which is what lets generated appearances measure with it and what survives hayro being retired in M2.8. The gate only understood the list for dependency-bundled files, and rejected adobe_data on a repository file outright. It now accepts it for a closed list of first-party paths, so crates/fonts/src/std14.rs carries an ATTRIBUTION entry with its SHA-256: regenerating the table fails the gate until the entry is updated, which makes a change to Adobe-derived data a deliberate act rather than an ordinary edit. Worth noting that .rs is not an asset extension, so the table would never have been scanned at all. Entering it on the list is a choice to be checked, not a requirement the gate imposed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tanish MankuandClaude Opus 5
|
d0d1ad8192 |
F1: exact standard-14 metrics
pdfcraft_fonts::helvetica_width was a fifteen-line guess by character class, and seven crates generated appearance streams through it: annot, edit, forms, ocr, redact, sign and xfa. Times was Helvetica x 0.9, bold was x 1.05, Courier a flat 0.6. Against the published metrics a Times capital A is 722, so that estimate said 600 -- a 17% error on a letter, compounding across a line. Std14 carries the real advance widths for all fourteen faces, indexed by character code: WinAnsiEncoding for the twelve text fonts, which is what we write into the appearance streams we generate, and their own built-in encodings for Symbol and ZapfDingbats. The tables are generated by joining the published encodings with the per-glyph widths. helvetica_width now delegates, so every call site gained exact Helvetica with no change. The three places that faked another face by scaling it -- Family in edit::added, the paragraph measure in edit::text, and the redaction overlay fonts -- now select the right face instead. So does Metrics::width in fonts::pdf, which measures existing document text when a font carries no /Widths; that path feeds redaction's glyph removal, where a wrong width means cutting the wrong glyphs. Tests spot-check the table against values typed in from the specification by hand, so it is verified against the spec rather than against whatever produced it; plus Courier being fixed pitch everywhere, full printable-ASCII and Latin-1 coverage, Symbol and ZapfDingbats not being WinAnsi, /BaseFont aliases and subset prefixes resolving, and hostile text staying finite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tanish MankuandClaude
|
681548a47f |
Add a PR template, and record the review practice and external-PR bar
Replaces this PR's earlier contents. It previously added a committed `docs/plan/` tree -- a status baseline, an architecture doc and an execution plan. Since then `main` has grown `docs/gaps.md`, `docs/target-app-parity.md`, `docs/roadmap.md` and `docs/architecture.md`, which are actively maintained (gaps.md was reviewed 2026-10-10 and updated 2026-10-11) and cover the same ground better, because they sit in the flow contributors already touch. Keeping a parallel plan would have meant two sources of truth and a document that rots; it had already gone conflicting twice in three days. So the plan is dropped and only the parts with no equivalent anywhere remain. `.github/pull_request_template.md` is new -- the repository had none, with 167 PRs open and 66 of them conflicting. It leads with the two things a contributor cannot be expected to know and that cannot be undone after a merge: provenance (no Adobe asset, no committed font, no copied GPL/AGPL code, behaviour from the spec rather than from reading another implementation) and the never-crash rules. It also asks for a test that has been *seen to fail* before the fix, and says plainly that per-crate verification is preferred because CI runs the workspace anyway. CLAUDE.md gains three things it did not record: the four-zoom review habit (clause, function, file, project) with the tests that mirror it; the external-PR bar; and a note that builds and tests should be scoped per crate, because a full workspace build has locked up a development machine and CI covers it. The commit rule now also says that `main` moves fast enough for PRs to rot -- 396 commits in three days -- and that conflict risk tracks how contested the files are rather than how large the change is. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
0a8b230ca0 |
D1: pin and verify the pdf.js corpus (#339)
xtask corpus cloned mozilla/pdf.js at HEAD with no commit pin and no content check, and the corpus tests returned early when the directory was missing. So the corpus sweep measured a moving target, xtask/baselines/pdfjs.json was compared against whatever upstream happened to be that day, and a damaged or tampered checkout produced green results. AGENTS.md §2 asks for fixtures fetched pinned by commit and sha256-verified. Verification is two layers. The commit fixes which tree we ask for; the manifest digest fixes what landed on disk: one '<sha256> <path>' line per file, sorted by path, then one sha256 over that text. Git object ids are SHA-1, so the manifest is what makes this a sha256 check. The pin is 5d923c9 with 1455 files, which matches what the GitHub tree API reports for test/pdfs. The checkout sets core.autocrlf=false and core.eol=lf. This repository has autocrlf=true globally, so without that git would rewrite line endings on checkout and the same commit would hash differently on Windows than on Linux; the pin has to mean the same thing everywhere. A successful verification writes corpus/pdfjs/.pdfcraft-corpus.json. The corpus tests now skip only when nothing is fetched, and fail loudly when a checkout exists without that stamp. xtask check verifies before sweeping, so the baseline is never compared against an unknown tree. Checked end to end: the unrecorded-pin guard refuses to fetch; --update-pin fetches and prints the constants; appending one byte to one of the 1455 files is detected and repaired by re-fetching; removing the stamp makes the corpus tests fail with a message saying what to run. Against the verified corpus the tests pass: 964 files ok and 7 password cases. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Brandon Thomas <echelon@gmail.com> |
||
+1       
|
cecbfad0ae |
D2, D3: fuzz every tool's arguments, and prove root confinement (#351)
* XFA: choice lists, signature, password, image and barcode fields; PNG and GIF pictures The xfa crate lays out the field kinds that were left blank or shown as text fields: - Choice lists become combo boxes (editable with textEntry) or list boxes (open="always"; multi-select with open="multiSelect"), with the shown items and the saved values of <items save="1"> as /Opt pairs, /V, /I and /DV as saved values, several on separate lines in the data. Field::saved_value maps shown text to saved values for the layout and the script tree alike, so scripts read what is saved. - Password fields are password text fields; signature fields are /Sig fields the app can sign. - Image fields show the picture the data (base64) or the template holds; barcode fields show their value as text, with a warning that no bars are drawn. - PNG and GIF pictures (fields and draws) are decoded to Flate image XObjects with a soft mask for transparency; JPEGs stay as they were. Sizes come from the headers, so layout never decodes; decoding is capped at 40 million pixels and failures are warnings, not errors. Engine: multi-select list values from scripts and data are one per line. README and parity notes updated (form.xfa-dynamic). Hardened after an adversarial review pass: picture sizes refused from the header past 20 million pixels or 65,535 a side, the GIF decoder given limits (a frame far larger than its screen is refused before allocation), template pictures read whole up to 32 MB, pictures decoded once and remembered in the catalog by content hash so re-layouts reuse them, data pictures that aren't images reported by field; choice values matched against saved values first, several values deduplicated and capped, /I ascending and unique, /DV through the same mapping, <value> children read, the item cap reported; the reopen path takes another viewer's choice values (several per line); CMYK JPEGs inverted only with Adobe APP14. * D5: README for cos and engine, and an honest cos status line Thirteen crates have no README, although the session protocol tells every session to read the README of the crate it is touching. These are the two biggest: cos is where every hostile file lands first, and engine is the whole API surface a frontend gets. Each states its layer and the rules that bind it, sketches the API against the real signatures, and ends with what is deliberately not done. The cos README spells out the clone-is-a-snapshot property and why both writers return bytes rather than taking a sink; the engine README spells out the four-step editing model that makes what you see what Save writes. Also corrects the module doc on cos/src/lib.rs, which still said encryption and object-stream output for full saves were not done. Both shipped: security.rs handles R2-R6 with crypt filters and /EFF, and SaveOptions::object_streams defaults to true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * D2, D3: fuzz every tool's arguments, and prove root confinement AGENTS.md §3 and §4 treat tool arguments as untrusted, but nothing fed hostile arguments to the tool table: xtask fuzz mutates PDFs only. And tools::check_args validates shape alone (object, no unknown keys, required present), not types or ranges, so a string where an integer belongs or i64::MIN where a page number belongs reaches the handler. Two tests. The first calls every tool with a zoo of wrong types, numeric extremes, empty and 100,000-character strings, NUL and right-to-left-override names, mistyped and deeply nested containers: 19,299 calls over 132 tools, each required to return Ok or a readable ToolError rather than panic. The second is the D3 audit mechanized: every path argument is tried with .. traversal, absolute, drive and UNC paths, and a planted symbolic link, and must be refused; 451 attempts, all refused. Reading the code says every handler resolves its own paths and that no tool dispatches another internally, so the PhotoCraft hole does not exist here. A test keeps that true as tools are added; a written finding would not. Both were proved to fail before passing. An out-of-bounds index in page_rotate is reported as: page_rotate panicked on {"degrees":-1,"doc":1}. Disabling resolve() is reported as 82 leaks naming the tool and path, including doc_save writing through ../ and through the symbolic link. The first draft of the confinement test did not catch a fully disabled resolve(): almost every tool failed on a non-existent document id before it ever looked at a path, and a file-listing diff cannot see a read. It now opens a real document first so handlers reach their path arguments, and asserts each call is refused rather than only diffing the filesystem. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Preserve alternate appearances and indirect push-button values Keep rollover, down and custom appearance entries when normal appearances are regenerated, without modifying shared dictionaries. Resolve indirect button caption and layout values. Cover full and incremental saved output, and wait for raster readiness in the existing Linux scroll test. Co-authored-by: Codex <codex@openai.com> * M13: common MCP tools through the confined automation path Preserve task tools and compact mode, add complete hints and strict argument-key errors, and catch escaped tool panics. Co-Authored-By: OpenAI Codex <noreply@openai.com> * M13: session resources and version-aware MCP result hints Keep per-document resources and legacy response shapes; add document/command resources and private cache hints for modern clients. Co-Authored-By: OpenAI Codex <noreply@openai.com> * M7: select and edit grouped Form XObject figures Co-Authored-By: Codex <noreply@openai.com> * M14: let neutral Windows locales fall through to display language Co-authored-by: Codex <codex@openai.com> * pdfcraft-cli --help lists `mcp` only when the build has it Follow-up to #540: a build without the `mcp` feature (`--no-default-features`) still listed `pdfcraft-cli mcp` in its usage, and `pdfcraft-cli mcp` then said "unknown command". USAGE is now put together with `concat!`, and its `mcp` lines come from a macro that is empty without the feature. In such a build `pdfcraft-cli mcp` says the MCP server was left out instead of calling it an unknown command. - tests/help.rs: usage_lists_mcp_only_when_built_with_it (runs in both feature sets) and mcp_without_the_feature_says_it_was_left_out (without the feature) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * engine README: the execution plan is local-only, so don't link a path that doesn't exist * Merge land/450 into land/414; XFA pictures are found again by a SHA-256 on their XObject The image cache was a /PCXfaImages map in the catalog, keyed by DefaultHasher (not stable across Rust releases) and read back from the file. Each image XObject now records the SHA-256 of its picture's bytes, and a re-layout finds the XObjects on the pages it replaces by that hash; nothing goes in the catalog. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Merge land/414 into land/451; keep main's MCP protocol versions, cap command_batch at 1000 steps The 2026-07-28 revision and its resultType/ttlMs/cacheScope fields are dropped: main doesn't advertise them and they couldn't be checked against the spec. command_batch refuses more than 1000 steps. docs/mcp.md says that unknown argument keys are now refused with -32602 (a behaviour change for clients that sent extra keys). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Merge land/451 into land/351; escape targets stay in the test's temp dir, and out/image paths are tried Absolute escape targets are now paths under the test's own temp base instead of /etc/passwd, C:\Windows\… and a UNC share, so a confinement bug can only touch files the test made. String 'out' and 'image' properties (and any described as a file path) are tried too. The backslash traversal runs only on Windows: elsewhere it is an ordinary file name inside the root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Yuvraj Singh <singh.yuvraj31@icloud.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: storybored379 <storybored379@users.noreply.github.com> Co-authored-by: OpenAI Codex <noreply@openai.com> Co-authored-by: Flowershangfromthebranches <152056395+Flowershangfromthebranches@users.noreply.github.com> Co-authored-by: Streetblock <github@streetblock.de> Co-authored-by: lloyd <lloyd@stackworx.io> Co-authored-by: Brandon Thomas <echelon@gmail.com> |
||
|
|
2d40d85450 |
D5: READMEs for cos and engine (#332)
* D5: README for cos and engine, and an honest cos status line Thirteen crates have no README, although the session protocol tells every session to read the README of the crate it is touching. These are the two biggest: cos is where every hostile file lands first, and engine is the whole API surface a frontend gets. Each states its layer and the rules that bind it, sketches the API against the real signatures, and ends with what is deliberately not done. The cos README spells out the clone-is-a-snapshot property and why both writers return bytes rather than taking a sink; the engine README spells out the four-step editing model that makes what you see what Save writes. Also corrects the module doc on cos/src/lib.rs, which still said encryption and object-stream output for full saves were not done. Both shipped: security.rs handles R2-R6 with crypt filters and /EFF, and SaveOptions::object_streams defaults to true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * M14: let neutral Windows locales fall through to display language Co-authored-by: Codex <codex@openai.com> * pdfcraft-cli --help lists `mcp` only when the build has it Follow-up to #540: a build without the `mcp` feature (`--no-default-features`) still listed `pdfcraft-cli mcp` in its usage, and `pdfcraft-cli mcp` then said "unknown command". USAGE is now put together with `concat!`, and its `mcp` lines come from a macro that is empty without the feature. In such a build `pdfcraft-cli mcp` says the MCP server was left out instead of calling it an unknown command. - tests/help.rs: usage_lists_mcp_only_when_built_with_it (runs in both feature sets) and mcp_without_the_feature_says_it_was_left_out (without the feature) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * engine README: the execution plan is local-only, so don't link a path that doesn't exist --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Streetblock <github@streetblock.de> Co-authored-by: Codex <codex@openai.com> Co-authored-by: lloyd <lloyd@stackworx.io> Co-authored-by: Brandon Thomas <echelon@gmail.com> |