fix(pdf): serve embedpdf runtime assets from same origin#3941
Conversation
EmbedPDF fetched the Pdfium wasm, the default stamp library and its glyph-fallback fonts from jsDelivr, which the production CSP blocks. Copy them into build/static (served by the public /static route) and point the viewer at those same-origin paths, and disable the Google Fonts UI/signature webfonts. To stay under the registry size limit, only the Latin Regular and Bold fallback fonts ship (CJK is omitted) and the bundled Pdfium wasm copy rspack emits from `new URL` is dropped before emit, since the engine loads the wasm from the copied file instead.
Crash--
requested review from
JF-Cozy,
doubleface,
lethemanh,
rezk2ll and
zatteo
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR self-hosts EmbedPDF runtime assets to eliminate external CDN dependencies. Six new Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
BundleMonFiles updated (1)
Unchanged files (19)
Total files change +5.44KB +0.27% Groups updated (1)
Unchanged groups (2)
Final result: ✅ View report in BundleMon website ➡️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rsbuild.config.mjs (1)
160-162: Add cardinality guard to WASM deletion hook to future-proof against multiple WASM sources.The deletion pattern at line 161 targets
static/wasm/*.module.wasmbroadly, not specifically by pdfium hash or metadata. Currently, only@embedpdf/pdfiumuses thenew URL('...')pattern that triggers rspack's asyncWebAssembly bundling to this location. However, if another dependency later emits WASM through the same mechanism, its JS reference would remain but the asset would be silently deleted at runtime, causing 404 errors.Harden the hook with cardinality assertion:
Suggested fix
- for (const name of Object.keys(compilation.assets)) { - if (/static[\\/]wasm[\\/].*\.module\.wasm$/.test(name)) { - delete compilation.assets[name] - } - } + const bundledWasmAssets = Object.keys(compilation.assets).filter( + name => /static[\\/]wasm[\\/].*\.module\.wasm$/.test(name) + ) + + if (bundledWasmAssets.length > 1) { + throw new Error( + `Refusing to delete multiple bundled WASM assets: ${bundledWasmAssets.join(', ')}` + ) + } + + for (const name of bundledWasmAssets) { + compilation.deleteAsset?.(name) ?? delete compilation.assets[name] + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rsbuild.config.mjs` around lines 160 - 162, The WASM deletion hook in the loop that iterates over Object.keys(compilation.assets) uses a broad regex pattern to delete files matching `/static[\\/]wasm[\\/].*\.module\.wasm$` without verifying the expected cardinality. Add a cardinality assertion before or after the deletion loop to verify that exactly one WASM file matches the pattern, ensuring that if multiple WASM sources are added in the future, the hook will fail safely rather than silently deleting unexpected assets. This can be done by counting the matches against the regex pattern and asserting that the count equals the expected value (typically 1).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@rsbuild.config.mjs`:
- Around line 160-162: The WASM deletion hook in the loop that iterates over
Object.keys(compilation.assets) uses a broad regex pattern to delete files
matching `/static[\\/]wasm[\\/].*\.module\.wasm$` without verifying the expected
cardinality. Add a cardinality assertion before or after the deletion loop to
verify that exactly one WASM file matches the pattern, ensuring that if multiple
WASM sources are added in the future, the hook will fail safely rather than
silently deleting unexpected assets. This can be done by counting the matches
against the regex pattern and asserting that the count equals the expected value
(typically 1).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c59b386-6cf7-455e-bfd1-e33e14b732f8
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.jsonrsbuild.config.mjssrc/modules/views/Pdf/PdfEditor.jsxsrc/modules/views/Pdf/pdfAssets.js
The Pdfium engine and glyph fallback run inside a blob: Web Worker
whose base URL is the blob itself, so the root-relative
`/static/pdfium.wasm` could not be resolved there ("Failed to parse
URL") and the editor hung forever on "Initializing plugins". Anchor the
asset URLs to window.location.origin so they resolve in the worker as
well as on the main thread.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
|
You can test with this branch, it works git://github.com/cozy/cozy-drive.git#build-qv-210626 |
2 participants
Summary
build/staticand point the PDF editor at those same-origin paths, so they no longer load from jsDelivr (blocked by the CSP).Summary by CodeRabbit