From 0640b09243358f203af64563de093d2b8cbd8d2e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 21 Jan 2019 22:40:55 +0100 Subject: [PATCH] lib: refactor policy code for readability Simplify a few particularly quirky bits of code, and add whitespace for readability. PR-URL: https://github.com/nodejs/node/pull/25629 Reviewed-By: Denys Otrishko Reviewed-By: Colin Ihrig Reviewed-By: Gus Caplan Reviewed-By: Anto Aravinth Reviewed-By: Sakthipriyan Vairamani --- lib/internal/policy/manifest.js | 13 +++++++++++++ lib/internal/policy/sri.js | 22 ++++++++++------------ lib/internal/process/policy.js | 4 ++++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js index 272abf2457ddc3..d0d3431b546073 100644 --- a/lib/internal/policy/manifest.js +++ b/lib/internal/policy/manifest.js @@ -22,9 +22,11 @@ const kReactions = new SafeWeakMap(); const kRelativeURLStringPattern = /^\.{0,2}\//; const { shouldAbortOnUncaughtException } = internalBinding('config'); const { abort, exit, _rawDebug } = process; + function REACTION_THROW(error) { throw error; } + function REACTION_EXIT(error) { REACTION_LOG(error); if (shouldAbortOnUncaughtException) { @@ -32,9 +34,11 @@ function REACTION_EXIT(error) { } exit(1); } + function REACTION_LOG(error) { _rawDebug(error.stack); } + class Manifest { constructor(obj, manifestURL) { const integrities = { @@ -44,6 +48,7 @@ class Manifest { __proto__: null, integrity: REACTION_THROW, }; + if (obj.onerror) { const behavior = obj.onerror; if (behavior === 'throw') { @@ -55,8 +60,10 @@ class Manifest { throw new ERR_MANIFEST_UNKNOWN_ONERROR(behavior); } } + kReactions.set(this, Object.freeze(reactions)); const manifestEntries = entries(obj.resources); + for (var i = 0; i < manifestEntries.length; i++) { let url = manifestEntries[i][0]; const integrity = manifestEntries[i][1].integrity; @@ -65,10 +72,12 @@ class Manifest { if (RegExpTest(kRelativeURLStringPattern, url)) { url = new URL(url, manifestURL).href; } + const sri = Object.freeze(SRI.parse(integrity)); if (url in integrities) { const old = integrities[url]; let mismatch = false; + if (old.length !== sri.length) { mismatch = true; } else { @@ -85,6 +94,7 @@ class Manifest { break compare; } } + if (mismatch) { throw new ERR_MANIFEST_INTEGRITY_MISMATCH(url); } @@ -96,10 +106,12 @@ class Manifest { kIntegrities.set(this, integrities); Object.freeze(this); } + assertIntegrity(url, content) { debug(`Checking integrity of ${url}`); const integrities = kIntegrities.get(this); const realIntegrities = new Map(); + if (integrities && url in integrities) { const integrityEntries = integrities[url]; // Avoid clobbered Symbol.iterator @@ -122,6 +134,7 @@ class Manifest { kReactions.get(this).integrity(error); } } + // Lock everything down to avoid problems even if reference is leaked somehow Object.setPrototypeOf(Manifest, null); Object.setPrototypeOf(Manifest.prototype, null); diff --git a/lib/internal/policy/sri.js b/lib/internal/policy/sri.js index fff4e066b17451..1cdec8d739eb99 100644 --- a/lib/internal/policy/sri.js +++ b/lib/internal/policy/sri.js @@ -18,29 +18,27 @@ const { freeze } = Object; Object.seal(kSRIPattern); const kAllWSP = new RegExp(`^${kWSP}*$`); Object.seal(kAllWSP); + const RegExpExec = Function.call.bind(RegExp.prototype.exec); const RegExpTest = Function.call.bind(RegExp.prototype.test); const StringSlice = Function.call.bind(String.prototype.slice); -const { - Buffer: { - from: BufferFrom - } -} = require('buffer'); + +const BufferFrom = require('buffer').Buffer.from; const { defineProperty } = Object; + const parse = (str) => { kSRIPattern.lastIndex = 0; let prevIndex = 0; - let match = RegExpExec(kSRIPattern, str); + let match; const entries = []; - while (match) { + while (match = RegExpExec(kSRIPattern, str)) { if (match.index !== prevIndex) { throw new ERR_SRI_PARSE(str, prevIndex); } - if (entries.length > 0) { - if (match[1] === '') { - throw new ERR_SRI_PARSE(str, prevIndex); - } + if (entries.length > 0 && match[1] === '') { + throw new ERR_SRI_PARSE(str, prevIndex); } + // Avoid setters being fired defineProperty(entries, entries.length, { enumerable: true, @@ -53,8 +51,8 @@ const parse = (str) => { }) }); prevIndex = prevIndex + match[0].length; - match = RegExpExec(kSRIPattern, str); } + if (prevIndex !== str.length) { if (!RegExpTest(kAllWSP, StringSlice(str, prevIndex))) { throw new ERR_SRI_PARSE(str, prevIndex); diff --git a/lib/internal/process/policy.js b/lib/internal/process/policy.js index f5ca4eeb07a3e0..0b037d4ef53cf4 100644 --- a/lib/internal/process/policy.js +++ b/lib/internal/process/policy.js @@ -5,6 +5,7 @@ const { } = require('internal/errors').codes; const { Manifest } = require('internal/policy/manifest'); let manifest; + module.exports = Object.freeze({ __proto__: null, setup(src, url) { @@ -12,6 +13,7 @@ module.exports = Object.freeze({ manifest = null; return; } + const json = JSON.parse(src, (_, o) => { if (o && typeof o === 'object') { Reflect.setPrototypeOf(o, null); @@ -21,12 +23,14 @@ module.exports = Object.freeze({ }); manifest = new Manifest(json, url); }, + get manifest() { if (typeof manifest === 'undefined') { throw new ERR_MANIFEST_TDZ(); } return manifest; }, + assertIntegrity(moduleURL, content) { this.manifest.matchesIntegrity(moduleURL, content); }