mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-12 13:54:09 +00:00
c73cccfb33a9146470060bf692578fbabb543607
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Max Abramowitz
|
3a117c3c0e |
M10: open Word, Excel and PowerPoint files through LibreOffice
## What this changes
**File ▸ Open opens Word, Excel, PowerPoint, OpenDocument and RTF files** as new, unsaved PDFs (`Report.docx` → `Report.pdf`), the "LibreOffice sidecar" that `docs/gaps.md` (gap 8) and `docs/file-format-parity.md` planned for `create.from-office`.
- **Conversion** (`crates/create/src/office.rs`): an installed LibreOffice runs headless as a separate process, in a private temporary folder (the input is always `document.<ext>`, so the user's file name never reaches its command line). It gets a fresh profile with `DisableMacrosExecution`, `MacroSecurityLevel` 3 and `BlockUntrustedRefererLinks` set, and Writer's and Calc's `Content/Update/Link` set to *Never* (2 for Writer and 1 for Calc: the two number their values differently), no console window on Windows (`CREATE_NO_WINDOW`), and is killed after 120 s. The output is size-capped (512 MB), must start `%PDF-`, and must open as a PDF before it is used. LibreOffice is found through `PDFCRAFT_SOFFICE`, the `PATH`, then the usual install folders per OS. Without it, the user is told it's needed rather than "not a readable PDF".
- **Off the frame thread:** File ▸ Open converts on the existing open worker (`ResultKind::Converted`); the frame thread only opens the result.
- **Detection:** `SourceKind::Office` requires an Office extension *and* an OLE, ZIP or RTF signature, so `photos.zip`, or a PDF saved with a `.docx` name, are not misread.
- **Open dialog:** PDF (still the default), Images, Office documents, Text, All files. All files is left out on macOS, where rfd merges every filter into one allowed-types list, and on the web; Office documents are left out on the web. The extension lists are now shared constants in `pdfcraft-create` (`IMAGE_EXTENSIONS`, `TEXT_EXTENSIONS`, `OFFICE_EXTENSIONS`), used by both detection and the picker. The duplicate filter list in `open_dialog` is gone. The two new labels are in all 22 catalogues, with their `ATTRIBUTION.toml` hashes updated.
- **Automation:** `doc_create_multiple`, `doc_combine` and batch `convert` go through `convert_to_pdf`, so they take Office documents too; their descriptions say so.
**Deliberately not in this PR:** Insert pages, Combine files and Create ▸ Multiple files convert on the frame thread, so they now refuse Office documents with a pointer to File ▸ Open (`convert_to_pdf_inline` / `as_pdf_inline`) rather than freezing the window for seconds. Moving them onto a worker is the follow-up. `create.from-office` goes planned → **partial**, with that and the rest listed in its notes.
## How you verified it
On Windows 11 Pro with LibreOffice 26.8.1 installed, per crate:
- [x] `cargo fmt --check`
- [x] `cargo clippy -p pdfcraft-create -p pdfcraft-engine -p pdfcraft-ui-egui -p pdfcraft-automation -j 2 --all-targets -- -D warnings`
- [x] `cargo test -p pdfcraft-create -p pdfcraft-engine --lib` (42 and 276 pass); `-p pdfcraft-automation --test automation` (the combine/create tests); `-p pdfcraft-ui-egui --lib open_worker` and `--test editing --test create`
- [x] `cargo xtask layers`, `cargo xtask assets`, `cargo xtask parity --partial`
- [ ] wasm check: the `wasm32-unknown-unknown` target isn't installed here, so I'm leaving it to CI. The web paths are `cfg`'d and were reviewed by eye.
**Tests seen failing before the fix**, by reverting each part:
- Detection off → `office_documents_are_their_own_kind`, `source_kind_tells_pdfs_images_and_text_apart`, and both engine Office tests fail.
- Worker conversion off → `file_open_converts_an_office_document_on_the_worker` fails with the old user-facing message, *"the file is not a readable PDF: no readable page tree or cross-reference data was found"*.
New tests also cover a converter that hangs (it's stopped; the test binary stands in for LibreOffice), one that writes no PDF, a missing converter, temp-folder cleanup, `file://` URL escaping (spaces, drive letters, UTF-8), and the search order.
**Real conversions, end to end:** `crates/automation/tests/it/office.rs` builds a minimal DOCX, an XLSX (with a formula, `=6*7`), a PPTX and an RTF inside the test, so no binary fixtures are committed. It runs them through `doc_create_multiple` in `separate` mode (each output reopened and its text checked, including `42`) and in `combine` mode (page order and bookmarks), and through `doc_combine` beside a PDF, with a page range on one file. These tests, `libreoffice_converts_a_document_when_installed` and the process-tree cleanup test (5 out of 5 reruns) pass with LibreOffice 26.8.1. The Office tests skip where LibreOffice isn't installed, which includes CI.
Three existing tests used a 4-byte `report.docx` as their "file that can't be converted" example. That's no longer true, so they now use `archive.zip`, which keeps what they test.
## Never-crash
- [x] No new `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` outside tests
- [x] No input-derived indexing or slicing (`rsplit_once` for the extension); the PDF read back is size-capped
- [x] No `unsafe`. The Windows flag uses `std::os::windows::process::CommandExt`
- [x] A crashing, hanging or misbehaving LibreOffice is an error, never a panic or a stuck app: the worker's `catch_unwind`, a 120 s kill, and output checks
## Provenance
- [x] No Adobe assets; no fonts; no copied code
- [x] LibreOffice (MPL-2.0) is only *run* as an external process, as the docs planned; nothing of it is linked or copied. Its command-line options and the profile keys come from LibreOffice's public documentation.
- [x] The translations are contributor-original, written from the plain meaning of "Office documents" and "All files" ("Office" kept as a product name); hashes updated, `cargo xtask assets` passes
## Security
- [x] `--root` confinement unchanged: automation still resolves and reads paths before converting; only the private temp folder is written outside it
- [x] MCP and the control channel untouched
- [ ] **No new outbound network: please decide.** PdfCraft itself opens no connection. The profile blocks untrusted referer links and sets Writer and Calc never to update linked content on load. I converted eight synthetic files that link to a listener on 127.0.0.1: RTF `INCLUDETEXT` and `INCLUDEPICTURE`, a DOCX external picture, an ODT linked section and linked picture, an ODS table source and an external cell reference, and an ODP linked picture. None made a connection, with or without the link settings. Two things remain open: Impress has no link-update setting, and the process's network access isn't sandboxed (documented in `crates/create/README.md` and `docs/gaps.md`).
## Parity and reach
- [x] `create.from-office` planned → partial, with the evidence tests and what's still missing
- [x] Reachable headlessly (`doc_create_multiple`, `doc_combine`, batch `convert`)
- [x] Docs: `docs/file-format-parity.md` (Word/Excel/PowerPoint and RTF rows), `docs/gaps.md` gap 8, `docs/target-app-parity.md` row J (numbers not re-measured), `docs/roadmap.md` M10, the `ROADMAP.md` log, and the `crates/create` README
## Anything a reviewer should look at first
1. **The network question** above. It's the owner's call.
2. **The frame-thread restraint:** Insert, Combine and Create Multiple refuse Office documents rather than block. Is a worker for those wanted in this PR, or as a follow-up?
|
||
 Michael Higgins
|
4efcfa6ea3 |
fix(crypt, engine): refuse page extraction when Protect allows any change except extracting pages (#5081)
## What this changes Adds `Permissions::extract_pages()`: `assemble()` **and** copying (bit 5), as #5081 asks, since extracted pages carry their text and graphics into a file without the source's protection. It is used for Extract, Split, Split by size and every Combine/Insert/Replace/paste source check (`sources::source_document`). Changes within the document (rotate, delete, move, insert) keep `assemble()`. Since this PR was opened, #6309 made `assemble()` revision-aware on `main`: bit 11 alone at revision 3 and later, bit 4 at revision 2. That already refuses "Any except extracting pages" at revision 3+. This PR builds on it: - At revision 2, `assemble()` reads bit 4, which "Any except extracting pages" sets, so such a file could still be extracted and split into unprotected copies. Requiring copying closes that. - New `Needs::PageExtraction` greys out Extract and Split up front. It reuses the existing security-refusal string, so no catalogue changes. - `Document::allows_page_extraction()`, and an `extract_pages` key in the automation security JSON. - `page_extract` with `separate: true` extracts and writes one page at a time (`extract_separate`), so it no longer buffers every page. The first page is extracted before `out_dir` is created, so a refused extraction leaves no empty directory. If a later page fails, the files this call wrote are removed, and `out_dir` too if this call created it (review on this PR). - Insert Pages checks a PDF through the same path as the insert itself: new `Session::insert_source(dest, name, bytes)`, which the UI's insert now calls (review on this PR). - The `organize.combine` parity notes now say "page extraction". Fixes #5081 ## How you verified it `cargo fmt --all` is clean. Regression tests (adopted from Harrison-Booth's review patch, plus CodeRabbit's follow-ups): - `crates/crypt/src/tests.rs::page_extraction_needs_assembly_and_copying`: unit test for `extract_pages()`. - `crates/engine/src/tests.rs::extraction_permissions_cover_revision_three_to_six_and_leave_no_partial_output`: AnyExceptExtract on RC4-128, AES-128 and AES-256 refuses Extract, Split, Split by size and Combine/Insert/Replace sources without the permissions password, leaves source and destination untouched, and the owner can do everything. The authorized combine output has 2 pages, and a differently encrypted or unprotected destination is refused. - `crates/engine/src/tests.rs::page_extraction_needs_the_copy_permission_on_every_revision`: a file that withholds only copying (R2 RC4-40 and AES-256) refuses extraction; the owner password lifts it. - `crates/engine/src/tests.rs::pages_under_the_same_protection_are_inserted_without_page_extraction`: a separately built file under matching protection is inserted, and the result stays restricted. - `crates/automation/tests/it/automation.rs::refused_extraction_writes_no_split_or_page_output`, and the `extract_pages` checks in `protecting_through_tools`, which now cover three presets, including a user (not the owner) for whom extraction is allowed. **Only the crypt unit test has been run locally, and it passes.** The engine and automation tests timed out while compiling on this machine and have not been run, nor seen failing without the fix. The one `pdfcraft-ui-egui` call-site change has not been compiled. CI is the first place these build. - [x] `cargo fmt --check` - [ ] clippy with `-D warnings` on the crates I changed (not re-run after the rebase) - [ ] tests pass on the crates I changed (crypt only) ## Anything a reviewer should look at first - **Rebased onto #6309.** The tests no longer expect "Any except extracting pages" to allow assembly at revision 3+. The same-protection paste tests now use the "Inserting, deleting and rotating pages" preset without copying. - **Copy (bit 5) is now required**, as the issue suggests. `crates/ui-egui/tests/it/editing.rs::protected_files_that_can_be_combined_are_flagged` used a copy-withheld file as "protected but combinable"; it now uses a print-withheld file, since a no-copy file is no longer a Combine source without the permissions password. - **Same-document paste**: Paste re-opens the document's own bytes as an Insert source, which the new check would refuse. `sources::open_source_into` accepts a source that withholds extraction but allows assembly only when the destination will be saved under exactly the same protection (file key and `/Encrypt` values), so cut/copy and paste within one protected document still works. Pasting into an unprotected or differently protected document is refused. One limit: after changing security in the same session without saving, a cut can't be pasted back until you save. - **Not in this PR:** CodeRabbit's `disabled_reason` suggestion for read-only documents predates this change and covers other commands too, so it belongs in a separate UI change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit - **New Features** - Page extraction and splitting now follow a dedicated permission check, distinct from page assembly. - Pages can be inserted from certain protected PDFs when their protection settings match the destination document. - **Bug Fixes** - Separate-page extraction now cleans up files created during a failed operation and avoids creating an output directory when the first page cannot be extracted. - Security details now report whether page extraction is permitted. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
 .CooCoo
|
222a124650 |
M5: Import comments takes another PDF and copies its comments onto the same pages (#4595)
* M5: Import comments takes another PDF and copies its comments onto the same pages Import comments read only XFDF, FDF, XML, XDP, CSV and text data files. It now also takes a PDF: each comment of that PDF is copied onto the page with the same number, at the same place, with its appearance stream, its pop-up, its replies and its layer. Comments on pages past the document's last page are left out, together with replies to them. Links, form fields and redaction marks are not copied. A comment named like one already on its page replaces it, as an XFDF or FDF import does. A PDF without comments is refused with "nothing in the file matches this document", and a PDF that needs a password is refused. - organize: import_comments copies the comments with the page copier that Insert Pages uses, and registers the layers they use. - engine: Edit::ImportData reads a file that starts with %PDF- as a PDF. Any other file goes to the data importer first, and is read as a PDF when that fails, %PDF- occurs in its first 1,024 bytes and the PDF reader opens it. - ui: Comments > Import comments lists .pdf files; form.import_data lists the same files as before. - automation: the doc_import_data description names the PDF case. - parity: comment.import-from-pdf is shipped. * Import comments: an annotation that is no comment keeps its name A comment imported from a PDF replaced any annotation of the same /NM on its page that was not locked, a redaction mark or a widget, so a link, screen or 3D annotation of that name was overwritten. Only an annotation whose subtype is one of the copied comment subtypes is replaced now; for any other, the copy is added beside it without the name. The subtype is read through an indirect reference. * Import comments: index each page's annotations instead of scanning them per comment * Import comments: a layer that BaseState OFF hides stays hidden * Import comments: a PDF with binary bytes before its header imports on an XFA form, and two comments with one name both land On an XFA form, a decode failure no longer stops the import when a PDF header is in the first 1,024 bytes; the PDF's comments are copied. Two incoming comments on one page with the same /NM no longer replace the same comment: the first replaces it, and the second is added without the name. * Import comments: an inline annotation on the page is replaced by name An annotation written inline in a page's /Annots is made an indirect object before names are matched, so an incoming comment with its /NM replaces it instead of being added beside it. * Import comments: two new comments with one name keep it once When no comment on the page has the name, the first incoming comment keeps its /NM and each later one with the same name is added without it. |
||
  alehelandClaude Opus 5.5
|
20974e7822 |
refactor(engine): move the combine and page-source helpers to sources.rs
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |