Skip to content

Commit

Permalink
CrOS MediaApp: set "oject-src" & "frame-src" CSP so we can load PDFs.
Browse files Browse the repository at this point in the history
There seems to be a change in M87 unrelated to the CrOS MediaApp
which broke PDF loading. I think this was due to tighter CSP.

This change sets our CSP so we can load PDFs as blob urls in our
untrusted context and adds a test for coverage of the "object-src"
policy.

Further work to make our tests better & provide coverage for "frame-src"
is captured in crbug/1148090.

Bug: b/172881869, 1148090
Change-Id: I6e5ea0816c7f8c5a4eb382ed376e4fd98f9f671c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531075
Reviewed-by: dstockwell <dstockwell@chromium.org>
Reviewed-by: David Lei <dlei@google.com>
Reviewed-by: Patti <patricialor@chromium.org>
Commit-Queue: David Lei <dlei@google.com>
Commit-Queue: dstockwell <dstockwell@chromium.org>
Auto-Submit: David Lei <dlei@google.com>
Cr-Commit-Position: refs/heads/master@{#827021}
  • Loading branch information
David authored and Commit Bot committed Nov 12, 2020
1 parent e0de6e9 commit e66d538
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,34 @@ IN_PROC_BROWSER_TEST_P(MediaAppIntegrationTest, MediaAppLaunchWithFile) {
EXPECT_EQ("640x480", WaitForImageAlt(app, kFileJpeg640x480));
}

// Regression test for b/172881869.
IN_PROC_BROWSER_TEST_P(MediaAppIntegrationTest, LoadsPdf) {
WaitForTestSystemAppInstall();
LaunchApp(web_app::SystemAppType::MEDIA);
content::WebContents* app = PrepareActiveBrowserForTest();
// TODO(crbug/1148090): To fully load PDFs, "frame-src" needs to be set, this
// test doesn't provide coverage for that.
// Note: If "object-src" is not set in the CSP, the `<embed>` element fails to
// load and times out.
constexpr char loadPdf[] = R"(
(() => {
const embedBlob = document.createElement('embed');
embedBlob.type ='application/pdf';
embedBlob.height = '100%';
embedBlob.width = '100%';
const loadPromise = new Promise((resolve, reject) => {
embedBlob.addEventListener('load', () => resolve(true));
embedBlob.addEventListener('error', () => reject(false));
});
document.body.appendChild(embedBlob);
embedBlob.src = 'blob:chrome-untrusted://media-app/fake-pdf-blob-hash';
return loadPromise;
})();
)";

EXPECT_EQ(true, MediaAppUiBrowserTest::EvalJsInAppFrame(app, loadPdf));
}

// Test that the MediaApp can load RAW files passed on launch params.
IN_PROC_BROWSER_TEST_P(MediaAppIntegrationTest, HandleRawFiles) {
WaitForTestSystemAppInstall();
Expand Down
7 changes: 6 additions & 1 deletion chromeos/components/media_app_ui/media_app_guest_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,12 @@ content::WebUIDataSource* CreateMediaAppUntrustedDataSource(
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::StyleSrc,
"style-src 'self' 'unsafe-inline';");

// Allow loading PDFs as blob URLs.
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ObjectSrc, "object-src blob:;");
// Required to successfully load PDFs in the `<embed>` element.
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::FrameSrc, "frame-src blob:;");
// Allow wasm.
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ScriptSrc,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ GUEST_TEST('GuestLoadsLoadTimeData', () => {
// Test can load files with CSP restrictions. We expect `error` to be called
// as these tests are loading resources that don't exist. Note: we can't violate
// CSP in tests or Js Errors will cause test failures.
// TODO(crbug/1148090): PDF loading tests should also appear here, they are
// currently in media_app_integration_browsertest.cc due to 'wasm-eval' JS
// errors.
GUEST_TEST('GuestCanLoadWithCspRestrictions', async () => {
// Can load images served from chrome-untrusted://media-app/.
const image = new Image();
Expand Down

0 comments on commit e66d538

Please sign in to comment.