diff --git a/assets/js/elp-upload.js b/assets/js/elp-upload.js index aa7e8b2..ea31c94 100644 --- a/assets/js/elp-upload.js +++ b/assets/js/elp-upload.js @@ -573,7 +573,9 @@ // allow-modals so the preview cannot raise "Leave site?" // dialogs. (Isolating untrusted content in a separate origin // is tracked as follow-up; see the proxy CSP for mitigation.) - sandbox: 'allow-scripts allow-same-origin allow-popups', + // allow-downloads lets the package's own .elpx download + // button save its file (exelearning/exelearning#2488). + sandbox: 'allow-scripts allow-same-origin allow-popups allow-downloads', style: { width: '100%', height: '100%', diff --git a/assets/js/wp-exe-download.js b/assets/js/wp-exe-download.js index 97793b6..af945bc 100644 --- a/assets/js/wp-exe-download.js +++ b/assets/js/wp-exe-download.js @@ -349,12 +349,68 @@ } ); } + /** + * Point the package's own "Download .elpx" button at the original upload. + * + * The download-source-file iDevice's inline onclick calls the package's + * global downloadElpx(), which refetches every file of the package and + * rebuilds the ZIP in the browser. Inside the embed that never saves a file: + * the content CSP blocks the blob: workers fflate compresses in, and the + * sandbox drops downloads (exelearning/exelearning#2488). Packages exported + * before exelearning/exelearning#2196 also rebuild without content.xml. The + * toolbar already offers the original .elpx, so serve that instead. + * + * Only while this embed's toolbar offers an enabled .elpx item, and only + * while the frame is same-origin: with a cross-origin content origin the + * access throws and the package keeps its own download. + * + * @param {Element} frame Element whose load event fired. + */ + function routeContentDownload( frame ) { + if ( ! frame || frame.tagName !== 'IFRAME' || ! frame.classList.contains( 'exelearning-iframe' ) ) { + return; + } + var embed = frame.closest( '.exelearning-preview, .exelearning-block-frontend' ); + var container = embed && embed.querySelector( '.exelearning-download[data-elp-url]' ); + var item = container && container.querySelector( '[data-format="elpx"]:not(.exelearning-download__item--disabled)' ); + if ( ! item ) { + return; + } + var params = { + format: 'elpx', + suffix: item.getAttribute( 'data-suffix' ) || '.elpx', + attachmentId: parseInt( container.getAttribute( 'data-attachment-id' ), 10 ), + elpUrl: container.getAttribute( 'data-elp-url' ), + slug: container.getAttribute( 'data-slug' ), + container: container, + }; + try { + var win = frame.contentWindow; + if ( ! win || typeof win.downloadElpx !== 'function' ) { + return; + } + win.downloadElpx = function() { + return downloadFormat( params ); + }; + } catch ( e ) { + // Cross-origin frame: leave the package's own download in place. + } + } + + // `load` does not bubble, but it is dispatched through the capture phase, so + // one listener sees every embed iframe, including later in-frame navigations. + document.addEventListener( 'load', function( event ) { + routeContentDownload( event.target ); + }, true ); + // Public API so the block editor can reuse the exact same export pipeline. window.wpExeDownload = { downloadFormat: downloadFormat, }; function init() { + // Frames that finished loading before this script ran. + Array.prototype.forEach.call( document.querySelectorAll( 'iframe.exelearning-iframe' ), routeContentDownload ); document.addEventListener( 'click', onClick ); document.addEventListener( 'keydown', function( e ) { if ( e.key === 'Escape' ) { diff --git a/docs/architecture/adr/ADR-156-01-let-the-package-download-button-work-in-embeds.md b/docs/architecture/adr/ADR-156-01-let-the-package-download-button-work-in-embeds.md new file mode 100644 index 0000000..61f6348 --- /dev/null +++ b/docs/architecture/adr/ADR-156-01-let-the-package-download-button-work-in-embeds.md @@ -0,0 +1,158 @@ +--- +id: ADR-156-01 +title: "Let the package's own .elpx download button work in embeds" +status: Proposed +date: 2026-09-29 +tracking_issue: 156 +deciders: + - "@erseco" + - "claude-code" +related: + prs: [156] + changes: [] + adrs: [] +external_refs: + - "https://github.com/exelearning/exelearning/issues/2488" + - "https://github.com/exelearning/exelearning/pull/2489" + - "https://github.com/exelearning/exelearning/pull/2196" + - "https://github.com/exelearning/omeka-s-exelearning/pull/63" +supersedes: [] +superseded_by: [] +ai_assistance: + tool: "Claude Code" + model: "claude-opus-5-5" +--- + +# ADR-156-01: Let the package's own .elpx download button work in embeds + +## Context + +eXeLearning packages can contain a *download-source-file* iDevice: a "Download +.elpx" button inside the content. Its inline `onclick` calls the package's global +`downloadElpx()` (`libs/exe_elpx_download/exe_elpx_download.js`). That function +refetches every file listed in `libs/elpx-manifest.js`, zips the files in the +browser with fflate, and clicks an `` inside the content document. + +In an embed, that button never saved a file. It failed in two independent places: + +1. **CSP.** `ExeLearning_Content_Proxy` served HTML with `script-src 'self' + 'unsafe-inline' 'unsafe-eval'` and no `worker-src`. The async `fflate.zip()` + starts `blob:` workers and listens only for their messages. The workers are + blocked, the callback never fires, and the button stays at "Processing... + 100%" (exelearning/exelearning#2488). +2. **Sandbox.** The block, shortcode, Media Library and Gutenberg preview iframes + use `sandbox="allow-scripts allow-same-origin allow-popups"`. Without + `allow-downloads`, Chrome drops any download the frame starts. + +Rebuilding is also a poor substitute for the original upload. It refetches the +whole package and holds it in memory two to three times over. Packages exported +before exelearning/exelearning#2196 rebuild without `content.xml`, so the result +cannot be re-imported. + +## Problem + +How should the package's own "Download .elpx" button behave in a WordPress +embed? The answer must cover packages that are already published, whose script +cannot be changed. + +## Decision drivers + +- Fix content that is already published, without re-uploading it. +- Give users the same file the toolbar already offers, byte for byte. +- Respect embeds that do not offer the `.elpx` (`show_download` off, the + default). +- Do not widen what package JavaScript can do in the site origin. +- Keep working under a cross-origin `exelearning_content_origin`. + +## Alternatives considered + +### Option 1: Wait for the upstream script fix + +exelearning/exelearning#2489 adds a `zipSync` fallback when workers are blocked. +It helps only packages exported after it ships, and the sandbox still drops the +download. + +### Option 2: Relax the CSP and the sandbox only + +Add `worker-src 'self' blob:` and `allow-downloads`. The rebuild then completes, +but it still refetches the whole package and may omit `content.xml`. + +### Option 3: Serve the original from the parent, and relax the CSP and the sandbox as a fallback + +While an embed's toolbar offers an enabled `.elpx` item, `wp-exe-download.js` +replaces the package's `downloadElpx()` with a function that downloads the +original attachment through the toolbar's own `downloadFormat()`. Everywhere else +the package's own rebuild runs, and Option 2's relaxations let it finish. + +## Evidence + +- The same CSP and sandbox reproduce on the Omeka S module: 34 `worker-src + blob:` violations and a button stuck at 100 %. With the fixed script, Chrome + logged "Download is disallowed … 'allow-downloads' is not set" + (exelearning/omeka-s-exelearning#63). +- With the routing in place on that site, the in-content button downloaded the + original, with the same SHA-256 as the upload. +- fflate 0.8.3 `zip()` starts a worker for each compressible file of 160 kB or + more, and its browser worker wrapper sets only `onmessage`. +- `includes/class-content-proxy.php`, `includes/class-elp-upload-block.php`, + `public/class-shortcodes.php`, `includes/integrations/class-media-library.php` + and `assets/js/elp-upload.js` at `7ec2e82`. + +## Decision + +We will take Option 3: + +- `assets/js/wp-exe-download.js` routes `downloadElpx()` per embed, only while + that embed offers an enabled `.elpx` item. A capturing `load` listener + reapplies it on each page the frame navigates to. A cross-origin frame raises + an access error, which is caught. +- HTML content gets `worker-src 'self' blob:`. +- The embed iframes get `allow-downloads`. + +Neither relaxation grants new capability. Package scripts already run with +`'unsafe-inline'` and `'unsafe-eval'` in the site origin and can start downloads +through the same-origin parent. + +## Consequences + +### Positive + +- The in-content button works for every existing package: it gives the + original when the toolbar offers it, and a completed rebuild otherwise. +- There is no full refetch or in-memory rebuild in the common case. + +### Negative + +- The routing reaches into package globals. If a future eXeLearning renames + `downloadElpx`, the routing stops, and the package's own download still works. + +### Neutral + +- No change to the script-free SVG/XML policy, to extraction, or to the + `.htaccess` rules. + +## Risks + +- A package could redefine `downloadElpx` after `load`. Its own download would + then run instead of the original. Low severity. +- Moving content to an opaque or cross-origin frame in the future must keep + `allow-downloads` and a `worker-src blob:` allowance, or the fallback breaks. + +## Validation + +- `tests/js/wp_exe_download.test.js` covers the routing, including scoping, + disabled or missing `.elpx`, navigation, a cross-origin frame and several + embeds on one page. +- PHPUnit shortcode, block and Media Library tests pin `allow-downloads`. +- Manual check with a package that contains the download-source-file iDevice, + with `show_download` on and off. + +## Follow-up work + +- None required. Remove the routing if eXeLearning adds an explicit + host-download protocol. + +## References + +- exelearning/exelearning#2488, #2489 and #2196 +- exelearning/omeka-s-exelearning#63 (same fix, with a live reproduction) diff --git a/includes/class-content-proxy.php b/includes/class-content-proxy.php index d4533ab..4be0897 100644 --- a/includes/class-content-proxy.php +++ b/includes/class-content-proxy.php @@ -615,6 +615,11 @@ private function send_headers( $mime_type, $file_size ) { array( "default-src 'self'", "script-src 'self' 'unsafe-inline' 'unsafe-eval'", + // The package's download-source-file button rebuilds the .elpx + // with fflate, which compresses in blob: workers. Scripts here + // already run with 'unsafe-inline'/'unsafe-eval', so this adds + // no capability (exelearning/exelearning#2488). + "worker-src 'self' blob:", "style-src 'self' 'unsafe-inline'", "img-src 'self' data: blob: https:", "media-src 'self' data: blob: https:", diff --git a/includes/class-elp-upload-block.php b/includes/class-elp-upload-block.php index ba73525..dedd112 100644 --- a/includes/class-elp-upload-block.php +++ b/includes/class-elp-upload-block.php @@ -381,7 +381,7 @@ class="exelearning-iframe" style="width: 100%%; height: %dpx; border: 1px solid #ddd; border-radius: 4px;" title="%s" loading="lazy" - sandbox="allow-scripts allow-same-origin allow-popups" + sandbox="allow-scripts allow-same-origin allow-popups allow-downloads" referrerpolicy="no-referrer" >', $this->build_preview_url( $data ), diff --git a/includes/integrations/class-media-library.php b/includes/integrations/class-media-library.php index 7862dff..d832c3e 100644 --- a/includes/integrations/class-media-library.php +++ b/includes/integrations/class-media-library.php @@ -321,7 +321,7 @@ public function render_preview_meta_box( $post ) { $preview_url = ExeLearning_Content_Proxy::get_proxy_url( $directory ); echo '
'; - echo ''; + echo ''; echo '
'; echo '

