From 088ce6c00919e1b29fcaa5ab42d71d038054a522 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Sat, 13 Aug 2016 12:20:03 +0200
Subject: [PATCH] Add a unit-test to check that `ProblematicCharRanges`
 contains valid entries

When adding new entries to `ProblematicCharRanges`, you have to be careful to not make any mistakes since that could cause glyph mapping issues.
Currently the existing reference tests should probably help catch any errors, but based on experience I think that having a unit-test which specifically checks `ProblematicCharRanges` would be both helpful and timesaving when modifying/reviewing changes to this code.

Hence this patch which adds a function (and unit-test) that is used to validate the entries in `ProblematicCharRanges`, and also checks that we don't accidentally add more character ranges than the Private Use Area can actually contain.
The way that the validation code, and thus the unit-test, is implemented also means that we have an easy way to tell how much of the Private Use Area is potentially utilized by re-mapped characters.
---
 src/core/fonts.js        | 68 ++++++++++++++++++++++++++++++++++++++++
 test/unit/fonts_spec.js  | 13 ++++++++
 test/unit/unit_test.html |  1 +
 3 files changed, 82 insertions(+)
 create mode 100644 test/unit/fonts_spec.js

diff --git a/src/core/fonts.js b/src/core/fonts.js
index 9bfc375f8..e838341af 100644
--- a/src/core/fonts.js
+++ b/src/core/fonts.js
@@ -471,6 +471,74 @@ var ProblematicCharRanges = new Int32Array([
   0xFFF0, 0x10000
 ]);
 
+//#if !PRODUCTION
+/**
+ * Used to validate the entries in `ProblematicCharRanges`, and to ensure that
+ * its total number of characters does not exceed the PUA (Private Use Area)
+ * length.
+ * @returns {Object} An object with {number} `numChars`, {number} `puaLength`,
+ *   and {number} `percentage` parameters.
+ */
+function checkProblematicCharRanges() {
+  function printRange(limits) {
+    return '[' + limits.lower.toString('16').toUpperCase() + ', ' +
+                 limits.upper.toString('16').toUpperCase() + ')';
+  }
+
+  var numRanges = ProblematicCharRanges.length;
+  if (numRanges % 2 !== 0) {
+    throw new Error('Char ranges must contain an even number of elements.');
+  }
+  var previousLimits, numChars = 0;
+  for (var i = 0; i < numRanges; i += 2) {
+    var limits = {
+      lower: ProblematicCharRanges[i],
+      upper: ProblematicCharRanges[i + 1],
+    };
+    if (!isInt(limits.lower) || !isInt(limits.upper)) {
+      throw new Error('Range endpoints must be integers: ' +
+                      printRange(limits));
+    }
+    if (limits.lower < 0 || limits.upper < 0) {
+      throw new Error('Range endpoints must be non-negative: ' +
+                      printRange(limits));
+    }
+    var range = limits.upper - limits.lower;
+    if (range < 1) {
+      throw new Error('Range must contain at least one element: ' +
+                      printRange(limits));
+    }
+    if (previousLimits) {
+      if (limits.lower < previousLimits.lower) {
+        throw new Error('Ranges must be sorted in ascending order: ' +
+                        printRange(limits) + ', ' + printRange(previousLimits));
+      }
+      if (limits.lower < previousLimits.upper) {
+        throw new Error('Ranges must not overlap: ' +
+                        printRange(limits) + ', ' + printRange(previousLimits));
+      }
+    }
+    previousLimits = {
+      lower: limits.lower,
+      upper: limits.upper,
+    };
+    // The current range is OK.
+    numChars += range;
+  }
+  var puaLength = (PRIVATE_USE_OFFSET_END + 1) - PRIVATE_USE_OFFSET_START;
+  if (numChars > puaLength) {
+    throw new Error('Total number of chars must not exceed the PUA length.');
+  }
+  return {
+    numChars: numChars,
+    puaLength: puaLength,
+    percentage: 100 * (numChars / puaLength),
+  };
+}
+
+exports.checkProblematicCharRanges = checkProblematicCharRanges;
+//#endif
+
 /**
  * 'Font' is the class the outside world should use, it encapsulate all the font
  * decoding logics whatever type it is (assuming the font type is supported).
diff --git a/test/unit/fonts_spec.js b/test/unit/fonts_spec.js
new file mode 100644
index 000000000..92f2758ac
--- /dev/null
+++ b/test/unit/fonts_spec.js
@@ -0,0 +1,13 @@
+/* globals describe, it, expect, beforeAll, afterAll,
+           checkProblematicCharRanges */
+
+'use strict';
+
+describe('Fonts', function() {
+  it('checkProblematicCharRanges', function() {
+    var EXPECTED_PERCENTAGE = 45;
+    var result = checkProblematicCharRanges();
+
+    expect(result.percentage).toBeLessThan(EXPECTED_PERCENTAGE);
+  });
+});
diff --git a/test/unit/unit_test.html b/test/unit/unit_test.html
index 0ab0afafd..0b7a125e5 100644
--- a/test/unit/unit_test.html
+++ b/test/unit/unit_test.html
@@ -15,6 +15,7 @@
   <script src="primitives_spec.js"></script>
   <script src="cff_parser_spec.js"></script>
   <script src="type1_parser_spec.js"></script>
+  <script src="fonts_spec.js"></script>
   <script src="unicode_spec.js"></script>
   <script src="function_spec.js"></script>
   <script src="crypto_spec.js"></script>