New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a query string parsing problem happend to PDF Viewer #10008

Merged
merged 4 commits into from Jul 18, 2017
Jump to file or symbol
Failed to load files and symbols.
+40 −3
Diff settings

Always

Just for now

@@ -87,8 +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, original_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);
}
@@ -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,15 @@ 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 =

This comment has been minimized.

@deepak1556

deepak1556 Jul 14, 2017

Member

SPOOFING_AND_CONTROL_CHARS should be avoided. Also NORMAL rule need not be specified, any rule specified includes the normal rule by default.

@deepak1556

deepak1556 Jul 14, 2017

Member

SPOOFING_AND_CONTROL_CHARS should be avoided. Also NORMAL rule need not be specified, any rule specified includes the normal rule by default.

net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
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()) {
View
@@ -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)
ProTip! Use n and p to navigate between commits in a pull request.