From 1f9d1f369609fac818ddadb5c51ed94ea7e46f6a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 21 Jun 2023 18:28:35 +0200 Subject: [PATCH 1/2] [Firefox] Disable the ability to change preferences directly from the viewer Please note that we've never had any functionality in the viewer itself that *set* preferences, and we've thus only ever read them. For the GENERIC viewer it obviously makes sense for the user to be able to modify preferences, e.g. via the console, but that doesn't really apply to the *built-in* Firefox PDF Viewer since preferences are already accessible via `about:config` there. Hence it does seems somewhat strange to expose, a limited part of, the Firefox preference system in this way when we're not even using it. Note that the unused preference setting-code also include a fair amount of *additional* validation on the platform-side, such as limiting any possible preference changes to the `pdfjs.`-branch and also an explicit white-list of preference names[1], to make sure that this is safe; please see: - https://searchfox.org/mozilla-central/rev/4e8f62a231e71dc53eb50b6d74afca21d6b254e9/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs#458-495 - https://searchfox.org/mozilla-central/rev/4e8f62a231e71dc53eb50b6d74afca21d6b254e9/toolkit/modules/AsyncPrefs.sys.mjs#21-48 Assuming that this patch lands, I'll follow-up with a mozilla-central patch to remove the code mentioned above. --- [1] This hard-coded list contains preferences that no longer exist, and also at least one (fairly obvious) typo. --- web/firefoxcom.js | 4 ---- web/preferences.js | 6 ++++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 6d8b35250..6d5401fc4 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -174,10 +174,6 @@ class DownloadManager { } class FirefoxPreferences extends BasePreferences { - async _writeToStorage(prefObj) { - return FirefoxCom.requestAsync("setPreferences", prefObj); - } - async _readFromStorage(prefObj) { const prefStr = await FirefoxCom.requestAsync("getPreferences", prefObj); return JSON.parse(prefStr); diff --git a/web/preferences.js b/web/preferences.js index eebc480ca..dead99e98 100644 --- a/web/preferences.js +++ b/web/preferences.js @@ -83,6 +83,9 @@ class BasePreferences { * have been reset. */ async reset() { + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { + throw new Error("Please use `about:config` to change preferences."); + } await this.#initializedPromise; const prefs = this.#prefs; @@ -102,6 +105,9 @@ class BasePreferences { * provided that the preference exists and the types match. */ async set(name, value) { + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { + throw new Error("Please use `about:config` to change preferences."); + } await this.#initializedPromise; const defaultValue = this.#defaults[name], prefs = this.#prefs; From 5c0872d1b09440f729168503b799cddcc3befc49 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 21 Jun 2023 19:53:30 +0200 Subject: [PATCH 2/2] [Firefox] Avoid unnecessary string-parsing when reading preferences Note how the [`ChromeActions.getPreferences` method](https://searchfox.org/mozilla-central/rev/4e8f62a231e71dc53eb50b6d74afca21d6b254e9/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs#497-530) returns the preferences as a string, which we then have to convert back into an Object in the viewer. Back when that code was originally written it wasn't possible to send Objects from the platform-code, however that's no longer the case and we should be able to (eventually) remove this unnecessary string-parsing now. *Please note that in order to prevent breakage we'll need to land these changes in stages:* - Land this patch in mozilla-central, as part of regular the PDF.js updates. - Change the return type in the `ChromeActions.getPreferences` method, in a mozilla-central patch. - Remove the string-handling from the `FirefoxPreferences._readFromStorage` method. --- web/firefoxcom.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 6d5401fc4..8673a3fd0 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -175,8 +175,8 @@ class DownloadManager { class FirefoxPreferences extends BasePreferences { async _readFromStorage(prefObj) { - const prefStr = await FirefoxCom.requestAsync("getPreferences", prefObj); - return JSON.parse(prefStr); + const prefs = await FirefoxCom.requestAsync("getPreferences", prefObj); + return typeof prefs === "string" ? JSON.parse(prefs) : prefs; } }