' . esc_html__( 'Open in new tab', 'exelearning' ) . '

'; } else { diff --git a/public/class-shortcodes.php b/public/class-shortcodes.php index be3c194..99f7590 100644 --- a/public/class-shortcodes.php +++ b/public/class-shortcodes.php @@ -366,7 +366,7 @@ class="exelearning-iframe" title="%s" loading="lazy" allow="fullscreen" - sandbox="allow-scripts allow-same-origin allow-popups" + sandbox="allow-scripts allow-same-origin allow-popups allow-downloads" referrerpolicy="no-referrer" >', $iframe_src_attr, diff --git a/tests/js/wp_exe_download.test.js b/tests/js/wp_exe_download.test.js index c03e27e..dc8482b 100644 --- a/tests/js/wp_exe_download.test.js +++ b/tests/js/wp_exe_download.test.js @@ -789,3 +789,124 @@ describe( 'wp-exe-download: exports that never finish', () => { expect( objectUrls[ 0 ].revoked ).toBe( true ); } ); } ); + +// The download-source-file iDevice inside a package has its own "Download .elpx" +// button. Its inline onclick calls the package's global downloadElpx(), which refetches +// every file and rebuilds the ZIP in the browser -- and, under the content CSP and the +// iframe sandbox, never saves anything (exelearning/exelearning#2488). While the embed +// offers the .elpx, the script points that function at the original attachment instead. +describe( 'wp-exe-download: routing the package\'s own .elpx button', () => { + /** + * Render an embed: toolbar with the split button, and the content iframe. + * + * @param {Object} [options] + * @param {string} [options.wrapperClass] Embed wrapper class. + * @param {boolean} [options.offerElpx] Whether the toolbar offers the .elpx. + * @param {boolean} [options.disabled] Whether that item is disabled. + * @param {string} [options.slug] Download slug. + * @return {HTMLIFrameElement} The content iframe. + */ + function renderEmbed( { wrapperClass = 'exelearning-shortcode exelearning-preview', offerElpx = true, disabled = false, slug = 'project' } = {} ) { + const item = offerElpx + ? 'Download' + : ''; + const wrapper = document.createElement( 'div' ); + wrapper.className = wrapperClass; + wrapper.innerHTML = + '
` + item + '
' + + ''; + document.body.appendChild( wrapper ); + return wrapper.querySelector( 'iframe' ); + } + + /** Give the iframe a package window and fire its load event. */ + function loadPackage( iframe, win ) { + Object.defineProperty( iframe, 'contentWindow', { configurable: true, get: () => win } ); + iframe.dispatchEvent( new window.Event( 'load' ) ); + return win; + } + + function packageWindow() { + const rebuild = vi.fn(); + return { downloadElpx: rebuild, rebuild }; + } + + it( 'downloads the original attachment when the package button is clicked', async () => { + const iframe = renderEmbed(); + const blob = { size: 10 }; + window.fetch = vi.fn( () => Promise.resolve( { ok: true, blob: () => Promise.resolve( blob ) } ) ); + const win = loadPackage( iframe, packageWindow() ); + + win.downloadElpx(); + await settle(); + + expect( win.rebuild ).not.toHaveBeenCalled(); + expect( window.fetch ).toHaveBeenCalledWith( 'http://example.test/uploads/project.elpx', { credentials: 'same-origin' } ); + expect( downloads ).toEqual( [ { href: 'blob:mock/0', download: 'project.elpx' } ] ); + } ); + + it( 'routes the package button in a block embed too', () => { + const iframe = renderEmbed( { wrapperClass: 'wp-block-exelearning-elp-upload exelearning-block-frontend' } ); + const win = loadPackage( iframe, packageWindow() ); + + expect( win.downloadElpx ).not.toBe( win.rebuild ); + } ); + + it( 'routes every page the frame navigates to', () => { + const iframe = renderEmbed(); + loadPackage( iframe, packageWindow() ); + const next = loadPackage( iframe, packageWindow() ); + + expect( next.downloadElpx ).not.toBe( next.rebuild ); + } ); + + it( 'keeps the package\'s own download when the embed does not offer the .elpx', () => { + const iframe = renderEmbed( { offerElpx: false } ); + const win = loadPackage( iframe, packageWindow() ); + + expect( win.downloadElpx ).toBe( win.rebuild ); + } ); + + it( 'keeps the package\'s own download when the .elpx item is disabled', () => { + const iframe = renderEmbed( { disabled: true } ); + const win = loadPackage( iframe, packageWindow() ); + + expect( win.downloadElpx ).toBe( win.rebuild ); + } ); + + it( 'leaves packages without the download button alone', () => { + const iframe = renderEmbed(); + const win = loadPackage( iframe, {} ); + + expect( win.downloadElpx ).toBeUndefined(); + } ); + + it( 'leaves a cross-origin frame alone', () => { + const iframe = renderEmbed(); + Object.defineProperty( iframe, 'contentWindow', { + configurable: true, + get: () => ( { + get downloadElpx() { + throw new window.DOMException( 'Blocked a frame with origin', 'SecurityError' ); + }, + } ), + } ); + + expect( () => iframe.dispatchEvent( new window.Event( 'load' ) ) ).not.toThrow(); + } ); + + it( 'serves each embed its own attachment', async () => { + renderEmbed( { slug: 'first' } ); + const second = renderEmbed( { slug: 'second' } ); + window.fetch = vi.fn( () => Promise.resolve( { ok: true, blob: () => Promise.resolve( {} ) } ) ); + const win = loadPackage( second, packageWindow() ); + + win.downloadElpx(); + await settle(); + + expect( window.fetch ).toHaveBeenCalledWith( 'http://example.test/uploads/second.elpx', { credentials: 'same-origin' } ); + expect( downloads[ 0 ].download ).toBe( 'second.elpx' ); + } ); +} ); diff --git a/tests/unit/ElpUploadBlockTest.php b/tests/unit/ElpUploadBlockTest.php index e22f081..46d4de4 100644 --- a/tests/unit/ElpUploadBlockTest.php +++ b/tests/unit/ElpUploadBlockTest.php @@ -178,6 +178,8 @@ public function test_iframe_has_sandbox() { $this->assertStringContainsString( 'sandbox=', $result ); $this->assertStringContainsString( 'allow-scripts', $result ); + // The package's own .elpx download button must be able to save its file. + $this->assertStringContainsString( 'allow-downloads', $result ); } /** diff --git a/tests/unit/MediaLibraryTest.php b/tests/unit/MediaLibraryTest.php index 5b314c0..e8c95d6 100644 --- a/tests/unit/MediaLibraryTest.php +++ b/tests/unit/MediaLibraryTest.php @@ -202,6 +202,7 @@ public function test_render_preview_meta_box_with_preview() { $this->assertStringContainsString( 'assertStringContainsString( 'sandbox=', $output ); + $this->assertStringContainsString( 'allow-downloads', $output ); $this->assertStringContainsString( 'referrerpolicy="no-referrer"', $output ); } diff --git a/tests/unit/ShortcodesTest.php b/tests/unit/ShortcodesTest.php index fc4cbc6..6eb6aba 100644 --- a/tests/unit/ShortcodesTest.php +++ b/tests/unit/ShortcodesTest.php @@ -366,6 +366,20 @@ public function test_iframe_sandbox_allows_popups() { $this->assertStringContainsString( 'allow-popups', $result ); } + /** + * Test iframe sandbox lets the package's own .elpx download button save its file. + */ + public function test_iframe_sandbox_allows_downloads() { + $attachment_id = $this->factory->attachment->create(); + $hash = str_repeat( 'a', 40 ); + update_post_meta( $attachment_id, '_exelearning_extracted', $hash ); + update_post_meta( $attachment_id, '_exelearning_has_preview', '1' ); + + $result = $this->shortcodes->display_exelearning( array( 'id' => $attachment_id ) ); + + $this->assertStringContainsString( 'sandbox="allow-scripts allow-same-origin allow-popups allow-downloads"', $result ); + } + /** * Test wrapper has exelearning-shortcode class. */