buffer: update character validation approach in atob

* Small optimization in atob validation by assuming positions of whitespace characters in allowed chars array * Improved tests for ASCII whitespace characters * Explicitly validate properties of `DOMException` in tests
nodejs · Apr 9, 2022 · a5828d8 · a5828d8
1 parent a85b856
commit a5828d8
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 11 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -26,7 +26,7 @@ const {
   ArrayFrom,
   ArrayIsArray,
   ArrayPrototypeForEach,
-  ArrayPrototypeIncludes,
+  ArrayPrototypeIndexOf,
   MathFloor,
   MathMin,
   MathTrunc,
@@ -1232,14 +1232,12 @@ function btoa(input) {
   return buf.toString('base64');
 }
 
-const asciiWhitespaceCharacters = [
+// Refs: https://infra.spec.whatwg.org/#forgiving-base64-decode
+const kForgivingBase64AllowedChars = [
   // ASCII whitespace
   // Refs: https://infra.spec.whatwg.org/#ascii-whitespace
   0x09, 0x0A, 0x0C, 0x0D, 0x20,
-];
 
-// Refs: https://infra.spec.whatwg.org/#forgiving-base64-decode
-const kForgivingBase64AllowedChars = [
   // Uppercase letters
   ...ArrayFrom({ length: 26 }, (_, i) => StringPrototypeCharCodeAt('A') + i),
 
@@ -1266,11 +1264,13 @@ function atob(input) {
   let nonAsciiWhitespaceCharCount = 0;
 
   for (let n = 0; n < input.length; n++) {
-    const char = StringPrototypeCharCodeAt(input, n);
+    const index = ArrayPrototypeIndexOf(
+      kForgivingBase64AllowedChars,
+      StringPrototypeCharCodeAt(input, n));
 
-    if (ArrayPrototypeIncludes(kForgivingBase64AllowedChars, char)) {
+    if (index > 4) { // The first five char are ASCII whitespace.
       nonAsciiWhitespaceCharCount++;
-    } else if (!ArrayPrototypeIncludes(asciiWhitespaceCharacters, char)) {
+    } else if (index === -1) {
       throw lazyDOMException('Invalid character', 'InvalidCharacterError');
     }
   }

diff --git a/test/parallel/test-btoa-atob.js b/test/parallel/test-btoa-atob.js
@@ -14,7 +14,7 @@ throws(() => buffer.atob(), /TypeError/);
 throws(() => buffer.btoa(), /TypeError/);
 
 strictEqual(atob(' '), '');
-strictEqual(atob('  YW\tJ\njZA=\r= '), 'abcd');
+strictEqual(atob('  Y\fW\tJ\njZ A=\r= '), 'abcd');
 
 strictEqual(atob(null), '\x9Eée');
 strictEqual(atob(NaN), '5£');
@@ -26,5 +26,14 @@ strictEqual(atob({ toString: () => '' }), '');
 strictEqual(atob({ [Symbol.toPrimitive]: () => '' }), '');
 
 throws(() => atob(Symbol()), /TypeError/);
-[undefined, false, () => {}, 0, 1, 0n, 1n, -Infinity, [1], {}].forEach((value) =>
-  throws(() => atob(value), { constructor: DOMException }));
+[
+  undefined, false, () => {}, {}, [1],
+  0, 1, 0n, 1n, -Infinity,
+  'a', 'a\n\n\n', '\ra\r\r', '  a ', '\t\t\ta', 'a\f\f\f', '\ta\r \n\f',
+].forEach((value) =>
+  // See #2 - https://html.spec.whatwg.org/multipage/webappapis.html#dom-atob
+  throws(() => atob(value), {
+    constructor: DOMException,
+    name: 'InvalidCharacterError',
+    code: 5,
+  }));