From 1b75e45a62290a8d519ca7ba4848e8ae73565425 Mon Sep 17 00:00:00 2001 From: rreimann Date: Wed, 17 May 2017 15:45:24 +0200 Subject: [PATCH 1/7] Suppress pdf plugin dispatch if plugins are disabled --- .../atom_resource_dispatcher_host_delegate.cc | 5 ++++- atom/browser/web_contents_preferences.cc | 15 +++++++++++++++ atom/browser/web_contents_preferences.h | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index abd2fb53f1e2..d54021859703 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -6,6 +6,7 @@ #include "atom/browser/login_handler.h" #include "atom/browser/web_contents_permission_helper.h" +#include "atom/browser/web_contents_preferences.h" #include "atom/common/atom_constants.h" #include "atom/common/platform_util.h" #include "base/strings/stringprintf.h" @@ -124,7 +125,9 @@ bool AtomResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream( std::string* payload) { const content::ResourceRequestInfo* info = content::ResourceRequestInfo::ForRequest(request); - if (mime_type == "application/pdf" && info->IsMainFrame()) { + content::WebContents* web_contents = info->GetWebContentsGetterForRequest().Run(); + if (mime_type == "application/pdf" && info->IsMainFrame() && + WebContentsPreferences::IsPluginsEnabled(web_contents)) { *origin = GURL(kPdfViewerUIOrigin); content::BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 2bd0900311a8..7d99dd7ed5fb 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -237,6 +237,21 @@ bool WebContentsPreferences::UsesNativeWindowOpen( return use; } +bool WebContentsPreferences::IsPluginsEnabled(content::WebContents* web_contents) { + WebContentsPreferences* self; + if (!web_contents) + return false; + + self = FromWebContents(web_contents); + if (!self) + return false; + + base::DictionaryValue& web_preferences = self->web_preferences_; + bool plugins = false; + web_preferences.GetBoolean("plugins", &plugins); + return plugins; +} + // static void WebContentsPreferences::OverrideWebkitPrefs( content::WebContents* web_contents, content::WebPreferences* prefs) { diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index f046cdfc99ac..aa3dc36287b7 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/browser/web_contents_preferences.h @@ -39,6 +39,7 @@ class WebContentsPreferences static bool IsSandboxed(content::WebContents* web_contents); static bool UsesNativeWindowOpen(content::WebContents* web_contents); + static bool IsPluginsEnabled(content::WebContents* web_contents); // Modify the WebPreferences according to |web_contents|'s preferences. static void OverrideWebkitPrefs( From 79827549589dc2a640df85c3bc9212f21b52c120 Mon Sep 17 00:00:00 2001 From: rreimann Date: Wed, 17 May 2017 16:02:42 +0200 Subject: [PATCH 2/7] Fix linting violations --- atom/browser/atom_resource_dispatcher_host_delegate.cc | 5 +++-- atom/browser/web_contents_preferences.cc | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index d54021859703..f673672544ab 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -125,8 +125,9 @@ bool AtomResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream( std::string* payload) { const content::ResourceRequestInfo* info = content::ResourceRequestInfo::ForRequest(request); - content::WebContents* web_contents = info->GetWebContentsGetterForRequest().Run(); - if (mime_type == "application/pdf" && info->IsMainFrame() && + content::WebContents* web_contents = + info->GetWebContentsGetterForRequest().Run(); + if (mime_type == "application/pdf" && info->IsMainFrame() && WebContentsPreferences::IsPluginsEnabled(web_contents)) { *origin = GURL(kPdfViewerUIOrigin); content::BrowserThread::PostTask( diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 7d99dd7ed5fb..5a9e22edb740 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -237,7 +237,8 @@ bool WebContentsPreferences::UsesNativeWindowOpen( return use; } -bool WebContentsPreferences::IsPluginsEnabled(content::WebContents* web_contents) { +bool WebContentsPreferences::IsPluginsEnabled( + content::WebContents* web_contents) { WebContentsPreferences* self; if (!web_contents) return false; From 13665090d5032f0e6c517d65e7704fa0d6adac34 Mon Sep 17 00:00:00 2001 From: rreimann Date: Wed, 17 May 2017 17:01:53 +0200 Subject: [PATCH 3/7] Add new test and enable plugins for existing tests --- spec/chromium-spec.js | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 6567477b0728..4d93c8c5a130 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -1012,16 +1012,18 @@ describe('chromium feature', function () { slashes: true }) - beforeEach(function () { + function createBrowserWindow (isPluginsEnabled) { w = new BrowserWindow({ show: false, webPreferences: { - preload: path.join(fixtures, 'module', 'preload-inject-ipc.js') + preload: path.join(fixtures, 'module', 'preload-inject-ipc.js'), + plugins: isPluginsEnabled } }) - }) + } it('opens when loading a pdf resource as top level navigation', function (done) { + createBrowserWindow(true) ipcMain.once('pdf-loaded', function (event, success) { if (success) done() }) @@ -1043,7 +1045,23 @@ describe('chromium feature', function () { w.webContents.loadURL(pdfSource) }) + it('should download a pdf when plugins are disabled', function (done) { + createBrowserWindow(false) + ipcRenderer.sendSync('set-download-option', false, false) + ipcRenderer.once('download-done', function (event, state, url, + mimeType, receivedBytes, + totalBytes, disposition, + filename) { + assert.equal(state, 'completed') + assert.equal(filename, 'cat.pdf') + assert.equal(mimeType, 'application/pdf') + done() + }) + w.webContents.loadURL(pdfSource) + }) + it('should not open when pdf is requested as sub resource', function (done) { + createBrowserWindow(true) webFrame.registerURLSchemeAsPrivileged('file', { secure: false, bypassCSP: false, From 65da983ccbc573dd340f6d8eeae574759bafd047 Mon Sep 17 00:00:00 2001 From: rreimann Date: Fri, 19 May 2017 11:49:22 +0200 Subject: [PATCH 4/7] Move preferences check to OnPdfResourceIntercepted --- .../atom_resource_dispatcher_host_delegate.cc | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index f673672544ab..12038ff1bf50 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -4,6 +4,7 @@ #include "atom/browser/atom_resource_dispatcher_host_delegate.h" +#include "atom/browser/atom_browser_context.h" #include "atom/browser/login_handler.h" #include "atom/browser/web_contents_permission_helper.h" #include "atom/browser/web_contents_preferences.h" @@ -12,6 +13,7 @@ #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/download_manager.h" #include "content/public/browser/stream_info.h" #include "net/base/escape.h" #include "net/ssl/client_cert_store.h" @@ -70,6 +72,17 @@ void OnPdfResourceIntercepted( if (!web_contents) return; + if (!WebContentsPreferences::IsPluginsEnabled(web_contents)) { + auto browser_context = web_contents->GetBrowserContext(); + auto download_manager = + content::BrowserContext::GetDownloadManager(browser_context); + + download_manager->DownloadUrl( + content::DownloadUrlParameters::CreateForWebContentsMainFrame( + web_contents, original_url)); + return; + } + // The URL passes the original pdf resource url, that will be requested // by the webui page. // chrome://pdf-viewer/index.html?src=https://somepage/123.pdf @@ -125,10 +138,7 @@ bool AtomResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream( std::string* payload) { const content::ResourceRequestInfo* info = content::ResourceRequestInfo::ForRequest(request); - content::WebContents* web_contents = - info->GetWebContentsGetterForRequest().Run(); - if (mime_type == "application/pdf" && info->IsMainFrame() && - WebContentsPreferences::IsPluginsEnabled(web_contents)) { + if (mime_type == "application/pdf" && info->IsMainFrame()) { *origin = GURL(kPdfViewerUIOrigin); content::BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, From 791486433d62203ff7f5465baf4edf322f94b3ba Mon Sep 17 00:00:00 2001 From: rreimann Date: Fri, 19 May 2017 11:51:12 +0200 Subject: [PATCH 5/7] Extract common code into IsPreferenceEnabled method --- atom/browser/web_contents_preferences.cc | 40 +++++++----------------- atom/browser/web_contents_preferences.h | 2 ++ 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 5a9e22edb740..168a6452fd5a 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -206,7 +206,9 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( } } -bool WebContentsPreferences::IsSandboxed(content::WebContents* web_contents) { +bool WebContentsPreferences::IsPreferenceEnabled( + const std::string& attributeName, + content::WebContents* web_contents) { WebContentsPreferences* self; if (!web_contents) return false; @@ -216,41 +218,23 @@ bool WebContentsPreferences::IsSandboxed(content::WebContents* web_contents) { return false; base::DictionaryValue& web_preferences = self->web_preferences_; - bool sandboxed = false; - web_preferences.GetBoolean("sandbox", &sandboxed); - return sandboxed; + bool boolValue = false; + web_preferences.GetBoolean(attributeName, &boolValue); + return boolValue; +} + +bool WebContentsPreferences::IsSandboxed(content::WebContents* web_contents) { + return IsPreferenceEnabled("sandbox", web_contents); } bool WebContentsPreferences::UsesNativeWindowOpen( content::WebContents* web_contents) { - WebContentsPreferences* self; - if (!web_contents) - return false; - - self = FromWebContents(web_contents); - if (!self) - return false; - - base::DictionaryValue& web_preferences = self->web_preferences_; - bool use = false; - web_preferences.GetBoolean("nativeWindowOpen", &use); - return use; + return IsPreferenceEnabled("nativeWindowOpen", web_contents); } bool WebContentsPreferences::IsPluginsEnabled( content::WebContents* web_contents) { - WebContentsPreferences* self; - if (!web_contents) - return false; - - self = FromWebContents(web_contents); - if (!self) - return false; - - base::DictionaryValue& web_preferences = self->web_preferences_; - bool plugins = false; - web_preferences.GetBoolean("plugins", &plugins); - return plugins; + return IsPreferenceEnabled("plugins", web_contents); } // static diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index aa3dc36287b7..d3b565201b7a 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/browser/web_contents_preferences.h @@ -37,6 +37,8 @@ class WebContentsPreferences static void AppendExtraCommandLineSwitches( content::WebContents* web_contents, base::CommandLine* command_line); + static bool IsPreferenceEnabled(const std::string& attributeName, + content::WebContents* web_contents); static bool IsSandboxed(content::WebContents* web_contents); static bool UsesNativeWindowOpen(content::WebContents* web_contents); static bool IsPluginsEnabled(content::WebContents* web_contents); From 25015c4c6373e74eb496f448e0d5d730aa4c9fe8 Mon Sep 17 00:00:00 2001 From: rreimann Date: Mon, 22 May 2017 09:08:47 +0200 Subject: [PATCH 6/7] Fix naming and formatting --- atom/browser/web_contents_preferences.cc | 8 ++++---- atom/browser/web_contents_preferences.h | 2 +- spec/chromium-spec.js | 5 +---- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 168a6452fd5a..a17c44fe05af 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -207,7 +207,7 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( } bool WebContentsPreferences::IsPreferenceEnabled( - const std::string& attributeName, + const std::string& attribute_name, content::WebContents* web_contents) { WebContentsPreferences* self; if (!web_contents) @@ -218,9 +218,9 @@ bool WebContentsPreferences::IsPreferenceEnabled( return false; base::DictionaryValue& web_preferences = self->web_preferences_; - bool boolValue = false; - web_preferences.GetBoolean(attributeName, &boolValue); - return boolValue; + bool bool_value = false; + web_preferences.GetBoolean(attribute_name, &bool_value); + return bool_value; } bool WebContentsPreferences::IsSandboxed(content::WebContents* web_contents) { diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index d3b565201b7a..a2312e4ab44d 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/browser/web_contents_preferences.h @@ -37,7 +37,7 @@ class WebContentsPreferences static void AppendExtraCommandLineSwitches( content::WebContents* web_contents, base::CommandLine* command_line); - static bool IsPreferenceEnabled(const std::string& attributeName, + static bool IsPreferenceEnabled(const std::string& attribute_name, content::WebContents* web_contents); static bool IsSandboxed(content::WebContents* web_contents); static bool UsesNativeWindowOpen(content::WebContents* web_contents); diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 4d93c8c5a130..42d6ff2f519c 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -1048,10 +1048,7 @@ describe('chromium feature', function () { it('should download a pdf when plugins are disabled', function (done) { createBrowserWindow(false) ipcRenderer.sendSync('set-download-option', false, false) - ipcRenderer.once('download-done', function (event, state, url, - mimeType, receivedBytes, - totalBytes, disposition, - filename) { + ipcRenderer.once('download-done', function (event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename) { assert.equal(state, 'completed') assert.equal(filename, 'cat.pdf') assert.equal(mimeType, 'application/pdf') From 647e88da5a24de9ee1229cbe6a73a8fadd25df82 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 22 May 2017 14:24:28 -0700 Subject: [PATCH 7/7] Delete mock.pdf after download completes --- spec/chromium-spec.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 42d6ff2f519c..fe132166d602 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -1012,18 +1012,18 @@ describe('chromium feature', function () { slashes: true }) - function createBrowserWindow (isPluginsEnabled) { + function createBrowserWindow ({plugins}) { w = new BrowserWindow({ show: false, webPreferences: { preload: path.join(fixtures, 'module', 'preload-inject-ipc.js'), - plugins: isPluginsEnabled + plugins: plugins } }) } it('opens when loading a pdf resource as top level navigation', function (done) { - createBrowserWindow(true) + createBrowserWindow({plugins: true}) ipcMain.once('pdf-loaded', function (event, success) { if (success) done() }) @@ -1046,19 +1046,20 @@ describe('chromium feature', function () { }) it('should download a pdf when plugins are disabled', function (done) { - createBrowserWindow(false) + createBrowserWindow({plugins: false}) ipcRenderer.sendSync('set-download-option', false, false) ipcRenderer.once('download-done', function (event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename) { assert.equal(state, 'completed') assert.equal(filename, 'cat.pdf') assert.equal(mimeType, 'application/pdf') + fs.unlinkSync(path.join(fixtures, 'mock.pdf')) done() }) w.webContents.loadURL(pdfSource) }) it('should not open when pdf is requested as sub resource', function (done) { - createBrowserWindow(true) + createBrowserWindow({plugins: true}) webFrame.registerURLSchemeAsPrivileged('file', { secure: false, bypassCSP: false,