Skip to content

Commit

Permalink
buffer: update character validation approach in atob
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
austinkelleher committed Apr 9, 2022
1 parent a85b856 commit a5828d8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
16 changes: 8 additions & 8 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const {
ArrayFrom,
ArrayIsArray,
ArrayPrototypeForEach,
ArrayPrototypeIncludes,
ArrayPrototypeIndexOf,
MathFloor,
MathMin,
MathTrunc,
Expand Down Expand Up @@ -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),

Expand All @@ -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');
}
}
Expand Down
15 changes: 12 additions & 3 deletions test/parallel/test-btoa-atob.js
Original file line number Diff line number Diff line change
Expand Up @@ -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£');
Expand All @@ -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,
}));

0 comments on commit a5828d8

Please sign in to comment.