feat(ocr): install the OCR engine and languages on demand instead of bundling them - #7259
feat(ocr): install the OCR engine and languages on demand instead of bundling them#7259samuelsl27 wants to merge 26 commits into
Conversation
Tesseract was invoked as the bare command "tesseract" in three places, so it could only ever be found through the system PATH. That is why OCR is unavailable on desktop installs: the MSI bundles the JRE and the JAR but no Tesseract, and Windows users are told to install it themselves. Add a tesseractPath to RuntimePathConfig resolved in this order: 1. system.customPaths.operations.tesseract from settings.yml 2. a Tesseract bundled next to the application 3. the bare "tesseract" command, as before Step 3 keeps Docker images, distro packages and dev machines behaving exactly as they did; only installs that actually ship a bundled binary change. The tessdata lookup gains the same bundled fallback ahead of the Linux default, which does not exist on Windows. Bundled resources are probed relative to the install path, the JAR directory and its parent, because Tauri puts the JAR in libs/ while resources sit at the bundle root. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Windows installer bundles a JRE and the JAR but no OCR engine, so a fresh desktop install cannot OCR at all: HowToUseOCR tells Windows users to install Tesseract themselves and edit settings.yml, and the desktop app is documented as needing a server for OCR. Add scripts/prepare-tesseract-bundle.ps1, which assembles a self-contained Tesseract from the UB Mannheim build (Apache-2.0, the reference Windows distribution) plus tessdata_fast language models, and register it as a `prepare` dependency so `task desktop:build` picks it up. The Tauri bundle now carries tesseract/, which RuntimePathConfig already knows how to find. The bundle keeps only what OCR needs. The DLL list was derived by elimination - removing each one and re-running OCR - and verified against the three ways Stirling-PDF drives Tesseract: text output, `pdf` output (which needs tessdata/configs/pdf, easy to miss) and `--psm 0` orientation detection (which needs osd.traineddata). Dropping the training tools and the Pango/Cairo/ICU stack they pull in takes the payload from 238 MB to 132 MB, English and Spanish included. Nothing is committed: the output directory is git-ignored and rebuilt on demand, so the repository does not grow by 132 MB of binaries. Also make the install-root lookup rely on java.home rather than the JAR location. The app ships as a Spring Boot fat JAR, whose code source URL has no file-system provider, so the JAR-relative probe cannot resolve there; the bundled JRE sits at a known depth under the install root and always can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ith the rest The bundle lookup is what makes OCR work on a fresh desktop install, but it was the one part with no test: the existing cases only covered an explicit settings.yml path and the fall-through to PATH. It could not be tested because the search took its roots from global state - the static install path and java.home - so a test could not stand up a fake install. Split the search from root discovery. findBundledPath(roots, relativePath) is now package-private and takes its roots as an argument, letting the tests build a directory laid out like a real install (libs/, runtime/jre/, tesseract/) under @tempdir and assert on it. Six cases, including the two that would silently break the feature: that a root's parent is probed, which is the only reason the JAR-in-libs/ layout Tauri produces resolves at all; and that an unresolvable name is swallowed rather than thrown, since this runs during bean construction where an escaping exception would take the application down instead of just leaving OCR unavailable. Also add tesseract:clean and wire it into desktop:clean. jlink:clean already removed the bundled JRE, so leaving the Tesseract runtime behind made "clean" mean two different things depending on which artifact you looked at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
What kind of size is the tesseract.exe bundle within the installer? What does the installer size delta? |
The already-built check looked for the executable, the DLLs and the config
files, but not for the languages that were asked for. So once a bundle existed,
./scripts/prepare-tesseract-bundle.ps1 -Languages spa,fra
printed "bundle already present" and exited without fetching French. The
parameter was unusable on that path, and it failed silently: no error, just a
bundle missing a language the caller had asked for.
Reported by the automated review on Stirling-Tools#7259.
The check now also requires every requested language to be present. Verified
against a bundle holding eng/osd/spa: asking for spa,fra now rebuilds and
fetches French, where before it exited early.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Measured on the NSIS installer built from this branch, against the current release installer. The bundle is trimmed rather than the whole Tesseract distribution. Unpacking the UB Mannheim installer gives 238 MB; dropping the training tools (lstmtraining, text2image and friends) and the Pango/Cairo/ICU stack they pull in takes it to 132 MB. Breakdown: The DLL list was derived by elimination — removing each one and re-running OCR — and verified against all three ways Stirling-PDF drives Tesseract: text output, pdf output (which needs tessdata/configs/pdf) and --psm 0 orientation detection (which needs osd.traineddata). Also fixed the -Languages bug the automated review caught (cb05995): the already-built check ignored the requested languages, so adding one to an existing bundle exited early and downloaded nothing. Good catch — it failed silently. On the other Aikido finding, invoking tesseract.exe from $OutputDir: that parameter is supplied by whoever runs the build, not by any external input, and the path defaults to a location inside the repo. |
|
Isn't this a better solution: https://docs.spring.io/spring-boot/api/java/org/springframework/boot/system/ApplicationHome.html It feels like you are essentially recreating what |
…directory Review on Stirling-Tools#7259 pointed out that ApplicationHome already does this, and it was right: two of the three probes here were hand-rolled versions of it. The JAR-relative probe read getCodeSource(), which reports "jar:file:...!/BOOT-INF/classes" under Spring Boot's nested class loader - no file-system provider, so it never resolved in the case it was written for. The java.home probe existed only to work around that failure, and it did so by assuming the JRE sits exactly two levels under the install root, which is true of the Tauri bundle and of nothing else. ApplicationHome.getDir() returns the directory holding the executable JAR and handles the nested loader properly, so both go away. What remains is the configured base path first - an operator who set one meant it - then the application home and its parent, since the bundler puts the JAR in libs/ and the tools beside it. Net 22 lines lighter, and the part most likely to be wrong is now library code rather than a guess about directory depth. 34 tests still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Stirling-PDF into feature/ocr-embebido
You're right, this is a better solution, I change it |
Groundwork for taking the ~130 MB Tesseract bundle out of the desktop installer and fetching it only when someone actually wants OCR. Nothing is wired to the installer or the UI yet; this is the backend half. The application knows exactly one address: a manifest listing, per platform, the engine and every language model with its URL, size and SHA-256. No download URL is compiled in. That indirection is the point - whoever publishes the manifest decides which engine build installations are handed, can withdraw or replace a bad artefact without shipping a new release, and can move the hosting anywhere. system.ocr.manifestUrl makes it configurable, which is also what lets an air-gapped or corporate install point at a local mirror. Everything lands in <installation path>/tesseract, which is already the first directory RuntimePathConfig probes for a bundled runtime, so path resolution is untouched: an installed engine is found exactly where an embedded one was. On the desktop that sits in the user's own application data, so no elevation is needed to add a language later. Guards, because this writes executable code next to the application: - SHA-256 is mandatory and verified before anything moves into place; an artefact listed without one is refused outright. The existing tessdata downloader in app/proprietary verifies nothing, and ChecksumUtils was sitting unused - it is used here. - Archive entries are resolved against the target and rejected if they land outside it, so an entry named ../../x cannot write there. - https or a local file only. Plain http would let whoever can rewrite the traffic rewrite the manifest and its digests in the same breath, which turns the checksum into theatre. - Entry-count and expanded-size ceilings, so a hostile archive is a clean failure rather than a full disk. - The engine is expanded into a sibling directory and only swapped in once it is complete, and models already installed are carried across, so an interrupted or failed install leaves the previous state rather than a runtime that exists but cannot run. - An archive without tessdata/configs/pdf is rejected: without that file Tesseract exits 0 having produced nothing, so a "successful" install would leave OCR silently mute. The endpoints are deliberately not admin-only. The desktop starts the backend with security.enableLogin=false, so an admin-gated endpoint is unreachable exactly where this is needed - which is why the existing tessdata downloader is useless there. Language changes take effect immediately, since the language list is read from disk on every request; only the engine needs a restart, because RuntimePathConfig resolves its paths once at startup. Fifteen tests, none of them touching the network - every artefact is served over a file: URL, the same path an air-gapped install uses. Verified by breaking the containment check and the digest comparison: three tests fail, precisely the ones guarding those two properties, and the remaining twelve keep passing. The disabled containment check really did write a file outside the temp directory, which is the behaviour the guard exists to stop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aller Takes the Tesseract bundle out of the desktop installer. tauri.conf.json no longer lists tesseract/**/* as a resource and desktop:prepare no longer builds one; the script that assembled it now produces the artefacts that get published once, which installations fetch on demand. Measured, since the size is what stalled Stirling-Tools#7259: the engine packs to 37.5 MB. The 131.97 MB figure was the bundle expanded on disk, and the "+97 MB installer delta" quoted earlier compared upstream's published MSI against a locally built NSIS package - two different formats, so that number should not be trusted. A like-for-like MSI measurement is still owed. What the engine keeps and what moved out: - eng.traineddata stays inside it. Tesseract falls back to English, so an engine without it refuses every job. - osd.traineddata is catalogued as an extra: 10 MB that only auto-rotate uses, and AutoRotateController already checks for it and degrades when it is absent. - Every other language is catalogued straight from tessdata_fast at a pinned commit rather than rehosted. Pinned, not main, because a SHA-256 only means something if the bytes it describes cannot move under it. The script refuses to publish an engine without tessdata/configs/pdf. That file is not decoration: "pdf" is the name of a config file Tesseract reads from there, not an output format, and an engine missing it exits 0 having written nothing - a "successful" install that silently cannot OCR. Two Windows PowerShell 5.1 traps worth recording, both found by inspecting the output rather than trusting the API: - Set-Content -Encoding utf8 emits a BOM, and a BOM in front of JSON is the parser's problem. The manifest is written through .NET instead. - Neither Compress-Archive nor ZipFile.CreateFromDirectory normalises separators there: both wrote "tessdata\configs\pdf", against the ZIP specification. Windows tolerates it; a Linux reader would create one file with backslashes in its name and the engine would arrive with no tessdata at all. Entries are now written one by one with '/'. Verified: zero backslash entries in the output. Four more tests cover the install end to end against a stand-in archive: it unpacks, an archive without configs/pdf is refused and leaves nothing behind, languages survive an engine reinstall, and a tampered archive leaves the previous runtime in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Windows section described a runtime shipped inside the installer, which is no longer what happens. Documentation that describes the previous version is worse than none. Also adds the warning the existing advice was missing: keep tessdata/configs. "pdf" is the name of a config file Tesseract reads from there, not an output format, so a tessdata directory of bare .traineddata files makes the engine exit successfully having written nothing - OCR appears to run and produces no file. Guidance circulating in the issue tracker tells people to delete exactly that directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Puts the on-demand installer where the need actually arises. The language picker's footer used to offer "View setup guide", which opened a documentation page and left the user to copy .traineddata files into a directory by hand - that is the whole of issue Stirling-Tools#6534, where someone downloaded ita.traineddata, could not work out where to put it, and got nowhere. It now opens an installer. Engine and languages are deliberately two separate actions. The engine is a one-off ~37 MB download that needs a restart before the tool becomes available; languages are small, changeable at any time, and take effect immediately because the backend re-reads the models from disk on every request. Rolling them into one step would imply that adding Catalan needs a restart too. Details that matter in use: - Every language shows its download size, because that is what makes someone pick two rather than ten. - English cannot be unticked. Tesseract falls back to it, so removing it leaves an engine installed and refusing every job. - A partial failure names the languages that failed and keeps the ones that landed, rather than reporting a blanket error that invites redoing all of it. - An unreachable catalogue is a notice, not an error screen: whatever is already installed still works. - The picker refreshes itself after the installed set changes, so a language added in the dialog is selectable without reopening the tool. Lives in src/core, not src/desktop: the latter is under a licence that forbids distributing modifications, and this has to be contributable. New keys go to en-US first, as the project requires, then es-ES. Both .toml files edited through Node - Windows PowerShell's Set-Content -Encoding utf8 has corrupted accented files here before. Verified: tsc --noEmit clean on src/core. The repository's own lint config could not be run - this branch predates the oxlint migration and wants eslint, while the installed node_modules matches the newer lock, so `npm ci` is owed before the PR. oxlint with default rules reports nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings the branch up to date with the original project before the on-demand OCR work goes any further. Only one file conflicted, LanguagePicker.tsx, and both sides were wanted: upstream moved the placeholder to a translated resolvedPlaceholder, and this branch wraps the dropdown so the installer dialog can sit beside it. Everything else auto-merged, including the three translation.toml files, which are the ones that usually hurt. Note for whoever builds next: this merge brings the ESLint to oxlint migration, so node_modules no longer matches package-lock.json and npm ci is owed.
Adds the wizard page the on-demand design needs: a checkbox for text recognition and one per common language, with the download happening while the install runs and the progress bar moving. The custom action is a DLL, and that is forced rather than preferred. An MSI custom action of type EXE - which is what the existing stirling-provision.exe is - runs out of process and is never handed the installer session handle, so it cannot drive the progress bar. A DLL entry point receives the MSIHANDLE and talks to the wizard through MsiProcessMessage: ACTIONSTART for the caption, PROGRESS to reset and advance the bar, ACTIONDATA for the line underneath. The MSI API is declared by hand against msi.lib; six functions is a smaller surface than a bindings crate that generates thousands, and it keeps the DLL at 1.8 MB, small enough to embed. Return="ignore" is the most important attribute in the fragment. A proxy that wants user credentials, a firewall, a laptop that loses its wifi mid-wizard are all ordinary, and none is a reason to roll back an otherwise good installation of a PDF editor. The action logs what happened, writes a note the application reads on first launch, and lets the install finish; the retry then happens in the app, with a real interface and no elevation. Same guards as the backend, because this runs elevated and writes executable code: SHA-256 mandatory and checked before anything is kept, https or a local file only, archive entries resolved against the target and refused if they climb out, and size and entry-count ceilings. An archive without tessdata/configs/pdf is rejected outright - without that file Tesseract exits 0 having written nothing, so a "successful" install would leave OCR silently mute. The panic is caught too: unwinding across the FFI boundary into msiexec is undefined behaviour. Everything is also driveable without any UI, which is what corporate deployment needs: msiexec /i Stirling-PDF.msi /qn STIRLING_OCR=1 STIRLING_OCR_LANGS=spa,eng The wizard contributes one property per checkbox, since MSI has no multi-select control that returns several values, and those are concatenated with the unattended list. That produces empty slots and a repeated eng, so the parser filters and de-duplicates rather than assuming clean input - covered by a test written from what the wizard actually sends. Uninstall removes the downloaded runtime through util:RemoveFolderEx against a path stashed in the registry. Without it, ~40 MB is orphaned, because none of it is in the MSI file table. The util namespace is declared in the fragment deliberately: tauri-bundler always passes WixUtilExtension to light, but candle picks its extensions by scanning each input file, so the declaration is what makes it compile. main.wxs is forked from tauri-bundler at @tauri-apps/cli v2.10.1, which is the version this repo pins, and the provenance is recorded at the top of the file. That fork is the real cost of this change and it is not hidden: Tauri gives no way to add a dialog without replacing the template. The change inside it is confined to one block, and the dialog is inserted using the same higher-Order NewDialog technique the template already uses to skip the license page. Verified as far as it can be without a full MSI build: the DLL compiles and its five tests pass, including a deliberate break of the containment check to prove the test catches it, and both .wxs files parse. The wizard page itself has not been through msiexec yet - that needs a complete desktop build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second entry point for the installer, plus the corrections that came out of checking this work against the project's own rules and re-reading my own code. Settings gains a Text recognition section. The panel it shows is the same one the OCR tool opens in a dialog, so it moved to components/shared/ocr and both render it; someone can now set OCR up before they need it rather than only at the moment they are blocked by it. Checked against the project's guidelines, three things were wrong: - The endpoints were under /api/v1/misc. That prefix is in the tool-model generator's allow list, so POST /api/v1/misc/ocr/languages would have been published as a pipeline step - which none of this is - and task tool-models:check would have failed on stale generated models. They now live under /api/v1/ui-data, beside the existing tessdata endpoints, which is both the right namespace and the project's own precedent. - The panel printed the backend's error text straight to the user, against the exception-handling guide's rule that user-facing text comes from the translation files. The heading is translated now and the backend string sits underneath as diagnostic detail, which is what it actually is: "SHA-256 mismatch", "HTTP 403". - A relative import, which the lint config forbids in favour of the @app alias. And two defects of my own, found by re-reading rather than by a tool: - The progress bar could never move. The download used Files.copy, which reports nothing, so the progress field always read zero - a bar that never advances is worse than no bar. It now copies in chunks and publishes the byte count as it goes. - The status endpoint fetched the catalogue on every call, so an open panel meant a remote request per poll. Cached for ten minutes, matching what the existing tessdata endpoint already does. Also closes the loop the installer opens: RuntimePathConfig now probes the machine-wide data directory, without which the elevated installer would write the engine to %PROGRAMDATA% and the application would never look there. And the service reads the note the installer leaves when its own download fails, so a proxy or a dropped connection during setup becomes an offer to retry instead of silence. Verified: tsc --noEmit clean, oxlint clean against the project config (which this branch could only run after the merge with main brought the oxlint migration), and the Java tests pass. The typecheck caught a real one - NavKey is a closed union and "ocr" had to be added to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Aikido flagged SSRF on the downloader and it was right. The install endpoints are deliberately not admin-gated - the desktop runs with login disabled, so gating them would make the feature unreachable where it is needed - which means that on a self-hosted server any user can trigger a fetch. A catalogue naming loopback or a cloud metadata address would turn that into a probe run by the server. Reuses the guard the project already has rather than writing another: the address checks behind GeneralUtils.isURLReachable are now also reachable on their own, for callers about to make the request themselves and with no use for an extra probe. A flat ban on private addresses would have broken the reason this feature exists. An air-gapped or corporate install points system.ocr.manifestUrl at an internal mirror whose artefacts sit on the same internal network; refusing those would leave exactly the users who cannot reach the internet unable to install OCR. So the rule follows the trust: a catalogue that is itself internal may name internal artefacts, and a public catalogue may not name anything that resolves inside. The first attempt at that got it wrong, and the test caught it. Asking isSensitiveHost whether the catalogue was internal conflates two questions: it answers "should I refuse to contact this?", and so reports true when a name does not resolve at all. A catalogue host with a DNS failure was therefore read as "an internal mirror" and switched the whole guard off - a resolution failure opening the hole the guard exists to close. It now says internal only when the host really does resolve inside. Two tests: a public catalogue may not send the server to loopback, and a local catalogue may name local artefacts, which is how a mirror works. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| return Files.newInputStream(Path.of(uri)); | ||
| } | ||
| HttpRequest request = | ||
| HttpRequest.newBuilder(uri) |
There was a problem hiding this comment.
HTTP request might enable SSRF attack - medium severity
If an attacker can control the URL input leading into this http request, the attack might be able to perform an SSRF attack. This kind of attack is even more dangerous is the application returns the result of the URL fetch to the user. It can serve as an initial access point for an attacker for stealing credentials in the cloud.
Show fix
Remediation: If possible, only allow requests to verified domains. If not, consult the article linked above to learn about other mitigating techniques such as disabling redirects, blocking private IPs and making sure private services have internal authentication. If you return data coming from the request to the user, validate the data before returning it to make sure you don't return random data.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
Good catch, and it was right about more than it could see. Fixed in f49578b4.
Two real holes, not one:
- The guard was at the download call sites, so
loadManifestwent straight past it — the catalogue URL itself was never checked. - The client followed redirects automatically, so a hop was taken after the check. A public catalogue could answer
302into the internal network and the guard would never see the real destination. Your remediation text names disabling redirects for exactly this reason.
Both are closed by moving the check into open(), the one place this class touches the network, and following redirects by hand with the guard re-applied to every hop. Refusing redirects outright was not an option — a GitHub release asset takes one — so it is a bounded manual loop instead.
The check reuses the address ranges already vetted in GeneralUtils rather than a second implementation. It is not a flat ban on private addresses, because that would break the case this feature exists for: an air-gapped or corporate install points system.ocr.manifestUrl at an internal mirror whose artefacts are on the same internal network. So the rule follows the trust — an internal catalogue may name internal artefacts, a public one may not name anything that resolves inside.
Worth recording that writing this introduced a bug the tests then caught: asking the existing helper whether the catalogue was internal conflates "resolves inside" with "cannot be resolved", so a DNS failure on the catalogue host read as "internal mirror" and disabled the guard entirely. That is now a separate check that only says internal when the host really does resolve inside.
Everything fetched is still verified against the SHA-256 the catalogue declares before anything is kept, and only https or a local file is accepted.
Aikido flagged the same finding again after the first fix, and it was right twice over. The guard sat at the download call sites, so loadManifest went straight past it: the catalogue URL itself was never checked. And the client followed redirects automatically, which meant a hop was taken *after* the check - a public catalogue could answer 302 into the internal network and the guard would never see the real destination. Aikido's own remediation text names disabling redirects for exactly this reason. Both are fixed by moving the check into open(), the one place this class reaches the network, and following redirects by hand with the guard re-applied to every hop. Refusing redirects outright was not an option: a GitHub release asset takes one, so that would have broken the download it exists to serve. Five hops, then it gives up. Two tests: a public catalogue may not send the server to loopback, and a catalogue host that does not resolve is treated as external rather than as a mirror. That second one is a bug the first attempt introduced and this test caught. Asking isSensitiveHost whether the catalogue was internal conflates two questions - it answers "should I refuse to contact this?" and so says yes when a name cannot be resolved at all. A catalogue with a DNS failure therefore read as "an internal mirror" and switched the whole guard off. A resolution failure must not open the hole the guard exists to close. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| for (int hop = 0; hop <= MAX_REDIRECTS; hop++) { | ||
| requireReachableFromServer(current); | ||
| HttpRequest request = | ||
| HttpRequest.newBuilder(current) |
There was a problem hiding this comment.
HTTP request might enable SSRF attack - medium severity
If an attacker can control the URL input leading into this http request, the attack might be able to perform an SSRF attack. This kind of attack is even more dangerous is the application returns the result of the URL fetch to the user. It can serve as an initial access point for an attacker for stealing credentials in the cloud.
Show fix
Remediation: If possible, only allow requests to verified domains. If not, consult the article linked above to learn about other mitigating techniques such as disabling redirects, blocking private IPs and making sure private services have internal authentication. If you return data coming from the request to the user, validate the data before returning it to make sure you don't return random data.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: the two concrete holes this rule helped find are fixed in f49578b4; what is left is the pattern itself, which is inherent to fetching a component described by a configurable manifest.
For a reviewer deciding whether that is reasonable, the mitigations now in place:
open()is the only place this class touches the network, and the guard runs at the top of every request, including the catalogue fetch.- Redirects are followed by hand with the guard re-applied to each hop, so a
302cannot smuggle a destination past the check. Bounded to five hops. - Only
httpsor a local file is accepted; plainhttpis refused, because it would let the manifest and the digests inside it be rewritten in the same breath. - Destinations are checked against the address ranges already vetted in
GeneralUtils, not a second implementation of the same idea. - Every artefact is verified against the SHA-256 the catalogue declares before anything is kept, and a mismatch installs nothing.
- Response bodies are written to a file and never returned to the caller, so this cannot be used as a read primitive.
The one thing deliberately allowed is an operator pointing system.ocr.manifestUrl at an internal mirror and that mirror naming internal artefacts — that is the air-gapped install case, and anyone who can edit settings.yml already controls the server. A public catalogue may not name anything that resolves inside.
Happy to be told this reasoning is wrong.
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
the two concrete holes this rule helped find are fixed in
f49578b4; what is left is the pattern itself, which is inherent to fetching a component described by a configurable manifest.
For a reviewer deciding whether that is reasonable, the mitigations now in place:
open()is the only place this class touches the network, and the guard runs at the top of every request, including the catalogue fetch.- Redirects are followed by hand with the guard re-applied to each hop, so a
302cannot smuggle a destination past the check. Bounded to five hops. - Only
httpsor a local file is accepted; plainhttpis refused, because it would let the manifest and the digests inside it be rewritten in the same breath. - Destinations are checked against the address ranges already vetted in
GeneralUtils, not a second implementation of the same idea. - Every artefact is verified against the SHA-256 the catalogue declares before anything is kept, and a mismatch installs nothing.
- Response bodies are written to a file and never returned to the caller, so this cannot be used as a read primitive.
The one thing deliberately allowed is an operator pointing system.ocr.manifestUrl at an internal mirror and that mirror naming internal artefacts — that is the air-gapped install case, and anyone who can edit settings.yml already controls the server. A public catalogue may not name anything that resolves inside.
Happy to be told this reasoning is wrong.
…stories Building the MSI for the first time turned up a real error in the WiX fragment: provisioning.wxs(126): error CNDL0037: The CustomAction/@win64 attribute can only be specified with one of the following attributes: Script, VBScriptCall, or JScriptCall present. Win64 is for script actions. A native DLL action takes its bitness from the package, which candle builds with -arch x64, so the attribute was both wrong and unnecessary. Nobody could have found this without building the installer; it would have failed CI for anyone who tried. The template fork compiles clean, which was the part worth worrying about: candle accepts main.wxs with the OCR dialog in it. That does not yet prove the dialog appears in the right place at run time - only msiexec can say that - but it rules out a malformed template. Stories for the panel, following the MSW pattern the LanguagePicker stories next door already use. The first draft stubbed globalThis.fetch, which would have done nothing: apiClient is axios, so nothing would have been intercepted and every story would have rendered an error. Four states: engine missing, choosing languages, catalogue unreachable, and no engine published for the platform. Measured while I was there, which answers the question that stalled this PR. Same machine, same JAR, same config, changing only the resources line: MSI without OCR 339.5 MB MSI with it embedded 382.2 MB delta 42.7 MB So 131.97 MB on disk costs 42.7 MB in the installer - a 32% ratio, not the "+97 MB" quoted earlier, which had compared an upstream MSI against a locally built NSIS package. Note these absolute numbers are not comparable to the published installer either: this JAR carries jpdfium for every platform, while release builds pass -PjpdfiumPlatforms=windows-x64. The delta is the honest figure because both sides were built identically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@Frooodle — updating this with the final numbers, now that the installer has been built and installed for real on a clean machine. Two figures I gave earlier were wrong, and both corrections make this smaller rather than larger. The "+97 MB" was wrong. It compared this project's published MSI against an NSIS package I had built locally — different formats, so the delta meant nothing. Sorry for the noise. The absolute figures I posted a few hours ago were also wrong, and that one was my build rather than anything in the project: I had built the JAR without What stands, all measured on one machine:
The 8.7 MB between the two MSIs is version drift — this branch is built against 2.14.2 and the published one is 2.14.3 — not this feature. The engine is fetched only when someone asks for it, at 37.5 MB. Where the weight actually sits, in case it is useful beyond this PR: The wizard page is no longer unprovenI flagged in the PR body that the installer page had never been through
An out-of-memory bug that predates this branchWorth reporting separately from the feature, because it affects everyone and is not a regression from here. Forcing OCR on an ordinary 19-page A4 document runs out of memory.
It now renders at 300 DPI — what Tesseract's own documentation asks for — clamped by |
Building the MSI and then reading its tables turned up a bug that no amount of
staring at the XML would have found: the OCR page was authored, compiled, and
unreachable.
An MSI is a database, so the ControlEvent table answers this directly:
InstallDirDlg | Next | NewDialog | OcrOptionsDlg | Order 3 | 1
InstallDirDlg | Next | NewDialog | VerifyReadyDlg | Order 4 | WIXUI_...
Control events run in ascending Order and the last NewDialog whose condition
holds is the one taken. The template's own license skip proves the rule - it
publishes InstallDirDlg at Order 2 against WixUI's LicenseAgreementDlg at Order 1
and wins. So Order 3 lost to Order 4 and the page was simply skipped, while
VerifyReadyDlg/Back at Order 3 *was* the highest there and would have gone back
to a dialog nobody ever reached.
Now Order 5 on Next, 3 on Back. The two differ deliberately and there is a
comment saying so, because making them match would break it again.
The condition matters as much as the number. It is copied from the publish this
overrides rather than left as "1": with an unconditional publish, an invalid
install path would advance to the OCR page anyway, stepping over the validation
InvalidDirDlg exists to enforce. That would have traded one bug for a worse one.
Also adds the write grant that the design promised and did not implement. The
application runs unelevated, so adding a language later has to work for an
ordinary user - but that argues for making the *data* directory writable, not the
one holding tesseract.exe. util:PermissionEx grants Users modify on tessdata and
nothing above it; granting the parent would let any user drop a DLL beside an
executable that later runs.
That needed an explicit component GUID. A component whose KeyPath is a directory,
which is what a bare CreateFolder produces, cannot have one generated (LGHT0230),
and it has to stay fixed or every upgrade would reapply the permissions from
scratch.
Verified by querying the rebuilt package rather than by installing it:
ControlEvent OcrOptionsDlg now Order 5, above VerifyReadyDlg at 4
SecureObjects OcrTessdataDir | CreateFolder | Users | -536805376
(GENERIC_READ|WRITE|EXECUTE|DELETE)
sequence SchedSecureObjects_x64 at 5801, rollback at 1801,
ExecSecureObjects_64 deferred
CustomAction InstallOcrRuntime type 3137 = DLL + deferred +
no-impersonate + continue-on-error
Binary OcrSetupBinary embedded
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Installing the built MSI and opening Settings showed a spinner that never resolved. The engine was there and working - the OCR tool listed its languages and the log confirmed the binary path - but GET /ui-data/ocr/runtime never returned. Curl against it hung past 45 seconds while the catalogue itself was reachable from the same machine in 0.3s. The HttpClient was built per call inside a try-with-resources and the response body handed back to the caller from inside that block. Since Java 21 HttpClient is AutoCloseable and close() blocks until every exchange has finished, so the body could not be read until close() returned and close() could not return until the body was read. One shared client, created once and never closed, which is how HttpClient is meant to be used anyway. The interesting part is why the tests said nothing. Every one of them serves its artefacts over file: URLs - deliberately, so they never touch the network - and that path returns at the top of open() without ever reaching the client. The logic was covered and the transport was not. So there are now two tests that stand up a real HTTP server: one reads a body back, one follows a redirect, which is how a GitHub release asset is served. Both are bounded by @timeout, because the failure mode is a hang rather than an error. The first version of the read test passed with the bug still in place. Its body was nineteen bytes, which arrives complete before anything can wait on it, so it proved nothing. At 2 MB - the real catalogue is 57 KB and the engine 37 MB - it fails with TimeoutException against the broken code and passes against the fix. A test that has only ever been seen to pass is not evidence, and this one had to be made worse-behaved before it was worth having. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…CR at 300 DPI Two problems a real installation exposed, and neither showed up in any test. The application could not see what its own installer had installed. There were two copies of the same decision: RuntimePathConfig.bundleRoots() knew about the machine-wide directory, which is why the engine *ran* fine, while OcrRuntimeService.runtimeRoot() looked only at the per-user one. So Settings reported "not installed" over a working installation and, when told to install, downloaded a second 122 MB copy beside the first. Both were on disk, identical, 78 seconds apart. That is the failure mode of duplicated knowledge rather than a typo: I fixed the resolver, saw the engine run, and called it done without checking whether anyone else held the same opinion separately. OcrRuntimeService now asks RuntimePathConfig, which is public for exactly this reason, and only picks a destination when there is genuinely nothing installed anywhere. Two tests pin it: the machine-wide root must be among the roots, and it must come last, so an install someone made for themselves beats one an administrator made for everyone. The second is older than this branch. Forcing OCR on an ordinary 19-page A4 document dies with an out-of-memory error on page 11. system.maxDPI is documented as "maximum allowed DPI" and defaults to 500; AutoRotateController reads that as a ceiling and clamps its own resolution to it, while OCRController adopted it as the target. At 500 DPI an A4 page is 24 megapixels, about 92 MB in memory, against the 2 GB heap the desktop app starts the backend with. OCR now renders at 300 - what Tesseract's own documentation asks for - clamped by maxDPI so lowering the setting still works. Above 300 the extra pixels cost memory and time and buy no accuracy. Worth being precise about blame: this is not a regression from this branch. The DPI code is byte-identical to two weeks ago, maxDPI has been 500 throughout, and -Xmx2g has not changed. It reproduces on a 19-page document and not on the one-page one used before, which is all that differed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Forcing OCR on a 19-page A4 document still ran out of memory at 300 DPI, after the previous commit brought it down from 500. The per-page cost was still 35 MB, and the heap trace showed which of the two possible shapes it was: a flat line and then a fall off a cliff, not a climb page after page. Nothing was leaking. One page simply asked for more than was left. PDFBox renders RGB by default. Tesseract converts to greyscale before it does anything else, so three quarters of that allocation is work that gets undone immediately. ImageType.GRAY makes an 8.7-megapixel A4 page cost 8.7 MB instead of 35, and the recognised text is unchanged, because the engine was going to throw the colour away regardless. The trace that found it stays, at DEBUG - a diagnostic, not something an operator wants nineteen lines of on every run. It answers the one question reading the code cannot: whether used memory climbs or spikes. The flush() after writing the PNG is not part of that saving and should not be read as if it were. The image is declared inside the loop body, so it already died each iteration; flush() releases the raster's cached data without waiting for the collector, which is worth doing on its own merits. The saving is entirely GRAY. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the full backend suite on a machine where the MSI had actually installed OCR failed two tests that pass everywhere else: expected: <tesseract> but was: <C:\ProgramData\Stirling-PDF\tesseract\tesseract.exe> The production code is right. Tesseract is resolved against the installer's bundle directories and one of those is machine-wide, so on a host that has installed OCR it correctly answers with a real path instead of a PATH lookup name. The tests asserted the bare name, which only holds while nobody has installed the feature - green in CI, red for anyone dogfooding it. The comment even said so out loud: "no Tesseract is bundled in a source checkout". The machine-wide directory is not in the checkout. So the assertion comes out of "defaults to bare command names", where tesseract never belonged: it is the one tool in that list resolved against an install layout, and BundledResources already covers that resolution against a simulated one. And the blank-path test now compares against the unset config rather than a literal, which is the claim it was making anyway - whitespace is indistinguishable from absence, whatever the fallback resolves to on this host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three things this branch got wrong about the project's conventions, all of them found by running the checks rather than by reading the diff again. Translations were added to es-ES as well as en-US. AGENTS.md is explicit that only en-US is edited by hand and every other language, en-GB included, is handled separately; the last fifteen commits on main that touch a locale file touch exactly one. Hand-edits to a Crowdin-managed file are a merge conflict waiting to happen, and en-US is the fallback anyway, so Spanish users see English until the translation arrives through the normal route rather than seeing nothing. The locale file was left unsorted. scripts/pre-commit/sort_locale_toml.py sorts tables case-insensitively, so [ocr.runtime] belongs after [ocr.results] and [settings.ocr] before [settings.planBilling], not where they read naturally to me. Both files were sorted before this branch touched them, so this was ours to fix, and `task pre-commit` would have failed on it. Two components were not Prettier-clean. Formatting only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
97 commits del original, hasta d3708c1 (version 2.14.3). La fusion sale limpia: ni un conflicto. El unico cambio del original que toca terreno de esta rama es el Stirling-Tools#6697, que anade la opcion rotatePages al OCR. Toca OCRController.java en zonas distintas de las nuestras -- el plumbing del parametro y el --rotate-pages de OcrMyPdf, frente al DPI de rasterizado que ajusta esta rama -- asi que git los combina sin intervencion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🌐 TOML Translation Verification Summary🔄 Reference Branch:
|
Description of Changes
Reworked from "bundle Tesseract in the installer" to "install it on demand", which
answers the question that stalled this PR: the size.
What changed. The Windows installer no longer carries a Tesseract runtime.
Instead the wizard asks whether text recognition is wanted and in which languages,
and fetches it while installing, with the progress bar moving. Languages can be
added or removed later from the OCR tool or from Settings, and take effect
immediately.
Why. The bundle was 131.97 MB on disk in every download, for a feature not
everyone uses — and 96.77 MB of that (73%) was a single file,
libtesseract-5.dll.Language models were only 16 MB of it. Packaged for download, the engine is
37.5 MB, and it ships in the installer not at all.
I owe a correction on my earlier answer here: the "+97 MB installer delta" I quoted
compared this project's published MSI against an NSIS package I had built
locally. Measured properly — same machine, same JAR, same config, changing only the
resourcesline — embedding the runtime costs 42.7 MB in the MSI, not 97.The installer this branch actually produces is 267.0 MB, against 258.3 MB for
the published one. The 8.7 MB between them is version drift (built against 2.14.2,
published is 2.14.3), not this feature: what this PR adds to the download is nothing
at all.
How components are found. The application has no download URL compiled into it.
It knows one address: a manifest listing, per platform, the engine and every
language model with its size and SHA-256. That indirection is deliberate — whoever
publishes the manifest decides which engine build installations receive, can
withdraw or replace a bad artefact without a new release of Stirling-PDF, and can
move the hosting anywhere.
system.ocr.manifestUrlmakes it configurable, which isalso what lets an air-gapped or corporate install point at an internal mirror.
Right now it points at a release on my fork; if you want this, point it at
yours and the control is entirely on your side.
Language models are catalogued straight from
tessdata_fastat a pinned commit, sothey are never rehosted — only hashed. 125 languages are offered, against the two
the bundle carried.
Safety, since this writes executable code next to the application:
SHA-256 is mandatory and verified before anything is moved into place (an artefact
listed without one is refused); archive entries are resolved against the target and
rejected if they climb out; https or a local file only, because plain http would
let the manifest and its digests be rewritten together; entry-count and
expanded-size ceilings; and the engine is expanded to a sibling directory and only
swapped in once complete, so a failed install leaves the previous state rather than
a runtime that exists but cannot run.
Three bugs found on the way, none of them from this branch
--tessdata-dirwas never passed to Tesseract (OCRController), whileAutoRotateControllerdid pass it. So the language dropdown was read fromtessDataPathwhile the engine resolvedtessdatanext to its own binary: settingsystem.tessdataDirmoved the list and left the OCR untouched. That is thebehaviour reported in #6534, where a user added
ita.traineddata, saw only Italianoffered, and still had no working Italian OCR.
Fixing it needed a guard, which is the second finding. Measured against Tesseract
5.4.0: pointing
--tessdata-dirat a directory of bare.traineddatafiles — thelayout the documentation tells users to assemble — makes the run print
read_params_file: Can't open pdf, write no output file, and still exit 0. Thepdfargument is the name of a config file read from<tessdata>/configs, not anoutput format. Since the code checks
rc != 0, that failure would pass unnoticed.So the flag is only passed when the directory carries
configs/pdf, andHowToUseOCR.mdnow warns about it.Forcing OCR on an ordinary 19-page A4 document ran out of memory.
system.maxDPIis documented as "maximum allowed DPI" and defaults to 500.AutoRotateControllerreads that as a ceiling and picks its own resolution under it;OCRControlleradopted it as the target. At 500 DPI an A4 page is 24 megapixels,roughly 92 MB, against the 2 GB heap the desktop app starts the backend with. On top
of that PDFBox renders RGB by default while Tesseract converts to greyscale before it
does anything else, so three quarters of that allocation was work being undone.
It now renders at 300 DPI — what Tesseract's own documentation asks for — clamped by
maxDPIso lowering the setting still works, and withImageType.GRAY: 8.7 MB perpage instead of 92, same recognised text. The DPI code is byte-identical to before
this branch and
-Xmx2ghas not changed, so this is not something the OCR workintroduced. Happy to lift it into its own PR if you would rather review it apart.
Notes for reviewers
/api/v1/ui-data, next to the existing tessdata ones,rather than
/api/v1/misc./api/v1/misc/is in the tool-model generator'sallow list, so a POST there would be published as a pipeline step, which none of
this is.
hasRole('ADMIN'): the desktop starts the backend withsecurity.enableLogin=false, so an admin-gated endpoint is unreachable exactlywhere this feature is needed — which is why the existing tessdata downloader
cannot be used there.
windows/wix/main.wxsis forked from tauri-bundler at@tauri-apps/cli v2.10.1because Tauri offers no way to add a dialog without replacing the template. The
provenance is recorded at the top of the file and the change is confined to one
block. This is the real maintenance cost of the wizard page and I am not hiding
it; happy to drop that page and keep only the in-app path if you would rather.
type EXE action runs out of process and never receives the MSIHANDLE, so it
cannot drive the progress bar.
Return="ignore"on that action is deliberate: a proxy wanting credentials or alaptop losing wifi mid-wizard should not roll back an otherwise good install. It
logs, leaves a note the app reads on first launch, and lets the install finish.
msiexec /i Stirling-PDF.msi /qn STIRLING_OCR=1 STIRLING_OCR_LANGS=spa,engscripts/counter_translation_v3.py:sync_files_v2.ymlregenerates the README table on main, and running it by hand would add a diff
across every language to this PR. Say the word if you would rather I did.
Closes #7257
Checklist
General
Documentation
Translations (if applicable)
scripts/counter_translation.pyUI Changes (if applicable)
Testing (if applicable)
task checkto verify linters, typechecks, and tests passWhat I actually ran, and what I did not
Precisely, since the box above is a single tick over several commands:
taskitself is not installed on this machine, so I ran what it delegates to. Backend
spotlessCheckclean; the Java suites pass — 3,598 inapp/core, 2,612 inapp/proprietary, andapp/commonclean apart from fourArchitectureTestfailures that are this machine, not the branch (the repository is on an SMB share
here and ArchUnit's importer reads zero classes over UNC; the same importer
reads 395 from a local copy, and the failure message is "failed to check any
classes", not a violation). Frontend:
tsc --noEmit,oxlint,stylelint,theme-lintandprettier --checkall clean, and the editor suite is running.Repo-wide:
whitespace,codespelland the locale TOML sort check pass. ThePython engine's checks I did not run — this branch does not touch
engine/.Running it caught three things worth naming, since the point of the gate is that
it catches what re-reading the diff does not:
Tesseract path was the bare command name, which stops being true the moment the
MSI actually installs OCR machine-wide — green in CI, red for anyone using the
feature. The assertion moved to where it belongs.
es-ESas well asen-US.AGENTS.mdisexplicit that only
en-USis hand-edited. Reverted.sort_locale_toml.py. Sorted.One gap remains, and I would rather name it than have a reviewer find it:
no screenshots yet for the Settings section, the tool dialog or the installer
page. They are owed.
The wizard page is no longer unproven. An earlier revision of this section
said the installer page had never been through
msiexec. It has now: the existingrelease was uninstalled and the MSI built from this branch installed in its place.
The OCR page appears where intended; the engine downloads during installation with
its SHA-256 verified before anything is moved into place;
configs/arrives withit; adding a language afterwards from Settings works and the application finds the
engine the installer left rather than downloading a second copy; the ACL grants
Modify to Users on
tessdataonly; and uninstalling removes the machine-wide copy.