Change the parseQueryString function to return a Map rather than an Object (issue 13829)

Even though the code as-is *should* be safe, given that we're using an Object with a `null` prototype, it cannot hurt to change this to a Map to prevent any issues (since we're parsing unknown and potentially unsafe data).

Overall I also think that these changes improve the `parseQueryString` call-sites, since we now have a proper way of checking for the existence of a particular key (and don't have to use `in` which stringifies the keys in the Object).

This patch also changes the default, when no `value` exists, from `null` to an empty string since the use of `decodeURIComponent` currently can modify the value in a somewhat surprising way (at least to me).
Note how `decodeURIComponent(null) === "null"` which is unlikely to be what you actually want, whereas `decodeURIComponent("") === ""` which seems much more helpful.
This commit is contained in:
Jonas Jenwald 2021-07-31 22:15:30 +02:00
parent 222c9e7e84
commit 4ab4efd42f
4 changed files with 47 additions and 42 deletions

View File

@ -329,35 +329,38 @@ const PDFViewerApplication = {
if (!hash) {
return undefined;
}
const hashParams = parseQueryString(hash),
const params = parseQueryString(hash),
waitOn = [];
if ("disableworker" in hashParams && hashParams.disableworker === "true") {
if (params.get("disableworker") === "true") {
waitOn.push(loadFakeWorker());
}
if ("disablerange" in hashParams) {
AppOptions.set("disableRange", hashParams.disablerange === "true");
if (params.has("disablerange")) {
AppOptions.set("disableRange", params.get("disablerange") === "true");
}
if ("disablestream" in hashParams) {
AppOptions.set("disableStream", hashParams.disablestream === "true");
if (params.has("disablestream")) {
AppOptions.set("disableStream", params.get("disablestream") === "true");
}
if ("disableautofetch" in hashParams) {
if (params.has("disableautofetch")) {
AppOptions.set(
"disableAutoFetch",
hashParams.disableautofetch === "true"
params.get("disableautofetch") === "true"
);
}
if ("disablefontface" in hashParams) {
AppOptions.set("disableFontFace", hashParams.disablefontface === "true");
if (params.has("disablefontface")) {
AppOptions.set(
"disableFontFace",
params.get("disablefontface") === "true"
);
}
if ("disablehistory" in hashParams) {
AppOptions.set("disableHistory", hashParams.disablehistory === "true");
if (params.has("disablehistory")) {
AppOptions.set("disableHistory", params.get("disablehistory") === "true");
}
if ("verbosity" in hashParams) {
AppOptions.set("verbosity", hashParams.verbosity | 0);
if (params.has("verbosity")) {
AppOptions.set("verbosity", params.get("verbosity") | 0);
}
if ("textlayer" in hashParams) {
switch (hashParams.textlayer) {
if (params.has("textlayer")) {
switch (params.get("textlayer")) {
case "off":
AppOptions.set("textLayerMode", TextLayerMode.DISABLE);
break;
@ -365,24 +368,24 @@ const PDFViewerApplication = {
case "shadow":
case "hover":
const viewer = this.appConfig.viewerContainer;
viewer.classList.add("textLayer-" + hashParams.textlayer);
viewer.classList.add(`textLayer-${params.get("textlayer")}`);
break;
}
}
if ("pdfbug" in hashParams) {
if (params.has("pdfbug")) {
AppOptions.set("pdfBug", true);
AppOptions.set("fontExtraProperties", true);
const enabled = hashParams.pdfbug.split(",");
const enabled = params.get("pdfbug").split(",");
waitOn.push(loadAndEnablePDFBug(enabled));
}
// It is not possible to change locale for the (various) extension builds.
if (
(typeof PDFJSDev === "undefined" ||
PDFJSDev.test("!PRODUCTION || GENERIC")) &&
"locale" in hashParams
params.has("locale")
) {
AppOptions.set("locale", hashParams.locale);
AppOptions.set("locale", params.get("locale"));
}
if (waitOn.length === 0) {
@ -2167,7 +2170,7 @@ function webViewerInitialized() {
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
const queryString = document.location.search.substring(1);
const params = parseQueryString(queryString);
file = "file" in params ? params.file : AppOptions.get("defaultUrl");
file = params.get("file") ?? AppOptions.get("defaultUrl");
validateFileURL(file);
} else if (PDFJSDev.test("MOZCENTRAL")) {
file = window.location.href;

View File

@ -544,8 +544,8 @@ class PDFHistory {
const hash = unescape(getCurrentHash()).substring(1);
const params = parseQueryString(hash);
const nameddest = params.nameddest || "";
let page = params.page | 0;
const nameddest = params.get("nameddest") || "";
let page = params.get("page") | 0;
if (!this._isValidPage(page) || (checkNameddest && nameddest.length > 0)) {
page = null;
@ -753,7 +753,7 @@ function isDestHashesEqual(destHash, pushHash) {
if (destHash === pushHash) {
return true;
}
const { nameddest } = parseQueryString(destHash);
const nameddest = parseQueryString(destHash).get("nameddest");
if (nameddest === pushHash) {
return true;
}

View File

@ -263,20 +263,20 @@ class PDFLinkService {
let pageNumber, dest;
if (hash.includes("=")) {
const params = parseQueryString(hash);
if ("search" in params) {
if (params.has("search")) {
this.eventBus.dispatch("findfromurlhash", {
source: this,
query: params.search.replace(/"/g, ""),
phraseSearch: params.phrase === "true",
query: params.get("search").replace(/"/g, ""),
phraseSearch: params.get("phrase") === "true",
});
}
// borrowing syntax from "Parameters for Opening PDF Files"
if ("page" in params) {
pageNumber = params.page | 0 || 1;
if (params.has("page")) {
pageNumber = params.get("page") | 0 || 1;
}
if ("zoom" in params) {
if (params.has("zoom")) {
// Build the destination array.
const zoomArgs = params.zoom.split(","); // scale,left,top
const zoomArgs = params.get("zoom").split(","); // scale,left,top
const zoomArg = zoomArgs[0];
const zoomArgNumber = parseFloat(zoomArg);
@ -336,16 +336,16 @@ class PDFLinkService {
} else if (pageNumber) {
this.page = pageNumber; // simple page
}
if ("pagemode" in params) {
if (params.has("pagemode")) {
this.eventBus.dispatch("pagemode", {
source: this,
mode: params.pagemode,
mode: params.get("pagemode"),
});
}
// Ensure that this parameter is *always* handled last, in order to
// guarantee that it won't be overridden (e.g. by the "page" parameter).
if ("nameddest" in params) {
this.goToDestination(params.nameddest);
if (params.has("nameddest")) {
this.goToDestination(params.get("nameddest"));
}
} else {
// Named (or explicit) destination.

View File

@ -180,16 +180,18 @@ function watchScroll(viewAreaElement, callback) {
}
/**
* Helper function to parse query string (e.g. ?param1=value&parm2=...).
* Helper function to parse query string (e.g. ?param1=value&param2=...).
* @param {string}
* @returns {Map}
*/
function parseQueryString(query) {
const parts = query.split("&");
const params = Object.create(null);
const params = new Map();
for (let i = 0, ii = parts.length; i < ii; ++i) {
const param = parts[i].split("=");
const key = param[0].toLowerCase();
const value = param.length > 1 ? param[1] : null;
params[decodeURIComponent(key)] = decodeURIComponent(value);
const param = parts[i].split("="),
key = param[0].toLowerCase(),
value = param.length > 1 ? param[1] : "";
params.set(decodeURIComponent(key), decodeURIComponent(value));
}
return params;
}