From 54d1d9fabc34fac8396669abdd57a8ed87383858 Mon Sep 17 00:00:00 2001 From: Andreas Farre Date: Mon, 23 Jun 2025 14:00:29 +0000 Subject: [PATCH] Bug 1972100 - Only block sandboxed pdfs handled internally. r=smaug a=dmeehan Differential Revision: https://phabricator.services.mozilla.com/D254662 --- uriloader/base/nsURILoader.cpp | 3 +- .../exthandler/tests/mochitest/browser.toml | 6 +- .../mochitest/browser_pdf_sandboxed_iframe.js | 71 ++++++++++++++++--- .../file_pdf_content_disposition.pdf | 0 .../file_pdf_content_disposition.pdf^headers^ | 1 + 5 files changed, 70 insertions(+), 11 deletions(-) create mode 100644 uriloader/exthandler/tests/mochitest/file_pdf_content_disposition.pdf create mode 100644 uriloader/exthandler/tests/mochitest/file_pdf_content_disposition.pdf^headers^ diff --git a/uriloader/base/nsURILoader.cpp b/uriloader/base/nsURILoader.cpp index c4c93baae075..7b77b667ade7 100644 --- a/uriloader/base/nsURILoader.cpp +++ b/uriloader/base/nsURILoader.cpp @@ -436,8 +436,7 @@ nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request) { // Check if this is a PDF which should be opened internally. We also handle // octet-streams that look like they might be PDFs based on their extension. - if ((maybeForceInternalHandling || IsSandboxed(aChannel)) && - IsContentPDF(aChannel, mContentType)) { + if (maybeForceInternalHandling && IsContentPDF(aChannel, mContentType)) { // For a PDF, check if the preference is set that forces attachments to be // opened inline. If so, treat it as a non-attachment by clearing // 'forceExternalHandling' again. This allows it open a PDF directly diff --git a/uriloader/exthandler/tests/mochitest/browser.toml b/uriloader/exthandler/tests/mochitest/browser.toml index 094e8452579f..d4585d805c55 100644 --- a/uriloader/exthandler/tests/mochitest/browser.toml +++ b/uriloader/exthandler/tests/mochitest/browser.toml @@ -119,7 +119,11 @@ support-files = [ ] ["browser_pdf_sandboxed_iframe.js"] -support-files = ["file_pdf.pdf"] +support-files = [ + "file_pdf.pdf", + "file_pdf_content_disposition.pdf", + "file_pdf_content_disposition.pdf^headers^", +] ["browser_pdf_save_as.js"] diff --git a/uriloader/exthandler/tests/mochitest/browser_pdf_sandboxed_iframe.js b/uriloader/exthandler/tests/mochitest/browser_pdf_sandboxed_iframe.js index 56c557b586dd..f45998047715 100644 --- a/uriloader/exthandler/tests/mochitest/browser_pdf_sandboxed_iframe.js +++ b/uriloader/exthandler/tests/mochitest/browser_pdf_sandboxed_iframe.js @@ -117,10 +117,10 @@ async function onDownload(test) { ok(true, "Download finished"); } -async function onBlockedBySandbox(test) { - const expected = `Download of “${TEST_PATH}file_pdf.pdf” was blocked because the triggering iframe has the sandbox flag set.`; +async function onBlockedBySandbox(test, file) { + const expected = `Download of “${TEST_PATH}${file}” was blocked because the triggering iframe has the sandbox flag set.`; return new Promise(resolve => { - Services.console.registerListener(function onMessage(msg) { + Services.console.registerListener(async function onMessage(msg) { let { message, logLevel } = msg; if (logLevel != Ci.nsIConsoleMessage.warn) { return; @@ -146,46 +146,101 @@ const tests = [ preferredAction: alwaysAsk, runTest: onExternalApplication, header: "preferredAction = alwaysAsk", + file: "file_pdf.pdf", + sandbox: "allow-downloads", }, { preferredAction: saveToDisk, runTest: onFilePickerShown, header: "preferredAction = saveToDisk", + file: "file_pdf.pdf", + sandbox: "allow-downloads", }, { + // The preferredAction handleInternally is only noticed when we're getting + // an externally handled PDF that we forcefully handle internally. preferredAction: handleInternally, runTest: onBlockedBySandbox, header: "preferredAction = handleInternally", + prefs: ["browser.download.open_pdf_attachments_inline", true], + file: "file_pdf_content_disposition.pdf", + sandbox: "allow-downloads", }, { preferredAction: useSystemDefault, runTest: onDownload, header: "preferredAction = useSystemDefault", + file: "file_pdf.pdf", + sandbox: "allow-downloads", + }, + { + preferredAction: alwaysAsk, + runTest: onBlockedBySandbox, + header: "preferredAction = alwaysAsk", + file: "file_pdf.pdf", + }, + { + preferredAction: saveToDisk, + runTest: onBlockedBySandbox, + header: "preferredAction = saveToDisk", + file: "file_pdf.pdf", + }, + { + // The preferredAction handleInternally is only noticed when we're getting + // an externally handled PDF that we forcefully handle internally. + preferredAction: handleInternally, + runTest: onBlockedBySandbox, + header: "preferredAction = handleInternally", + prefs: ["browser.download.open_pdf_attachments_inline", true], + file: "file_pdf_content_disposition.pdf", + }, + { + preferredAction: useSystemDefault, + runTest: onBlockedBySandbox, + header: "preferredAction = useSystemDefault", + file: "file_pdf.pdf", }, ]; /** * Tests that selecting the context menu item `Save Link As…` on a PDF link * opens the file picker when always_ask_before_handling_new_types is disabled, - * regardless of preferredAction. + * regardless of preferredAction if the iframe has sandbox="allow-downloads". */ add_task(async function test_pdf_save_as_link() { let mimeInfo; - for (let { preferredAction, runTest, header } of tests) { + for (let { + preferredAction, + runTest, + header, + prefs, + file, + sandbox, + } of tests) { mimeInfo = MIMEService.getFromTypeAndExtension("application/pdf", "pdf"); mimeInfo.alwaysAskBeforeHandling = preferredAction === alwaysAsk; mimeInfo.preferredAction = preferredAction; HandlerService.store(mimeInfo); - info(`Running test: ${header}`); + info(`Running test: ${header}, ${sandbox ? sandbox : "no sandbox"}`); + + if (prefs) { + await SpecialPowers.pushPrefEnv({ + set: [prefs], + }); + } await runTest(() => { gBrowser.selectedTab = BrowserTestUtils.addTab( gBrowser, - `data:text/html,` + `data:text/html,` ); - }); + }, file); + + if (prefs) { + await SpecialPowers.popPrefEnv(); + } BrowserTestUtils.removeTab(gBrowser.selectedTab); } diff --git a/uriloader/exthandler/tests/mochitest/file_pdf_content_disposition.pdf b/uriloader/exthandler/tests/mochitest/file_pdf_content_disposition.pdf new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/uriloader/exthandler/tests/mochitest/file_pdf_content_disposition.pdf^headers^ b/uriloader/exthandler/tests/mochitest/file_pdf_content_disposition.pdf^headers^ new file mode 100644 index 000000000000..71ed8e78051b --- /dev/null +++ b/uriloader/exthandler/tests/mochitest/file_pdf_content_disposition.pdf^headers^ @@ -0,0 +1 @@ +Content-Disposition: attachment