From 9a7651a93faaaef0534df1ab77268d7deb9d3165 Mon Sep 17 00:00:00 2001 From: Boik Date: Fri, 14 Jul 2017 12:09:14 +0800 Subject: [PATCH 1/4] fix a pdf-viewer's parsing logic, and this should resolve the related issue at https://github.com/electron/electron/issues/10007 --- atom/browser/atom_resource_dispatcher_host_delegate.cc | 3 ++- atom/browser/atom_web_ui_controller_factory.cc | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index 12038ff1bf50..53c7cf094035 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -86,9 +86,10 @@ void OnPdfResourceIntercepted( // 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 + GURL escaped_url(net::EscapeUrlEncodedData(original_url.spec(), true)); content::NavigationController::LoadURLParams params( GURL(base::StringPrintf("%sindex.html?%s=%s", kPdfViewerUIOrigin, - kPdfPluginSrc, original_url.spec().c_str()))); + kPdfPluginSrc, escaped_url.spec().c_str()))); web_contents->GetController().LoadURLWithParams(params); } diff --git a/atom/browser/atom_web_ui_controller_factory.cc b/atom/browser/atom_web_ui_controller_factory.cc index ef8b2785ae55..57a76e05cc77 100644 --- a/atom/browser/atom_web_ui_controller_factory.cc +++ b/atom/browser/atom_web_ui_controller_factory.cc @@ -11,6 +11,7 @@ #include "base/strings/string_split.h" #include "base/strings/string_util.h" #include "content/public/browser/web_contents.h" +#include "net/base/escape.h" namespace atom { @@ -52,9 +53,16 @@ AtomWebUIControllerFactory::CreateWebUIControllerForURL(content::WebUI* web_ui, base::StringPairs toplevel_params; base::SplitStringIntoKeyValuePairs(url.query(), '=', '&', &toplevel_params); std::string stream_id, src; + + const net::UnescapeRule::Type unescape_rules = + net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS | + net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS | + net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS | + net::UnescapeRule::NORMAL | net::UnescapeRule::REPLACE_PLUS_WITH_SPACE; + for (const auto& param : toplevel_params) { if (param.first == kPdfPluginSrc) { - src = param.second; + src = net::UnescapeURLComponent(param.second, unescape_rules); } } if (url.has_ref()) { From 8d6ee5aad20ecf69a7f59bd8c7fb416a0db87eda Mon Sep 17 00:00:00 2001 From: Boik Date: Sat, 15 Jul 2017 10:32:46 +0800 Subject: [PATCH 2/4] add some minor fixes --- atom/browser/atom_resource_dispatcher_host_delegate.cc | 7 ++++--- atom/browser/atom_web_ui_controller_factory.cc | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index 53c7cf094035..19359e2ca549 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -86,10 +86,11 @@ void OnPdfResourceIntercepted( // 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 - GURL escaped_url(net::EscapeUrlEncodedData(original_url.spec(), true)); content::NavigationController::LoadURLParams params( - GURL(base::StringPrintf("%sindex.html?%s=%s", kPdfViewerUIOrigin, - kPdfPluginSrc, escaped_url.spec().c_str()))); + GURL(base::StringPrintf("%sindex.html?%s=%s", + kPdfViewerUIOrigin, + kPdfPluginSrc, + net::EscapeUrlEncodedData(original_url.spec(), false).c_str()))); web_contents->GetController().LoadURLWithParams(params); } diff --git a/atom/browser/atom_web_ui_controller_factory.cc b/atom/browser/atom_web_ui_controller_factory.cc index 57a76e05cc77..d113e656084a 100644 --- a/atom/browser/atom_web_ui_controller_factory.cc +++ b/atom/browser/atom_web_ui_controller_factory.cc @@ -55,10 +55,9 @@ AtomWebUIControllerFactory::CreateWebUIControllerForURL(content::WebUI* web_ui, std::string stream_id, src; const net::UnescapeRule::Type unescape_rules = - net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS | net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS | net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS | - net::UnescapeRule::NORMAL | net::UnescapeRule::REPLACE_PLUS_WITH_SPACE; + net::UnescapeRule::REPLACE_PLUS_WITH_SPACE; for (const auto& param : toplevel_params) { if (param.first == kPdfPluginSrc) { From 4fb1fffe44e7ffdb5d701a90fefc1b6e71ff1b3c Mon Sep 17 00:00:00 2001 From: Boik Date: Sat, 15 Jul 2017 10:57:16 +0800 Subject: [PATCH 3/4] fix lint --- atom/browser/atom_resource_dispatcher_host_delegate.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index 19359e2ca549..53d91b4be954 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -87,10 +87,11 @@ void OnPdfResourceIntercepted( // by the webui page. // chrome://pdf-viewer/index.html?src=https://somepage/123.pdf content::NavigationController::LoadURLParams params( - GURL(base::StringPrintf("%sindex.html?%s=%s", - kPdfViewerUIOrigin, - kPdfPluginSrc, - net::EscapeUrlEncodedData(original_url.spec(), false).c_str()))); + GURL(base::StringPrintf( + "%sindex.html?%s=%s", + kPdfViewerUIOrigin, + kPdfPluginSrc, + net::EscapeUrlEncodedData(original_url.spec(), false).c_str()))); web_contents->GetController().LoadURLWithParams(params); } From 01d021e6a331eeff12f6a29e3930de3e8cfdde96 Mon Sep 17 00:00:00 2001 From: Boik Date: Sat, 15 Jul 2017 11:39:43 +0800 Subject: [PATCH 4/4] add a new spec for https://github.com/electron/electron/pull/10008 --- spec/chromium-spec.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index e4a9f04d78ef..2f9f2b2795ad 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -982,6 +982,15 @@ describe('chromium feature', function () { protocol: 'file', slashes: true }) + const pdfSourceWithParams = url.format({ + pathname: path.join(fixtures, 'assets', 'cat.pdf').replace(/\\/g, '/'), + query: { + a: 1, + b: 2 + }, + protocol: 'file', + slashes: true + }) function createBrowserWindow ({plugins}) { w = new BrowserWindow({ @@ -1009,6 +1018,24 @@ describe('chromium feature', function () { w.webContents.loadURL(pdfSource) }) + it('opens a pdf link given params, the query string should be escaped', function (done) { + createBrowserWindow({plugins: true}) + ipcMain.once('pdf-loaded', function (event, state) { + assert.equal(state, 'success') + done() + }) + w.webContents.on('page-title-updated', function () { + const parsedURL = url.parse(w.webContents.getURL(), true) + assert.equal(parsedURL.protocol, 'chrome:') + assert.equal(parsedURL.hostname, 'pdf-viewer') + assert.equal(parsedURL.query.src, pdfSourceWithParams) + assert.equal(parsedURL.query.b, undefined) + assert.equal(parsedURL.search, `?src=${pdfSource}%3Fa%3D1%26b%3D2`) + assert.equal(w.webContents.getTitle(), 'cat.pdf') + }) + w.webContents.loadURL(pdfSourceWithParams) + }) + it('should download a pdf when plugins are disabled', function (done) { createBrowserWindow({plugins: false}) ipcRenderer.sendSync('set-download-option', false, false)