Merge pull request #17694 from Snuffleupagus/validate-defaultOptions

Add better validation for the "PREFERENCE" kind `AppOptions`

gulpfile.mjs

@@ -863,11 +863,17 @@ async function parseDefaultPreferences(dir) {
     "./" + DEFAULT_PREFERENCES_DIR + dir + "app_options.mjs"
   );
 
-  const browserPrefs = AppOptions.getAll(OptionKind.BROWSER);
+  const browserPrefs = AppOptions.getAll(
+    OptionKind.BROWSER,
+    /* defaultOnly = */ true
+  );
   if (Object.keys(browserPrefs).length === 0) {
     throw new Error("No browser preferences found.");
   }
-  const prefs = AppOptions.getAll(OptionKind.PREFERENCE);
+  const prefs = AppOptions.getAll(
+    OptionKind.PREFERENCE,
+    /* defaultOnly = */ true
+  );
   if (Object.keys(prefs).length === 0) {
     throw new Error("No default preferences found.");
   }

test/unit/app_options_spec.js

@@ -0,0 +1,41 @@
+/* Copyright 2024 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { AppOptions, OptionKind } from "../../web/app_options.js";
+import { objectSize } from "../../src/shared/util.js";
+
+describe("AppOptions", function () {
+  it("checks that getAll returns data, for every OptionKind", function () {
+    const KIND_NAMES = ["BROWSER", "VIEWER", "API", "WORKER", "PREFERENCE"];
+
+    for (const name of KIND_NAMES) {
+      const kind = OptionKind[name];
+      expect(typeof kind).toEqual("number");
+
+      const options = AppOptions.getAll(kind);
+      expect(objectSize(options)).toBeGreaterThan(0);
+    }
+  });
+
+  it('checks that the number of "PREFERENCE" options does *not* exceed the maximum in mozilla-central', function () {
+    // If the following constant is updated then you *MUST* make the same change
+    // in mozilla-central as well to ensure that preference-fetching works; see
+    // https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs
+    const MAX_NUMBER_OF_PREFS = 50;
+
+    const options = AppOptions.getAll(OptionKind.PREFERENCE);
+    expect(objectSize(options)).toBeLessThanOrEqual(MAX_NUMBER_OF_PREFS);
+  });
+});

test/unit/clitests.json

@@ -7,6 +7,7 @@
     "annotation_spec.js",
     "annotation_storage_spec.js",
     "api_spec.js",
+    "app_options_spec.js",
     "bidi_spec.js",
     "cff_parser_spec.js",
     "cmap_spec.js",

test/unit/jasmine-boot.js

@@ -50,6 +50,7 @@ async function initializePDFJS(callback) {
       "pdfjs-test/unit/annotation_spec.js",
       "pdfjs-test/unit/annotation_storage_spec.js",
       "pdfjs-test/unit/api_spec.js",
+      "pdfjs-test/unit/app_options_spec.js",
       "pdfjs-test/unit/bidi_spec.js",
       "pdfjs-test/unit/cff_parser_spec.js",
       "pdfjs-test/unit/cmap_spec.js",

web/app_options.js

@@ -417,57 +417,64 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
 
 const userOptions = Object.create(null);
 
+if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING || LIB")) {
+  // Ensure that the `defaultOptions` are correctly specified.
+  for (const name in defaultOptions) {
+    const { value, kind } = defaultOptions[name];
+
+    if (kind & OptionKind.PREFERENCE) {
+      if (kind === OptionKind.PREFERENCE) {
+        throw new Error(`Cannot use only "PREFERENCE" kind: ${name}`);
+      }
+      if (kind & OptionKind.BROWSER) {
+        throw new Error(`Cannot mix "PREFERENCE" and "BROWSER" kind: ${name}`);
+      }
+      if (compatibilityParams[name] !== undefined) {
+        throw new Error(
+          `Should not have compatibility-value for "PREFERENCE" kind: ${name}`
+        );
+      }
+      // Only "simple" preference-values are allowed.
+      if (
+        typeof value !== "boolean" &&
+        typeof value !== "string" &&
+        !Number.isInteger(value)
+      ) {
+        throw new Error(`Invalid value for "PREFERENCE" kind: ${name}`);
+      }
+    }
+  }
+}
+
 class AppOptions {
   constructor() {
     throw new Error("Cannot initialize AppOptions.");
   }
 
-  static get(name) {
-    const userOption = userOptions[name];
-    if (userOption !== undefined) {
-      return userOption;
-    }
-    const defaultOption = defaultOptions[name];
-    if (defaultOption !== undefined) {
-      return compatibilityParams[name] ?? defaultOption.value;
-    }
-    return undefined;
+  static getCompat(name) {
+    return compatibilityParams[name] ?? undefined;
   }
 
-  static getAll(kind = null) {
+  static get(name) {
+    return (
+      userOptions[name] ??
+      compatibilityParams[name] ??
+      defaultOptions[name]?.value ??
+      undefined
+    );
+  }
+
+  static getAll(kind = null, defaultOnly = false) {
     const options = Object.create(null);
     for (const name in defaultOptions) {
       const defaultOption = defaultOptions[name];
-      if (kind) {
-        if (!(kind & defaultOption.kind)) {
-          continue;
-        }
-        if (
-          (typeof PDFJSDev === "undefined" || PDFJSDev.test("LIB")) &&
-          kind === OptionKind.PREFERENCE
-        ) {
-          if (defaultOption.kind & OptionKind.BROWSER) {
-            throw new Error(`Invalid kind for preference: ${name}`);
-          }
-          const value = defaultOption.value,
-            valueType = typeof value;
 
-          if (
-            valueType === "boolean" ||
-            valueType === "string" ||
-            (valueType === "number" && Number.isInteger(value))
-          ) {
-            options[name] = value;
+      if (kind && !(kind & defaultOption.kind)) {
         continue;
       }
-          throw new Error(`Invalid type for preference: ${name}`);
-        }
-      }
-      const userOption = userOptions[name];
-      options[name] =
-        userOption !== undefined
-          ? userOption
-          : compatibilityParams[name] ?? defaultOption.value;
+      options[name] = defaultOnly
+        ? defaultOption.value
+        : userOptions[name] ?? compatibilityParams[name] ?? defaultOption.value;
     }
     return options;
   }
@@ -501,4 +508,4 @@ class AppOptions {
   }
 }
 
-export { AppOptions, compatibilityParams, OptionKind };
+export { AppOptions, OptionKind };

web/pdf_page_view.js

@@ -41,7 +41,7 @@ import {
 } from "./ui_utils.js";
 import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.js";
 import { AnnotationLayerBuilder } from "./annotation_layer_builder.js";
-import { compatibilityParams } from "./app_options.js";
+import { AppOptions } from "./app_options.js";
 import { DrawLayerBuilder } from "./draw_layer_builder.js";
 import { GenericL10n } from "web-null_l10n";
 import { SimpleLinkService } from "./pdf_link_service.js";
@@ -83,8 +83,6 @@ import { XfaLayerBuilder } from "./xfa_layer_builder.js";
  *   the necessary layer-properties.
  */
 
