mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-12 05:42:52 +00:00
fix(engine, automation): Action Wizard and doc_batch honour JavaScript being off (#5080)
## What this changes Batch processing now keeps the caller's "Enable JavaScript" setting. Before, each file was processed in a fresh `Session::new()`, where JavaScript is on, so open-time XFA scripts and "Run a JavaScript" steps ran and their results were saved. - `actions::run_on` and `ocr::recognize_file` take a `javascript: bool` and call `set_javascript` before opening the file. In `recognize_file` the open now goes through a small private helper, `ocr::open_file`, so the open path can be tested without the recognition models. - `doc_batch`'s `Recipe` carries the setting into its per-file session and its OCR step. - Callers pass their own `session.javascript()`: the desktop Action Wizard, desktop multi-file OCR, `action_run`, `ocr_recognize_files` and `doc_batch`. Also relevant to #5054: an administrator's `DisableJavaScript` policy now reaches these sessions. Fixes #5080 ## How you verified it Regression tests, adopted from the reviewers' patches, plus one for the OCR open path: - `crates/engine/src/tests.rs::an_action_runs_a_javascript_step_only_when_javascript_is_on` (croquete1): a "Run a JavaScript" step writes its field with JavaScript on, and `run_on(.., false, ..)` refuses it instead of saving a scripted value. croquete1 reported it failing with the `set_javascript` call in `run_on` removed. - `crates/automation/tests/it/automation.rs::file_processing_keeps_javascript_off_for_xfa_open_scripts` (Harrison-Booth): with `js_enabled` off, an XFA initialize script that sets `qty` to 42 doesn't reach the output of `action_run`, `doc_batch` or (only when its models are installed) `ocr_recognize_files`. The contributor reported that each propagation removed on its own makes it fail. - `crates/engine/src/ocr.rs::tests::files_open_for_recognition_with_the_callers_javascript_preference`: opens the scripted XFA fixture through `recognize_file`'s open path with JavaScript off (the initialize script leaves `qty` empty) and on (it sets `qty` to 2). This covers the OCR propagation in runs without the OCR models. What was run locally, and what wasn't: - `cargo fmt --check` and `cargo clippy --all-targets -- -D warnings` were clean on `pdfcraft-engine`, `pdfcraft-automation` and `pdfcraft-ui-egui` at the earlier heads. On the latest commit only `cargo fmt --all` was run; clippy was not re-run. - No tests were run locally, including the new `ocr::tests` one: test builds time out on this machine, and the run was stopped before it compiled. The new test has not been seen failing either. If `set_javascript` is removed, the session keeps its JavaScript-on default, so the off case would read `qty` = 2 like the on case, and the first assertion would fail. Seeing that happen is still owed. - The tests are left to the maintainers' CI. No CI result is claimed for this PR. - [x] `cargo fmt --check` - [ ] clippy with `-D warnings` on the crates I changed (clean at the earlier heads; not re-run on the latest commit) - [ ] tests pass on the crates I changed (not run locally) - [x] `cargo xtask layers` and `cargo xtask assets`: N/A. No crate boundaries, dependencies or assets change. ## Never-crash - [x] No new `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` outside tests. The new non-test code only passes a `bool` and propagates errors with `?`. - [x] No indexing or slicing with input-derived positions, no new arithmetic, allocations or recursion. - [x] No `unsafe` - [x] Any crash this fixes has a regression test: N/A, this fixes no crash. ## Provenance - [x] No asset from an Adobe product, and no Adobe icon recreated, traced or imitated (no assets change) - [x] No font files committed here - [x] No GPL/AGPL code copied in - [x] Behaviour comes from the project's own JavaScript preference (`Session::set_javascript`) and issue #5080, not from another implementation's source - [x] Every new asset has an `ATTRIBUTION.toml` entry: N/A, no new assets ## Security - [x] `--root` confinement still holds: `action_run` and `ocr_recognize_files` still resolve every input and output through `resolve`, and `doc_batch`'s path handling is unchanged. No new paths are read or written. - [x] MCP and the UI control channel are untouched. - [x] No new outbound network access. - This narrows script execution: with JavaScript off, the per-file sessions of the Action Wizard, multi-file OCR, `action_run`, `ocr_recognize_files` and `doc_batch` no longer run document or step scripts. ## Parity and reach - [ ] `parity/acrobat-features.toml` not updated: this is a bug fix, and the `misc.batch-processing` entry's status (`partial`) and notes stay true. - [x] Reachable headlessly: `action_run`, `ocr_recognize_files` and `doc_batch` take the setting from the session, which `js_enabled` sets. - [x] Docs: N/A. No documented behaviour changes (`docs/cli-batch.md` doesn't describe JavaScript handling). ## Anything a reviewer should look at first - The existing `run_on` tests pass `true` to keep their behaviour; the tests above cover the JavaScript-off path. - `recognize_file` now has 7 parameters, the `too_many_arguments` limit. - The OCR branch of the end-to-end test runs only where the OCR models are installed. The engine unit test covers the OCR open path without them. Provisioning the models in CI is left as a separate decision. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * File actions, batch processing, and OCR now honor the JavaScript setting from your current session for each file. * When JavaScript is disabled, document scripts—including scripts that run when a document opens—won’t execute during processing. When enabled, JavaScript steps can run as expected. * This behavior applies across automated and in-app workflows. Per-file results and OCR output handling remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
de1fe40da5
commit
3be126f9bb
11 files changed
+212
-45
No files matched your search
@@ -118,6 +118,9 @@ impl Automation {
|
||||
Ok(json!({ "actions": actions, "steps": steps }))
|
||||
}
|
||||
|
||||
/// `action_run`: run a named action or a list of steps on each file in `paths`, writing the
|
||||
/// results into `folder`; each file is processed in its own session that keeps this
|
||||
/// session's JavaScript preference.
|
||||
pub(crate) fn action_run(&mut self, a: &Args) -> Result<Value> {
|
||||
use pdfcraft_engine::actions::{Action, Step, builtin, run_on};
|
||||
let action = if let Some(name) = a.opt_str("action")? {
|
||||
@@ -138,6 +141,8 @@ impl Automation {
|
||||
};
|
||||
let folder = self.resolve(a.str("folder")?, true)?;
|
||||
std::fs::create_dir_all(&folder).map_err(|e| failed(e.to_string()))?;
|
||||
// Each file runs in its own session; it keeps this one's JavaScript preference.
|
||||
let javascript = self.session.javascript();
|
||||
let mut out = Vec::new();
|
||||
for p in a.strs("paths")? {
|
||||
let name = std::path::Path::new(p).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "document.pdf".into());
|
||||
@@ -145,7 +150,7 @@ impl Automation {
|
||||
.resolve(p, false)
|
||||
.map_err(|e| e.to_string())
|
||||
.and_then(|src| crate::read_input_bytes(&src).map_err(|e| e.to_string()))
|
||||
.and_then(|bytes| run_on(&action, &name, std::sync::Arc::new(bytes), |_, _| {}));
|
||||
.and_then(|bytes| run_on(&action, &name, std::sync::Arc::new(bytes), javascript, |_, _| {}));
|
||||
out.push(match result {
|
||||
Ok(r) => {
|
||||
let target = crate::batch::publish(&child(&folder, &name), &r.bytes)?;
|
||||
|
||||
@@ -164,6 +164,8 @@ impl Automation {
|
||||
}
|
||||
// The engines are built (and named if they cannot be) before the first file is read.
|
||||
let recognizers = pdfcraft_engine::ocr::recognizers(&settings).map_err(failed)?;
|
||||
// Each file opens in its own session; it keeps this one's JavaScript preference.
|
||||
let javascript = self.session.javascript();
|
||||
let mut out = Vec::new();
|
||||
let (mut ok, mut skipped_files, mut words) = (0usize, 0usize, 0usize);
|
||||
for p in paths {
|
||||
@@ -171,7 +173,15 @@ impl Automation {
|
||||
let result =
|
||||
self.resolve(p, false).map_err(|e| e.to_string()).and_then(|src| crate::read_input_bytes(&src).map_err(|e| e.to_string())).and_then(
|
||||
|bytes| {
|
||||
pdfcraft_engine::ocr::recognize_file(&name, std::sync::Arc::new(bytes), None, settings.clone(), &recognizers, |_, _| true)
|
||||
pdfcraft_engine::ocr::recognize_file(
|
||||
&name,
|
||||
std::sync::Arc::new(bytes),
|
||||
None,
|
||||
settings.clone(),
|
||||
&recognizers,
|
||||
javascript,
|
||||
|_, _| true,
|
||||
)
|
||||
},
|
||||
);
|
||||
match result {
|
||||
|
||||
@@ -19,9 +19,13 @@ struct Recipe<'a> {
|
||||
images: optimize::ImageSettings,
|
||||
mono: optimize::MonoSettings,
|
||||
ocr: ocr::OcrSettings,
|
||||
/// The calling session's JavaScript preference, carried into each file's own session.
|
||||
javascript: bool,
|
||||
}
|
||||
|
||||
impl<'a> Recipe<'a> {
|
||||
/// Read and validate `doc_batch`'s steps and settings, taking the JavaScript preference from
|
||||
/// `auto`'s session.
|
||||
fn parse(auto: &Automation, args: &'a Args) -> Result<Self> {
|
||||
let steps = args.strs("steps")?;
|
||||
if steps.is_empty() || steps.len() > 3 {
|
||||
@@ -60,7 +64,7 @@ impl<'a> Recipe<'a> {
|
||||
if args.opt_num("dpi")?.is_some_and(|dpi| !(72.0..=600.0).contains(&dpi)) {
|
||||
return Err(Args::wrong("dpi", "72 to 600"));
|
||||
}
|
||||
Ok(Self { steps, images, mono, ocr: auto.ocr_settings(args)? })
|
||||
Ok(Self { steps, images, mono, ocr: auto.ocr_settings(args)?, javascript: auto.session.javascript() })
|
||||
}
|
||||
|
||||
fn accepts(&self, path: &Path) -> bool {
|
||||
@@ -68,9 +72,13 @@ impl<'a> Recipe<'a> {
|
||||
extension == "pdf" || (self.steps.contains(&"convert") && pdfcraft_engine::CONVERTIBLE.contains(&extension.as_str()))
|
||||
}
|
||||
|
||||
/// Run the recipe's steps on the file at `path` in a fresh session; returns the output bytes
|
||||
/// and a report entry per step.
|
||||
fn process(&self, path: &Path) -> Result<(Arc<Vec<u8>>, Vec<Value>)> {
|
||||
// A fresh session per file bounds retained document state and never touches caller tabs.
|
||||
// It keeps the caller's JavaScript preference, set before any open runs open-time scripts.
|
||||
let mut session = Session::new();
|
||||
session.set_javascript(self.javascript);
|
||||
let mut input = Vec::new();
|
||||
std::fs::File::open(path).map_err(failed)?.take(MAX_INPUT + 1).read_to_end(&mut input).map_err(failed)?;
|
||||
if input.len() as u64 > MAX_INPUT {
|
||||
@@ -85,7 +93,8 @@ impl<'a> Recipe<'a> {
|
||||
"convert" => bytes = session.convert_to_pdf(name, &bytes)?.1,
|
||||
"ocr" => {
|
||||
let recognizers = ocr::recognizers(&self.ocr).map_err(failed)?;
|
||||
let result = ocr::recognize_file(name, bytes, None, self.ocr.clone(), &recognizers, |_, _| true).map_err(failed)?;
|
||||
let result =
|
||||
ocr::recognize_file(name, bytes, None, self.ocr.clone(), &recognizers, self.javascript, |_, _| true).map_err(failed)?;
|
||||
log.push(
|
||||
json!({"step":"ocr","words":result.words(),"skipped_pages":result.pages.iter().filter(|p| p.skipped.is_some()).count()}),
|
||||
);
|
||||
|
||||
@@ -19,14 +19,14 @@ fn one_step(step: Step) -> Action {
|
||||
/// The standard fonts can't draw the character, so the watermark step fails and no file is produced.
|
||||
#[test]
|
||||
fn failing_watermark_step_fails_the_file_and_publishes_nothing() {
|
||||
let r = run_on(&one_step(Step::AddWatermark("中".into())), "one.pdf", Arc::new(pdf()), |_, _| {});
|
||||
let r = run_on(&one_step(Step::AddWatermark("中".into())), "one.pdf", Arc::new(pdf()), true, |_, _| {});
|
||||
assert!(r.is_err(), "a failed step must fail its file: {r:?}");
|
||||
}
|
||||
|
||||
/// A comment step on a document with no comments has nothing to do, which is not a failure.
|
||||
#[test]
|
||||
fn nothing_to_do_step_still_succeeds() {
|
||||
let r = run_on(&one_step(Step::FlattenComments), "one.pdf", Arc::new(pdf()), |_, _| {});
|
||||
let r = run_on(&one_step(Step::FlattenComments), "one.pdf", Arc::new(pdf()), true, |_, _| {});
|
||||
assert!(r.is_ok(), "a step with nothing to do is not a failure: {r:?}");
|
||||
}
|
||||
|
||||
@@ -61,6 +61,6 @@ fn action_run_marks_the_failing_file_as_error_without_output() {
|
||||
/// A removal step with nothing to remove keeps its existing note and succeeds.
|
||||
#[test]
|
||||
fn removal_step_with_nothing_to_remove_still_succeeds() {
|
||||
let r = run_on(&one_step(Step::RemoveHiddenInformation), "one.pdf", Arc::new(pdf()), |_, _| {});
|
||||
let r = run_on(&one_step(Step::RemoveHiddenInformation), "one.pdf", Arc::new(pdf()), true, |_, _| {});
|
||||
assert!(r.is_ok(), "nothing to remove is not a failure: {r:?}");
|
||||
}
|
||||
@@ -9694,6 +9694,8 @@ fn comparing_documents_through_tools() {
|
||||
assert!(a.call("doc_compare", &json!({ "doc": v2, "other": 999 })).is_err());
|
||||
}
|
||||
|
||||
/// `action_list` and `action_run`: built-in actions by name and ad-hoc steps run on files,
|
||||
/// missing inputs are reported per file, and an unknown step is refused.
|
||||
#[test]
|
||||
fn actions_through_tools() {
|
||||
let dir = workdir("actions");
|
||||
@@ -9711,6 +9713,71 @@ fn actions_through_tools() {
|
||||
assert!(a.call("action_run", &json!({ "steps": [{ "step": "fly" }], "paths": ["a.pdf"], "folder": "x" })).is_err());
|
||||
}
|
||||
|
||||
/// JavaScript stays off in the short-lived sessions used by the three file-processing entry
|
||||
/// points (#5080). The input's initialize script changes `qty` to 42, which is visible in the
|
||||
/// saved XFA data if any entry point opens it with JavaScript enabled.
|
||||
#[test]
|
||||
fn file_processing_keeps_javascript_off_for_xfa_open_scripts() {
|
||||
let dir = workdir("batch-javascript-off-xfa");
|
||||
let mut seed = 0x1234_5678_u32;
|
||||
let padding: String = (0..32_000)
|
||||
.map(|_| {
|
||||
seed ^= seed << 13;
|
||||
seed ^= seed >> 17;
|
||||
seed ^= seed << 5;
|
||||
char::from(b'a' + (seed % 26) as u8)
|
||||
})
|
||||
.collect();
|
||||
let template = pdfcraft_xfa::fixtures::scripted_template()
|
||||
.replace("if (qty.rawValue === null) qty.rawValue = 2;", "qty.rawValue = 42; pad.rawValue = '';")
|
||||
.replace(
|
||||
"<field name=\"qty\"",
|
||||
&format!(
|
||||
"<field name=\"pad\" w=\"2in\" h=\"0.3in\"><ui><textEdit/></ui><value><text>{padding}</text></value></field>\n <field name=\"qty\""
|
||||
),
|
||||
);
|
||||
let mut source = pdfcraft_xfa::fixtures::shell(&template);
|
||||
// Give the optimizer an unreferenced stream to discard so the optimized output is saved,
|
||||
// carrying any XFA script mutation along with it if JavaScript was accidentally enabled.
|
||||
let previous_xref =
|
||||
std::str::from_utf8(&source).unwrap().rsplit_once("startxref\n").unwrap().1.split_whitespace().next().unwrap().parse::<usize>().unwrap();
|
||||
let orphan = "x".repeat(64_000);
|
||||
let orphan_offset = source.len();
|
||||
source.extend_from_slice(format!("999 0 obj\n<< /Length {} >>\nstream\n{orphan}\nendstream\nendobj\n", orphan.len()).as_bytes());
|
||||
let xref = source.len();
|
||||
source.extend_from_slice(
|
||||
format!("xref\n999 1\n{orphan_offset:010} 00000 n \ntrailer\n<< /Size 1000 /Root 1 0 R /Prev {previous_xref} >>\nstartxref\n{xref}\n%%EOF\n")
|
||||
.as_bytes(),
|
||||
);
|
||||
std::fs::write(dir.join("scripted.pdf"), &source).unwrap();
|
||||
|
||||
let mut a = auto(&dir);
|
||||
ok(&mut a, "js_enabled", json!({ "enabled": false }));
|
||||
|
||||
ok(&mut a, "action_run", json!({ "steps": [{ "step": "set_title", "arg": "Processed" }], "paths": ["scripted.pdf"], "folder": "actions" }));
|
||||
ok(&mut a, "doc_batch", json!({ "paths": ["scripted.pdf"], "steps": ["optimize"], "out_dir": "batch" }));
|
||||
|
||||
// Open each result through the same JavaScript-off session and inspect the field data.
|
||||
for path in ["actions/scripted.pdf", "batch/scripted.pdf"] {
|
||||
let doc = ok(&mut a, "doc_open", json!({ "path": path }))["doc"].as_u64().unwrap();
|
||||
let fields = ok(&mut a, "form_fields", json!({ "doc": doc }));
|
||||
let qty = fields["fields"].as_array().unwrap().iter().find(|field| field["name"] == "qty").unwrap();
|
||||
assert_ne!(qty["value"], "42", "{path} ran the XFA initialize script");
|
||||
}
|
||||
|
||||
// OCR needs its language models. When present, it uses the same per-file open path and must
|
||||
// also keep the script from changing the saved field value. Without them, the engine's
|
||||
// `ocr::tests::files_open_for_recognition_with_the_callers_javascript_preference` still
|
||||
// checks the preference `recognize_file` opens each file with.
|
||||
if ok(&mut a, "ocr_status", json!({}))["available"] == true {
|
||||
ok(&mut a, "ocr_recognize_files", json!({ "paths": ["scripted.pdf"], "folder": "ocr", "skip_text_pages": false }));
|
||||
let doc = ok(&mut a, "doc_open", json!({ "path": "ocr/scripted.pdf" }))["doc"].as_u64().unwrap();
|
||||
let fields = ok(&mut a, "form_fields", json!({ "doc": doc }));
|
||||
let qty = fields["fields"].as_array().unwrap().iter().find(|field| field["name"] == "qty").unwrap();
|
||||
assert_ne!(qty["value"], "42", "OCR ran the XFA initialize script");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn action_run_keeps_outputs_with_duplicate_input_basenames() {
|
||||
let dir = workdir("action-duplicate-basenames");
|
||||
|
||||
@@ -151,6 +151,8 @@ fn images_refused_note(images: usize) -> Option<String> {
|
||||
(images > 0).then(|| format!("Reduce file size: {images} image(s) stayed as they were, past what one run may decode"))
|
||||
}
|
||||
|
||||
/// Apply one action step to document `id`, adding what it did to `log`; returns the new
|
||||
/// document when the step replaced it.
|
||||
fn apply_step(s: &mut Session, id: DocId, step: &Step, log: &mut Vec<String>) -> Result<Option<DocId>, EditError> {
|
||||
let pages = s.get(id).map_or(0, |d| d.info.pages.len());
|
||||
let all: Vec<usize> = (0..pages).collect();
|
||||
@@ -209,9 +211,19 @@ fn apply_step(s: &mut Session, id: DocId, step: &Step, log: &mut Vec<String>) ->
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
/// Run `action` on one file. `progress(step, steps)` is called before each step.
|
||||
pub fn run_on(action: &Action, name: &str, bytes: Arc<Vec<u8>>, mut progress: impl FnMut(usize, usize)) -> Result<FileResult, String> {
|
||||
/// Run `action` on one file. `javascript` is the caller's [`Session::javascript`] preference:
|
||||
/// when off, neither open-time scripts nor "Run a JavaScript" steps run. `progress(step, steps)`
|
||||
/// is called before each step.
|
||||
pub fn run_on(
|
||||
action: &Action,
|
||||
name: &str,
|
||||
bytes: Arc<Vec<u8>>,
|
||||
javascript: bool,
|
||||
mut progress: impl FnMut(usize, usize),
|
||||
) -> Result<FileResult, String> {
|
||||
let mut s = Session::new();
|
||||
// Before the open: open-time (XFA) scripts read it there.
|
||||
s.set_javascript(javascript);
|
||||
let mut id = s.open(name, None, bytes, None).map_err(|e| e.to_string())?;
|
||||
if let Some(why) = s.get(id).and_then(|d| d.read_only_reason.clone()) {
|
||||
return Err(why);
|
||||
|
||||
+46
-19
@@ -1156,6 +1156,7 @@ pub struct FileResult {
|
||||
}
|
||||
|
||||
impl FileResult {
|
||||
/// How many words were recognized on all of the file's pages.
|
||||
pub fn words(&self) -> usize {
|
||||
self.pages.iter().map(|p| p.words.len()).sum()
|
||||
}
|
||||
@@ -1202,19 +1203,21 @@ pub fn low_confidence_pages(pages: &[OcrPage], threshold: f32) -> Option<(usize,
|
||||
(count > 0).then_some((count, page_numbers))
|
||||
}
|
||||
|
||||
/// Recognize text in multiple files: one PDF's bytes in, the searchable PDF out. `progress`
|
||||
/// works as for [`OcrJob::run`]. A file that the skip-files option passes over comes back with
|
||||
/// [`FileResult::file_skipped`] set and its original bytes; the caller decides what to do with
|
||||
/// it, but it is never an OCR success.
|
||||
/// Recognize text in multiple files: one PDF's bytes in, the searchable PDF out. `javascript` is
|
||||
/// the caller's [`Session::javascript`] preference (when off, no open-time script runs).
|
||||
/// `progress` works as for [`OcrJob::run`]. A file that the skip-files option passes over comes
|
||||
/// back with [`FileResult::file_skipped`] set and its original bytes; the caller decides what to
|
||||
/// do with it, but it is never an OCR success.
|
||||
pub fn recognize_file(
|
||||
name: &str,
|
||||
bytes: Arc<Vec<u8>>,
|
||||
password: Option<&str>,
|
||||
settings: OcrSettings,
|
||||
recognizers: &Recognizers,
|
||||
javascript: bool,
|
||||
progress: impl FnMut(usize, usize) -> bool,
|
||||
) -> Result<FileResult, String> {
|
||||
match FileJob::open(name, bytes, password, settings)? {
|
||||
match FileJob::open(name, bytes, password, settings, javascript)? {
|
||||
Opened::Skipped(result) => Ok(result),
|
||||
Opened::Read(mut file) => {
|
||||
let job = file.job.take().ok_or("the document could not be read")?;
|
||||
@@ -1242,12 +1245,9 @@ pub enum Opened {
|
||||
}
|
||||
|
||||
impl FileJob {
|
||||
pub fn open(name: &str, bytes: Arc<Vec<u8>>, password: Option<&str>, settings: OcrSettings) -> Result<Opened, String> {
|
||||
let mut session = Session::new();
|
||||
let id = session.open(name, None, bytes.clone(), password).map_err(|e| e.to_string())?;
|
||||
if let Some(why) = session.get(id).and_then(|d| d.read_only_reason.clone()) {
|
||||
return Err(why);
|
||||
}
|
||||
/// Open the file; `javascript` is as for [`recognize_file`].
|
||||
pub fn open(name: &str, bytes: Arc<Vec<u8>>, password: Option<&str>, settings: OcrSettings, javascript: bool) -> Result<Opened, String> {
|
||||
let (session, id) = open_file(name, bytes.clone(), password, javascript)?;
|
||||
let job = session.ocr_job(id, &[], settings.clone()).ok_or("the document could not be read")?;
|
||||
// The skip-files probe runs before any engine work: a file with text needs no engine at
|
||||
// all, and the original bytes come back unchanged.
|
||||
@@ -1280,11 +1280,38 @@ impl FileJob {
|
||||
}
|
||||
}
|
||||
|
||||
/// Open one of [`recognize_file`]'s files in a session of its own that keeps the caller's
|
||||
/// JavaScript preference; a file that cannot be changed is refused with the reason.
|
||||
fn open_file(name: &str, bytes: Arc<Vec<u8>>, password: Option<&str>, javascript: bool) -> Result<(Session, DocId), String> {
|
||||
let mut s = Session::new();
|
||||
// Before the open: open-time (XFA) scripts read it there.
|
||||
s.set_javascript(javascript);
|
||||
let id = s.open(name, None, bytes, password).map_err(|e| e.to_string())?;
|
||||
if let Some(why) = s.get(id).and_then(|d| d.read_only_reason.clone()) {
|
||||
return Err(why);
|
||||
}
|
||||
Ok((s, id))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::export;
|
||||
|
||||
/// #5080: [`recognize_file`] opens each file with the caller's JavaScript preference, so an
|
||||
/// XFA initialize script (it sets an empty `qty` to 2) runs only when that is on. This checks
|
||||
/// the open path without the recognition models, which the end-to-end test needs.
|
||||
#[test]
|
||||
fn files_open_for_recognition_with_the_callers_javascript_preference() {
|
||||
let bytes = Arc::new(pdfcraft_xfa::fixtures::shell(&pdfcraft_xfa::fixtures::scripted_template()));
|
||||
let qty = |javascript| {
|
||||
let (s, id) = open_file("scripted.pdf", bytes.clone(), None, javascript).unwrap();
|
||||
s.get(id).unwrap().form.iter().find(|f| f.name == "qty").unwrap().value.clone()
|
||||
};
|
||||
assert!(qty(false).is_empty(), "the initialize script ran with JavaScript off");
|
||||
assert_eq!(qty(true), ["2"], "the initialize script did not run with JavaScript on");
|
||||
}
|
||||
|
||||
/// One 300 × 300 page drawing `content`, with Helvetica as `/F1`.
|
||||
fn page(content: &str) -> Arc<Vec<u8>> {
|
||||
let pdf = format!(
|
||||
@@ -1741,7 +1768,7 @@ mod tests {
|
||||
let bytes = s.create_from_text("t", "Already typed here").unwrap();
|
||||
let (r, fake) = fake_recognizers();
|
||||
let settings = OcrSettings { skip_text_files: true, ..Default::default() };
|
||||
let out = recognize_file("t.pdf", bytes.clone(), None, settings, &r, |_, _| true).unwrap();
|
||||
let out = recognize_file("t.pdf", bytes.clone(), None, settings, &r, true, |_, _| true).unwrap();
|
||||
assert_eq!(fake.calls(), 0, "the engine never ran");
|
||||
assert_eq!(out.bytes.as_ptr(), bytes.as_ptr(), "the original bytes came back");
|
||||
assert_eq!(out.file_skipped.as_deref(), Some("the file already contains text"));
|
||||
@@ -1756,7 +1783,7 @@ mod tests {
|
||||
let bytes = s.create_from_text("t", "Already typed here").unwrap();
|
||||
let (r, fake) = fake_recognizers();
|
||||
let settings = OcrSettings { skip_text_files: true, force_ocr: true, ..Default::default() };
|
||||
let out = recognize_file("t.pdf", bytes, None, settings, &r, |_, _| true).unwrap();
|
||||
let out = recognize_file("t.pdf", bytes, None, settings, &r, true, |_, _| true).unwrap();
|
||||
assert_eq!(fake.calls(), 1);
|
||||
assert_eq!(out.file_skipped, None);
|
||||
assert_eq!(out.words(), 1);
|
||||
@@ -1770,7 +1797,7 @@ mod tests {
|
||||
let bytes = s.create_from_text("t", "A page with text").unwrap();
|
||||
let (r, fake) = fake_recognizers();
|
||||
let settings = OcrSettings { force_ocr: true, ..Default::default() };
|
||||
let out = recognize_file("t.pdf", bytes, None, settings, &r, |_, _| true).unwrap();
|
||||
let out = recognize_file("t.pdf", bytes, None, settings, &r, true, |_, _| true).unwrap();
|
||||
assert_eq!(fake.calls(), 1, "the page's own text did not stop the run");
|
||||
assert_eq!(out.pages[0].skipped, None);
|
||||
}
|
||||
@@ -1782,12 +1809,12 @@ mod tests {
|
||||
let s = Session::new();
|
||||
let bytes = s.create_from_text("t", "A page with text").unwrap();
|
||||
let (r, fake) = fake_recognizers();
|
||||
let out = recognize_file("t.pdf", bytes, None, OcrSettings::default(), &r, |_, _| true).unwrap();
|
||||
let out = recognize_file("t.pdf", bytes, None, OcrSettings::default(), &r, true, |_, _| true).unwrap();
|
||||
assert_eq!(fake.calls(), 0);
|
||||
assert_eq!(out.pages[0].skipped.as_deref(), Some("the page already contains text"));
|
||||
assert_eq!(out.words(), 0);
|
||||
// Second run over the same (already text-bearing) file: skipped pages again, no calls.
|
||||
let again = recognize_file("t.pdf", out.bytes, None, OcrSettings::default(), &r, |_, _| true).unwrap();
|
||||
let again = recognize_file("t.pdf", out.bytes, None, OcrSettings::default(), &r, true, |_, _| true).unwrap();
|
||||
assert_eq!(fake.calls(), 0);
|
||||
assert_eq!(again.pages[0].skipped.as_deref(), Some("the page already contains text"));
|
||||
}
|
||||
@@ -1863,14 +1890,14 @@ mod tests {
|
||||
let bytes = s.create_from_text("t", "Already typed here").unwrap();
|
||||
let (r, fake) = fake_recognizers();
|
||||
let skip = OcrSettings { skip_text_files: true, ..Default::default() };
|
||||
let Ok(Opened::Skipped(out)) = FileJob::open("t.pdf", bytes.clone(), None, skip) else { panic!("the file should be skipped") };
|
||||
let Ok(Opened::Skipped(out)) = FileJob::open("t.pdf", bytes.clone(), None, skip, true) else { panic!("the file should be skipped") };
|
||||
assert_eq!(out.file_skipped.as_deref(), Some("the file already contains text"));
|
||||
let force = OcrSettings { force_ocr: true, ..Default::default() };
|
||||
let Ok(Opened::Read(mut file)) = FileJob::open("t.pdf", bytes.clone(), None, force.clone()) else { panic!("the file should be read") };
|
||||
let Ok(Opened::Read(mut file)) = FileJob::open("t.pdf", bytes.clone(), None, force.clone(), true) else { panic!("the file should be read") };
|
||||
let pages = file.job.take().unwrap().run(&r, |_, _| true);
|
||||
let out = file.finish(pages).unwrap();
|
||||
assert_eq!((fake.calls(), out.words(), out.file_skipped.clone()), (1, 1, None));
|
||||
let whole = recognize_file("t.pdf", bytes, None, force, &r, |_, _| true).unwrap();
|
||||
let whole = recognize_file("t.pdf", bytes, None, force, &r, true, |_, _| true).unwrap();
|
||||
// The saved bytes carry a modification time, so two saves a second apart differ there alone.
|
||||
assert_eq!((out.pages, out.file_skipped), (whole.pages, whole.file_skipped), "the same as reading it in one call");
|
||||
}
|
||||
|
||||
@@ -7491,13 +7491,15 @@ fn comparing_two_versions_of_a_document() {
|
||||
assert!(marks[0].contents.as_deref().unwrap().starts_with("Replaced: \"Monday.\""));
|
||||
}
|
||||
|
||||
/// Built-in and custom actions run their steps on a file (with JavaScript on), report each
|
||||
/// step's progress, and fail cleanly on a file that is not a PDF.
|
||||
#[test]
|
||||
fn actions_run_their_steps_on_files() {
|
||||
let s = Session::new();
|
||||
let src = s.create_from_text("t", "Quarterly numbers").unwrap();
|
||||
let built = actions::builtin();
|
||||
let numbers = built.iter().find(|a| a.name == "Add Page Numbers").unwrap();
|
||||
let r = actions::run_on(numbers, "q.pdf", src.clone(), |_, _| {}).unwrap();
|
||||
let r = actions::run_on(numbers, "q.pdf", src.clone(), true, |_, _| {}).unwrap();
|
||||
let mut s2 = Session::new();
|
||||
let id = s2.open("out.pdf", None, r.bytes, None).unwrap();
|
||||
assert!(page_texts(&s2, id)[0].contains("Page 1 of 1"), "{:?}", page_texts(&s2, id));
|
||||
@@ -7515,14 +7517,38 @@ fn actions_run_their_steps_on_files() {
|
||||
builtin: false,
|
||||
};
|
||||
let mut steps = Vec::new();
|
||||
let r = actions::run_on(&custom, "q.pdf", src, |i, n| steps.push((i, n))).unwrap();
|
||||
let r = actions::run_on(&custom, "q.pdf", src, true, |i, n| steps.push((i, n))).unwrap();
|
||||
assert_eq!(steps, [(0, 4), (1, 4), (2, 4), (3, 4)]);
|
||||
assert_eq!(r.log.len(), 4, "{:?}", r.log);
|
||||
let id = s2.open("out2.pdf", None, r.bytes, None).unwrap();
|
||||
let d = s2.get(id).unwrap();
|
||||
assert_eq!(d.info.title.as_deref(), Some("Q3"));
|
||||
assert!(page_texts(&s2, id)[0].contains("DRAFT"));
|
||||
assert!(actions::run_on(&custom, "bad.pdf", Arc::new(b"nope".to_vec()), |_, _| {}).is_err());
|
||||
assert!(actions::run_on(&custom, "bad.pdf", Arc::new(b"nope".to_vec()), true, |_, _| {}).is_err());
|
||||
}
|
||||
|
||||
/// #5080: a "Run a JavaScript" step honours the caller's JavaScript preference. With it on, the
|
||||
/// step runs and writes the field; with it off, the step does not run, so `run_on` reports the
|
||||
/// refused script's error instead of saving a script-produced value. Before the fix each file
|
||||
/// opened in a fresh JavaScript-on session, so the step ran whatever the caller chose.
|
||||
#[test]
|
||||
fn an_action_runs_a_javascript_step_only_when_javascript_is_on() {
|
||||
let action = actions::Action {
|
||||
name: "Set price".into(),
|
||||
description: String::new(),
|
||||
steps: vec![actions::Step::RunJavaScript("getField('price').value = 3;".into())],
|
||||
builtin: false,
|
||||
};
|
||||
let src = Arc::new(scripted_form());
|
||||
// JavaScript on: the step runs, so the saved file has price = 3.
|
||||
let on = actions::run_on(&action, "f.pdf", src.clone(), true, |_, _| {}).unwrap();
|
||||
let mut s = Session::new();
|
||||
s.set_javascript(false); // read the saved state without re-running any script
|
||||
let id = s.open("on.pdf", None, on.bytes, None).unwrap();
|
||||
let price = s.get(id).unwrap().form.iter().find(|f| f.name == "price").unwrap().value.clone();
|
||||
assert_eq!(price, ["3"], "the script ran with JavaScript on");
|
||||
// JavaScript off: the step does not run, so run_on fails rather than writing a scripted value.
|
||||
assert!(actions::run_on(&action, "f.pdf", src, false, |_, _| {}).is_err());
|
||||
}
|
||||
|
||||
/// A removal step that fails (here: a signed file can't be rewritten without its hidden data)
|
||||
@@ -7539,7 +7565,7 @@ fn actions_fail_when_hidden_information_could_not_be_removed() {
|
||||
let signed = s.sign(id, &digital_id, SignOptions { page: 1, ..SignOptions::default() }).unwrap();
|
||||
let built = actions::builtin();
|
||||
let sanitize = built.iter().find(|a| a.name == "Sanitize Documents").unwrap();
|
||||
let e = actions::run_on(sanitize, "signed.pdf", signed, |_, _| {}).map(|r| r.log).unwrap_err();
|
||||
let e = actions::run_on(sanitize, "signed.pdf", signed, true, |_, _| {}).map(|r| r.log).unwrap_err();
|
||||
assert!(e.starts_with("Sanitize document:"), "{e}");
|
||||
}
|
||||
|
||||
@@ -10967,7 +10993,7 @@ fn an_action_appends_to_a_file_whose_field_walk_passes_a_signature_over() {
|
||||
let bytes = Arc::new(signed_field_named(64 << 10));
|
||||
let steps = vec![actions::Step::from_id("set_title", "Checked").unwrap()];
|
||||
let title = actions::Action { name: "Title".into(), description: String::new(), steps, builtin: false };
|
||||
let r = actions::run_on(&title, "form.pdf", bytes.clone(), |_, _| {}).unwrap();
|
||||
let r = actions::run_on(&title, "form.pdf", bytes.clone(), true, |_, _| {}).unwrap();
|
||||
assert!(r.bytes.starts_with(&bytes[..]), "the action rewrote the file instead of appending to it");
|
||||
let mut s = Session::new();
|
||||
let id = s.open("out.pdf", None, r.bytes, None).unwrap();
|
||||
|
||||
@@ -98,6 +98,8 @@ impl PdfCraftApp {
|
||||
let p = progress.clone();
|
||||
// The summary is written on the worker thread: draw it in the UI's language.
|
||||
let lang = crate::i18n::current();
|
||||
// Each file runs in its own session; it keeps this one's JavaScript preference.
|
||||
let javascript = self.session.javascript();
|
||||
let work = move || {
|
||||
crate::i18n::set_current(lang);
|
||||
let (mut ok, mut failed) = (0, Vec::new());
|
||||
@@ -105,7 +107,7 @@ impl PdfCraftApp {
|
||||
if let Ok(mut s) = p.lock() {
|
||||
s.done = i;
|
||||
}
|
||||
let r = pdfcraft_engine::actions::run_on(&action, &name, Arc::new(bytes), |_, _| {}).and_then(|r| {
|
||||
let r = pdfcraft_engine::actions::run_on(&action, &name, Arc::new(bytes), javascript, |_, _| {}).and_then(|r| {
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
write_result(&dir, &name, &r.bytes).map_err(|e| e.to_string())?;
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
|
||||
@@ -691,7 +691,8 @@ impl PdfCraftApp {
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
fn start_web_batch(&mut self, files: Vec<(String, Vec<u8>)>, settings: OcrSettings) {
|
||||
let progress = Arc::new(Mutex::new(BatchProgress { total: files.len(), ..Default::default() }));
|
||||
self.ocr_web_batch = Some(crate::ocr_web::Batch::new(files, settings));
|
||||
// Each file opens in its own session; it keeps this one's JavaScript preference.
|
||||
self.ocr_web_batch = Some(crate::ocr_web::Batch::new(files, settings, self.session.javascript()));
|
||||
self.ocr_batch = Some(progress);
|
||||
}
|
||||
|
||||
@@ -736,13 +737,16 @@ impl PdfCraftApp {
|
||||
let p = progress.clone();
|
||||
// The summary is written on the worker thread: draw it in the UI's language.
|
||||
let lang = crate::i18n::current();
|
||||
// Each file opens in its own session; it keeps this one's JavaScript preference.
|
||||
let javascript = self.session.javascript();
|
||||
let work = move || {
|
||||
crate::i18n::set_current(lang);
|
||||
let write = move |name: &str, r: &pdfcraft_engine::ocr::FileResult| {
|
||||
crate::editing::write_atomically(&dir.join(name).to_string_lossy(), &r.bytes).map_err(|e| e.to_string())
|
||||
};
|
||||
// A panic in the recognizer still ends the batch with a summary (#3919).
|
||||
let msg = pdfcraft_engine::guard(|| run_batch_files(files, settings, &recognizers, &p, write)).unwrap_or_else(|panic| failed(&panic));
|
||||
let msg = pdfcraft_engine::guard(|| run_batch_files(files, settings, &recognizers, javascript, &p, write))
|
||||
.unwrap_or_else(|panic| failed(&panic));
|
||||
if let Ok(mut s) = p.lock() {
|
||||
s.done = s.total;
|
||||
s.message = Some(msg);
|
||||
@@ -981,12 +985,14 @@ impl PdfCraftApp {
|
||||
/// The batch's worker: read each file, hand the searchable copy to `write` (the output folder,
|
||||
/// or the web's download), count the buckets, and compose the summary. A file the skip-files
|
||||
/// option passes over is its own bucket and `write` is never called for it. A cancel is asked
|
||||
/// for between files: the file in flight finishes, the rest are counted as not run.
|
||||
/// for between files: the file in flight finishes, the rest are counted as not run. `javascript`
|
||||
/// is the app session's preference, for each file's own session.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub(crate) fn run_batch_files(
|
||||
files: Vec<(String, Vec<u8>)>,
|
||||
settings: OcrSettings,
|
||||
recognizers: &pdfcraft_engine::ocr::Recognizers,
|
||||
javascript: bool,
|
||||
progress: &Mutex<BatchProgress>,
|
||||
mut write: impl FnMut(&str, &pdfcraft_engine::ocr::FileResult) -> Result<(), String>,
|
||||
) -> String {
|
||||
@@ -1001,7 +1007,7 @@ pub(crate) fn run_batch_files(
|
||||
if let Ok(mut s) = progress.lock() {
|
||||
s.done = i;
|
||||
}
|
||||
let r = pdfcraft_engine::ocr::recognize_file(&name, Arc::new(bytes), None, settings.clone(), recognizers, |_, _| true);
|
||||
let r = pdfcraft_engine::ocr::recognize_file(&name, Arc::new(bytes), None, settings.clone(), recognizers, javascript, |_, _| true);
|
||||
match r {
|
||||
Ok(res) if res.skipped() => skipped += 1,
|
||||
Ok(res) => match write(&name, &res) {
|
||||
@@ -1328,7 +1334,7 @@ mod tests {
|
||||
let recognizers = Recognizers { primary: Arc::new(Fake { confidence: 95.0 }), secondary: None };
|
||||
let writes = Arc::new(std::sync::atomic::AtomicUsize::new(0));
|
||||
let w = writes.clone();
|
||||
let msg = run_batch_files(files, OcrSettings { skip_text_files: true, ..Default::default() }, &recognizers, &progress, move |_, _| {
|
||||
let msg = run_batch_files(files, OcrSettings { skip_text_files: true, ..Default::default() }, &recognizers, true, &progress, move |_, _| {
|
||||
w.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
|
||||
Ok(())
|
||||
});
|
||||
@@ -1338,7 +1344,7 @@ mod tests {
|
||||
let files = vec![("a.pdf".into(), pdf.to_vec()), ("b.pdf".into(), pdf.to_vec())];
|
||||
let progress = Mutex::new(BatchProgress { total: files.len(), ..Default::default() });
|
||||
let w = writes.clone();
|
||||
let msg = run_batch_files(files, OcrSettings { skip_text_files: true, ..Default::default() }, &recognizers, &progress, move |_, _| {
|
||||
let msg = run_batch_files(files, OcrSettings { skip_text_files: true, ..Default::default() }, &recognizers, true, &progress, move |_, _| {
|
||||
w.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
|
||||
Ok(())
|
||||
});
|
||||
@@ -1359,13 +1365,13 @@ mod tests {
|
||||
// The fixtures are text PDFs standing in for scans; the default skip-text-pages option
|
||||
// would (correctly) leave them alone, so the batch here reads everything.
|
||||
let settings = OcrSettings { skip_text_pages: false, ..Default::default() };
|
||||
let msg = run_batch_files(files, settings.clone(), &recognizers, &progress, |_, _| Ok(()));
|
||||
let msg = run_batch_files(files, settings.clone(), &recognizers, true, &progress, |_, _| Ok(()));
|
||||
assert!(msg.contains("Recognized 2 words in 2 files"), "{msg}");
|
||||
assert!(msg.contains("scan.pdf: 1 low-confidence word(s) on pages 1"), "{msg}");
|
||||
// The write failing is a per-file failure, named in the summary.
|
||||
let files = vec![("scan.pdf".into(), pdf.to_vec())];
|
||||
let progress = Mutex::new(BatchProgress { total: files.len(), ..Default::default() });
|
||||
let msg = run_batch_files(files, settings, &recognizers, &progress, |_, _| Err("disk full".into()));
|
||||
let msg = run_batch_files(files, settings, &recognizers, true, &progress, |_, _| Err("disk full".into()));
|
||||
assert!(msg.contains("; failed: scan.pdf: disk full"), "{msg}");
|
||||
}
|
||||
|
||||
@@ -1380,7 +1386,7 @@ mod tests {
|
||||
let files = vec![("scan.pdf".into(), pdf.to_vec())];
|
||||
let progress = Mutex::new(BatchProgress { total: files.len(), ..Default::default() });
|
||||
let settings = OcrSettings { skip_text_pages: false, low_confidence_threshold: threshold, ..Default::default() };
|
||||
run_batch_files(files, settings, &recognizers, &progress, |_, _| Ok(()))
|
||||
run_batch_files(files, settings, &recognizers, true, &progress, |_, _| Ok(()))
|
||||
};
|
||||
assert!(run(OcrSettings::default().low_confidence_threshold).contains("1 low-confidence word(s)"), "the default flags a 40% word");
|
||||
assert!(run(40.0).contains("1 low-confidence word(s)"), "at the threshold counts");
|
||||
|
||||
@@ -446,6 +446,8 @@ fn place(shot: &OcrShot, lines: Result<Vec<Line>, String>) -> OcrPage {
|
||||
pub struct Batch {
|
||||
files: VecDeque<(String, Vec<u8>)>,
|
||||
settings: OcrSettings,
|
||||
/// The app session's JavaScript preference, for each file's own session.
|
||||
javascript: bool,
|
||||
current: Option<(String, Box<FileJob>, Pages)>,
|
||||
total: usize,
|
||||
pub ok: usize,
|
||||
@@ -459,11 +461,12 @@ pub struct Batch {
|
||||
}
|
||||
|
||||
impl Batch {
|
||||
pub fn new(files: Vec<(String, Vec<u8>)>, settings: OcrSettings) -> Batch {
|
||||
pub fn new(files: Vec<(String, Vec<u8>)>, settings: OcrSettings, javascript: bool) -> Batch {
|
||||
let total = files.len();
|
||||
Batch {
|
||||
files: files.into(),
|
||||
settings,
|
||||
javascript,
|
||||
current: None,
|
||||
total,
|
||||
ok: 0,
|
||||
@@ -489,7 +492,7 @@ impl Batch {
|
||||
self.files.clear();
|
||||
}
|
||||
let Some((name, bytes)) = self.files.pop_front() else { return true };
|
||||
match FileJob::open(&name, Arc::new(bytes), None, self.settings.clone()) {
|
||||
match FileJob::open(&name, Arc::new(bytes), None, self.settings.clone(), self.javascript) {
|
||||
Ok(Opened::Skipped(_)) => self.skipped += 1,
|
||||
Ok(Opened::Read(mut file)) => match file.job.take() {
|
||||
Some(job) => self.current = Some((name, file, Pages::new(job.steps()))),
|
||||
|
||||
Reference in new issue
Block a user