Repository navigation
fix(pdf): bundle PDF.js CMap/font assets so CJK documents render and index - #131
Merged
Merged
Conversation
…index
PDF.js fetches predefined CMaps, standard fonts and the wasm image decoders as
external assets by filename (`${cMapUrl}${name}.bcmap`), and neither getDocument
call site passed any of the three. Nor did it fail loudly - PDF.js logs
Warning: loadFont - translateFont failed:
"UnknownErrorException: Ensure that the `cMapUrl` API parameter is provided."
and then extracts the document without its CJK runs. So a Chinese/Japanese PDF
rendered blank *and* indexed as if the CJK text did not exist, which is why chat
could not retrieve it (#125, #130).
- Ship pdfjs-dist/{cmaps,standard_fonts,wasm} as real files under
resources/pdfjs/ via electron-builder extraResources. They stay outside
app.asar deliberately: the main process reads them with fs, and Chromium
cannot read inside the archive.
- Serve them to the renderer through a new `knownote-asset://` protocol that
mirrors `knownote-doc://` - the renderer names a directory, the main process
resolves the file from an allowlisted root. No filesystem path reaches the
renderer.
- Pass cMapUrl + cMapPacked + standardFontDataUrl + wasmUrl in PdfLoader (fs
paths) and PdfSourceReader (protocol URLs). PDF.js requires a trailing slash
on each; `pdfjsAssetUrl()` owns that shape.
- CSP: allow `knownote-asset:` in connect-src, and 'wasm-unsafe-eval' in
script-src so the wasm decoders can compile. 'unsafe-eval' is still not
allowed, so the #121 scripting boundary is unchanged.
Verification (Linux, Electron 39.8.10):
- noembed-jis7.pdf through the real protocol handler in a headless Electron
renderer: without cMapUrl the text is "ABC 1231"; with it the text is
"ABC あいうえお 1231" and page.render() resolves. CMaps fetched over the
protocol: H.bcmap, Adobe-Japan1-UCS2.bcmap.
- The same fixture through the real PdfLoader in Node returns the Japanese text.
- npm test 276 pass (5 new: URL shape, traversal rejection, both call sites,
CSP), npm run typecheck, npm run build.
- npm run build:unpack ships cmaps 169 / standard_fonts 16 / wasm 13, and
smoke:packaged passes 21/21 including the new packaged-assets check.
Not verified: a CJK document end to end in the packaged window (canvas paint,
text-layer selection). That needs a real CJK fixture and a display.
mrsibe
force-pushed
the
fix/pdfjs-cmap-assets
branch
from
September 25, 2026 17:22
97bff1c to
b5286bb
Compare
This was referenced Sep 25, 2026
Closed
mrsibe
added a commit
that referenced
this pull request
Sep 25, 2026
`protocol.registerSchemesAsPrivileged()` replaces the whole privilege table
instead of appending to it. `registerDocumentScheme()` and
`registerPdfjsAssetScheme()` each called it once, so the second call silently
dropped `knownote-doc`'s `supportFetchAPI` privilege. The renderer's
`fetch('knownote-doc://docs/<id>')` then failed with
Fetch API cannot load knownote-doc://docs/...: URL scheme "knownote-doc" is not supported
and every PDF failed to open. Introduced by #131.
Protocol modules now only declare their scheme
(`declareDocumentScheme()` / `declarePdfjsAssetScheme()`); the single call to
Electron lives in `privilegedSchemes.ts::applyPrivilegedSchemes()`, invoked once
from `index.ts` before the app is ready.
`smokeTest` gains a renderer-side fetch of `knownote-doc://`. The existing check
used the main process's `net.fetch`, which works even without the privilege -
that is why the packaged gate stayed green while the reader was broken. The
renderer check creates a hidden window and deliberately does not destroy it:
destroying the last window fires `window-all-closed`, and this app closes the
database on that event, which would abort the remaining checks.
Verification (Linux, Electron 39.8.10):
- headless Electron running the repo's real protocol modules: both
`knownote-doc://` (15306 bytes) and `knownote-asset://` (553 bytes) fetch from a
renderer. With two separate `registerSchemesAsPrivileged()` calls the same
harness returns "ERR Failed to fetch" for `knownote-doc://` - the regression,
reproduced.
- `npm test` 283 pass (2 new: one submission point, all declarations applied).
- `npm run typecheck`, `npm run build`.
- `npm run build:unpack` + `smoke:packaged` 22/22, including the new renderer
check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Makes CJK (Chinese / Japanese / Korean) PDFs work end to end by wiring up PDF.js's three external asset roots — predefined CMaps, standard fonts and the wasm image decoders — for both consumers:
PdfLoader) reads them as filesystem paths, sogetTextContent()returns the CJK text instead of dropping it;PdfSourceReader) reads them through a newknownote-asset://protocol, so the canvas paints CJK and the text layer is selectable.Ships
pdfjs-dist/{cmaps,standard_fonts,wasm}with the app as real files underresources/pdfjs/(electron-builderextraResources, outsideapp.asar).Why?
The v1.4 QA (#125) found CJK PDFs broken in a way that a
getDocument({ data })call hides:PDF.js treats CMaps / standard fonts / wasm as external assets fetched by filename (
${cMapUrl}${name}.bcmap). Nothing passed any of the three, and the failure is a warning, not an exception — so the reader painted blank and the loader indexed the document without its CJK text. Both are release blockers: the second is a plausible part of why notebook chat could not retrieve CJK sources.Reproduced against
mainwithnoembed-jis7.pdf(non-embedded Japanese font):cMapUrl"ABC 1231"cMapUrl+cMapPacked"ABCあいうえお123 1"Fixes #130. Related to #125, #129, #82.
What changed?
electron-builder.yml:extraResourcescopiescmaps,standard_fonts,wasmtoresources/pdfjs/. Real files, outside the asar — the main process reads them withfs, and Chromium cannot read insideapp.asar.src/shared/utils/pdfjsAssets.ts: the shared contract (directory allowlist, protocol URL shape with the trailing slash PDF.js requires, and a parser that rejects traversal).src/main/protocol/pdfjsAssetProtocol.ts:knownote-asset://, mirroringknownote-doc://. The renderer names a directory; the main process resolves the file from an allowlisted root and serves it with CORS +application/wasm.src/main/pdfjsAssetPaths.ts: resolves the asset root (resources/pdfjspackaged,node_modules/pdfjs-distin dev) and the three Node-side directory paths.PdfLoader.ts/PdfSourceReader.tsx: passcMapUrl,cMapPacked: true,standardFontDataUrl,wasmUrl.index.html:connect-srcgainsknownote-asset:;script-srcgains'wasm-unsafe-eval'.'unsafe-eval'is still absent, so the security(pdf): remediate GHSA-hq66-cqwq-w95j before v1.4 (pdfjs-dist 5.7.284 in affected range) #121 scripting boundary is unchanged.smokeTest.ts: packaged assertion that the three asset directories shipped and are non-empty.test/pdfjsAssets.test.ts: URL shape, traversal rejection, both call sites, and the CSP directives.How was this tested?
39.8.10), againstnoembed-jis7.pdf, with the repo's actualregisterPdfjsAssetScheme/registerPdfjsAssetProtocolHandler: text is"ABC あいうえお 1231"andpage.render()resolves. CMaps fetched over the protocol:H.bcmap,Adobe-Japan1-UCS2.bcmap.PdfLoaderin Node on the same fixture returns the Japanese text; withoutcMapUrlit returns"ABC 1231"and emits the warning above.npm test— 276 pass, 0 fail (5 new). The call-site guard was falsified by revertingcMapUrlin the reader, which fails it as intended.npm run typecheck,npm run build— pass.npm run build:unpack— shipscmaps169,standard_fonts16,wasm13 underresources/pdfjs/.smoke:packaged— 21/21 checks pass, including the new packaged-assets check (run headless; CI usesxvfb-run).Not verified: a CJK document end to end in the packaged window (canvas paint, text-layer selection). That needs a real CJK fixture and a display, and is manual QA for #125.
Screenshots / recordings
Not applicable (no bundled CJK fixture to screenshot), see "How was this tested?".
Checklist
npm run typecheckpasses.npm run buildpasses.Desktop / build changes
npm run build:unpackpasses.npm run smoke:packagedpasses.