-const MAX_CANVAS_PIXELS = compatibilityParams.maxCanvasPixels || 16777216;
-
 const DEFAULT_LAYER_PROPERTIES =
   typeof PDFJSDev === "undefined" || !PDFJSDev.test("COMPONENTS")
     ? null
@@ -152,7 +150,9 @@ class PDFPageView {
     this.#annotationMode =
       options.annotationMode ?? AnnotationMode.ENABLE_FORMS;
     this.imageResourcesPath = options.imageResourcesPath || "";
-    this.maxCanvasPixels = options.maxCanvasPixels ?? MAX_CANVAS_PIXELS;
+    this.maxCanvasPixels =
+      options.maxCanvasPixels ??
+      (AppOptions.getCompat("maxCanvasPixels") || 16777216);
     this.pageColors = options.pageColors || null;
 
     this.eventBus = options.eventBus;

web/preferences.js

@@ -23,7 +23,7 @@ import { AppOptions, OptionKind } from "./app_options.js";
 class BasePreferences {
   #defaults = Object.freeze(
     typeof PDFJSDev === "undefined"
-      ? AppOptions.getAll(OptionKind.PREFERENCE)
+      ? AppOptions.getAll(OptionKind.PREFERENCE, /* defaultOnly = */ true)
       : PDFJSDev.eval("DEFAULT_PREFERENCES")
   );
 
@@ -48,7 +48,7 @@ class BasePreferences {
       ({ browserPrefs, prefs }) => {
         const BROWSER_PREFS =
           typeof PDFJSDev === "undefined"
-            ? AppOptions.getAll(OptionKind.BROWSER)
+            ? AppOptions.getAll(OptionKind.BROWSER, /* defaultOnly = */ true)
             : PDFJSDev.eval("BROWSER_PREFERENCES");
         const options = Object.create(null);