From ac3298a7d061b0f8f9b8ea1d39372714a9c5d88c Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Mon, 26 Aug 2019 10:40:45 -0500 Subject: [PATCH] policy: minor perf opts and cleanup --- lib/internal/errors.js | 2 +- lib/internal/modules/cjs/helpers.js | 44 ++++---- lib/internal/modules/cjs/loader.js | 2 +- lib/internal/per_context/primordials.js | 14 ++- lib/internal/policy/manifest.js | 137 +++++++++++++++--------- lib/internal/policy/sri.js | 12 +-- lib/internal/process/policy.js | 2 +- 7 files changed, 131 insertions(+), 82 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 56c7a3bdbed5de..c478e83f60bd2e 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1129,7 +1129,7 @@ E('ERR_SOCKET_DGRAM_IS_CONNECTED', 'Already connected', Error); E('ERR_SOCKET_DGRAM_NOT_CONNECTED', 'Not connected', Error); E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running', Error); E('ERR_SRI_PARSE', - 'Subresource Integrity string %s had an unexpected at %d', + 'Subresource Integrity string %j had an unexpected %j at position %d', SyntaxError); E('ERR_STREAM_ALREADY_FINISHED', 'Cannot call %s after a stream was finished', diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index 5e0e1b06ae3a1c..7a9870024515ed 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -1,6 +1,6 @@ 'use strict'; -const { Object } = primordials; +const { Object, SafeMap } = primordials; const { ERR_MANIFEST_DEPENDENCY_MISSING, ERR_UNKNOWN_BUILTIN_MODULE @@ -28,34 +28,40 @@ function loadNativeModule(filename, request, experimentalModules) { // Invoke with makeRequireFunction(module) where |module| is the Module object // to use as the context for the require() function. // Use redirects to set up a mapping from a policy and restrict dependencies +const urlToFileCache = new SafeMap(); function makeRequireFunction(mod, redirects) { const Module = mod.constructor; let require; if (redirects) { - const { map, reaction } = redirects; + const { resolve, reaction } = redirects; const id = mod.filename || mod.id; require = function require(path) { let missing = true; - if (map === true) { + const destination = resolve(path); + if (destination === true) { missing = false; - } else if (map.has(path)) { - const redirect = map.get(path); - if (redirect === true) { - missing = false; - } else if (typeof redirect === 'string') { - const parsed = new URL(redirect); - if (parsed.protocol === 'node:') { - const specifier = parsed.pathname; - const mod = loadNativeModule( - specifier, - redirect, - experimentalModules); - if (mod && mod.canBeRequiredByUsers) return mod.exports; - throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier); - } else if (parsed.protocol === 'file:') { - return mod.require(fileURLToPath(parsed)); + } else if (destination) { + const href = destination.href; + if (destination.protocol === 'node:') { + const specifier = destination.pathname; + const mod = loadNativeModule( + specifier, + href, + experimentalModules); + if (mod && mod.canBeRequiredByUsers) { + return mod.exports; } + throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier); + } else if (destination.protocol === 'file:') { + let filepath; + if (urlToFileCache.has(href)) { + filepath = urlToFileCache.get(href); + } else { + filepath = fileURLToPath(destination); + urlToFileCache.set(href, filepath); + } + return mod.require(filepath); } } if (missing) { diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 3508aec36a0894..db93c6b787baca 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -899,7 +899,7 @@ Module.prototype._compile = function(content, filename) { let redirects; if (manifest) { moduleURL = pathToFileURL(filename); - redirects = manifest.getRedirects(moduleURL); + redirects = manifest.getRedirector(moduleURL); manifest.assertIntegrity(moduleURL, content); } diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 8d2150b7b7690a..7e02a9e3162d9c 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -13,6 +13,7 @@ // by the native module compiler. const ReflectApply = Reflect.apply; +const ReflectConstruct = Reflect.construct; // This function is borrowed from the function with the same name on V8 Extras' // `utils` object. V8 implements Reflect.apply very efficiently in conjunction @@ -102,10 +103,17 @@ primordials.SafePromise = makeSafe( 'String', 'Symbol', ].forEach((name) => { - const target = primordials[name] = Object.create(null); - copyProps(global[name], target); + const original = global[name]; + const target = primordials[name] = Object.setPrototypeOf({ + [name]: function(...args) { + return new.target ? + ReflectConstruct(original, args, new.target) : + ReflectApply(original, this, args); + } + }[name], null); + copyProps(original, target); const proto = primordials[name + 'Prototype'] = Object.create(null); - copyPrototype(global[name].prototype, proto); + copyPrototype(original.prototype, proto); }); Object.setPrototypeOf(primordials, null); diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js index f91eb5b0129d48..118a89efa6f88a 100644 --- a/lib/internal/policy/manifest.js +++ b/lib/internal/policy/manifest.js @@ -1,10 +1,11 @@ 'use strict'; const { - SafeMap, - SafeWeakMap, + Map, + MapPrototype, Object, RegExpPrototype, + SafeMap, uncurryThis } = primordials; const { @@ -21,16 +22,13 @@ const debug = require('internal/util/debuglog').debuglog('policy'); const SRI = require('internal/policy/sri'); const crypto = require('crypto'); const { Buffer } = require('buffer'); -const { URL } = require('url'); +const { URL } = require('internal/url'); const { createHash, timingSafeEqual } = crypto; const HashUpdate = uncurryThis(crypto.Hash.prototype.update); const HashDigest = uncurryThis(crypto.Hash.prototype.digest); const BufferEquals = uncurryThis(Buffer.prototype.equals); const BufferToString = uncurryThis(Buffer.prototype.toString); const { entries } = Object; -const kIntegrities = new SafeWeakMap(); -const kDependencies = new SafeWeakMap(); -const kReactions = new SafeWeakMap(); const kRelativeURLStringPattern = /^\.{0,2}\//; const { getOptionValue } = require('internal/options'); const shouldAbortOnUncaughtException = @@ -54,13 +52,12 @@ function REACTION_LOG(error) { } class Manifest { + #integrities = new SafeMap(); + #dependencies = new SafeMap(); + #reaction = null; constructor(obj, manifestURL) { - const integrities = { - __proto__: null, - }; - const dependencies = { - __proto__: null, - }; + const integrities = this.#integrities; + const dependencies = this.#dependencies; let reaction = REACTION_THROW; if (obj.onerror) { @@ -75,23 +72,33 @@ class Manifest { } } - kReactions.set(this, reaction); + this.#reaction = reaction; const manifestEntries = entries(obj.resources); + const parsedURLs = new SafeMap(); for (var i = 0; i < manifestEntries.length; i++) { - let url = manifestEntries[i][0]; - const originalURL = url; - if (RegExpPrototype.test(kRelativeURLStringPattern, url)) { - url = new URL(url, manifestURL).href; + let resourceHREF = manifestEntries[i][0]; + const originalHREF = resourceHREF; + let resourceURL; + if (parsedURLs.has(resourceHREF)) { + resourceURL = parsedURLs.get(resourceHREF); + resourceHREF = resourceURL.href; + } else if ( + RegExpPrototype.test(kRelativeURLStringPattern, resourceHREF) + ) { + resourceURL = new URL(resourceHREF, manifestURL); + resourceHREF = resourceURL.href; + parsedURLs.set(originalHREF, resourceURL); + parsedURLs.set(resourceHREF, resourceURL); } let integrity = manifestEntries[i][1].integrity; if (!integrity) integrity = null; if (integrity != null) { - debug(`Manifest contains integrity for url ${originalURL}`); + debug(`Manifest contains integrity for url ${originalHREF}`); if (typeof integrity === 'string') { const sri = Object.freeze(SRI.parse(integrity)); - if (url in integrities) { - const old = integrities[url]; + if (integrities.has(resourceHREF)) { + const old = integrities.get(resourceHREF); let mismatch = false; if (old.length !== sri.length) { @@ -112,14 +119,16 @@ class Manifest { } if (mismatch) { - throw new ERR_MANIFEST_INTEGRITY_MISMATCH(url); + throw new ERR_MANIFEST_INTEGRITY_MISMATCH(resourceURL); } } - integrities[url] = sri; + integrities.set(resourceHREF, sri); } else if (integrity === true) { - integrities[url] = true; + integrities.set(resourceHREF, true); } else { - throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(url, 'integrity'); + throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD( + resourceHREF, + 'integrity'); } } @@ -128,50 +137,72 @@ class Manifest { dependencyMap = {}; } if (typeof dependencyMap === 'object' && !Array.isArray(dependencyMap)) { - dependencies[url] = new SafeMap(Object.entries(dependencyMap).map( - ([ from, to ]) => { + /** + * @returns {true | URL} + */ + const dependencyRedirectList = (toSpecifier) => { + if (toSpecifier in dependencyMap !== true) { + return null; + } else { + const to = dependencyMap[toSpecifier]; if (to === true) { - return [from, to]; + return true; } - if (canBeRequiredByUsers(to)) { - return [from, `node:${to}`]; + if (parsedURLs.has(to)) { + return parsedURLs.get(to); + } else if (canBeRequiredByUsers(to)) { + const href = `node:${to}`; + const resolvedURL = new URL(href); + parsedURLs.set(to, resolvedURL); + parsedURLs.set(href, resolvedURL); + return resolvedURL; } else if (RegExpPrototype.test(kRelativeURLStringPattern, to)) { - return [from, new URL(to, manifestURL).href]; + const resolvedURL = new URL(to, manifestURL); + const href = resourceURL.href; + parsedURLs.set(to, resolvedURL); + parsedURLs.set(href, resolvedURL); + return resolvedURL; } - return [from, new URL(to).href]; - }) - ); + const resolvedURL = new URL(to); + const href = resourceURL.href; + parsedURLs.set(to, resolvedURL); + parsedURLs.set(href, resolvedURL); + return resolvedURL; + } + }; + dependencies.set(resourceHREF, dependencyRedirectList); } else if (dependencyMap === true) { - dependencies[url] = true; + const arbitraryDependencies = () => true; + dependencies.set(resourceHREF, arbitraryDependencies); } else { - throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(url, 'dependencies'); + throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD( + resourceHREF, + 'dependencies'); } } - Object.freeze(integrities); - kIntegrities.set(this, integrities); - Object.freeze(dependencies); - kDependencies.set(this, dependencies); Object.freeze(this); } - getRedirects(requester) { - const dependencies = kDependencies.get(this); - if (dependencies && requester in dependencies) { + getRedirector(requester) { + requester = `${requester}`; + const dependencies = this.#dependencies; + if (dependencies.has(requester)) { return { - map: dependencies[requester], - reaction: kReactions.get(this) + resolve: (to) => dependencies.get(requester)(`${to}`), + reaction: this.#reaction }; } return null; } assertIntegrity(url, content) { - debug(`Checking integrity of ${url}`); - const integrities = kIntegrities.get(this); - const realIntegrities = new SafeMap(); + const href = `${url}`; + debug(`Checking integrity of ${href}`); + const integrities = this.#integrities; + const realIntegrities = new Map(); - if (integrities && url in integrities) { - const integrityEntries = integrities[url]; + if (integrities.has(href)) { + const integrityEntries = integrities.get(href); if (integrityEntries === true) return true; // Avoid clobbered Symbol.iterator for (var i = 0; i < integrityEntries.length; i++) { @@ -186,11 +217,15 @@ class Manifest { timingSafeEqual(digest, expected)) { return true; } - realIntegrities.set(algorithm, BufferToString(digest, 'base64')); + MapPrototype.set( + realIntegrities, + algorithm, + BufferToString(digest, 'base64') + ); } } const error = new ERR_MANIFEST_ASSERT_INTEGRITY(url, realIntegrities); - kReactions.get(this)(error); + this.#reaction(error); } } diff --git a/lib/internal/policy/sri.js b/lib/internal/policy/sri.js index 55c3c0c507d7db..877c96a6f7b7d6 100644 --- a/lib/internal/policy/sri.js +++ b/lib/internal/policy/sri.js @@ -13,16 +13,16 @@ const { } = require('internal/errors').codes; const kWSP = '[\\x20\\x09]'; const kVCHAR = '[\\x21-\\x7E]'; -const kHASH_ALGO = 'sha256|sha384|sha512'; +const kHASH_ALGO = 'sha(?:256|384|512)'; // Base64 const kHASH_VALUE = '[A-Za-z0-9+/]+[=]{0,2}'; const kHASH_EXPRESSION = `(${kHASH_ALGO})-(${kHASH_VALUE})`; const kOPTION_EXPRESSION = `(${kVCHAR}*)`; const kHASH_WITH_OPTIONS = `${kHASH_EXPRESSION}(?:[?](${kOPTION_EXPRESSION}))?`; -const kSRIPattern = new RegExp(`(${kWSP}*)(?:${kHASH_WITH_OPTIONS})`, 'g'); +const kSRIPattern = RegExp(`(${kWSP}*)(?:${kHASH_WITH_OPTIONS})`, 'g'); const { freeze } = Object; Object.seal(kSRIPattern); -const kAllWSP = new RegExp(`^${kWSP}*$`); +const kAllWSP = RegExp(`^${kWSP}*$`); Object.seal(kAllWSP); const BufferFrom = require('buffer').Buffer.from; @@ -34,10 +34,10 @@ const parse = (str) => { const entries = []; while (match = RegExpPrototype.exec(kSRIPattern, str)) { if (match.index !== prevIndex) { - throw new ERR_SRI_PARSE(str, prevIndex); + throw new ERR_SRI_PARSE(str, str.charAt(prevIndex), prevIndex); } if (entries.length > 0 && match[1] === '') { - throw new ERR_SRI_PARSE(str, prevIndex); + throw new ERR_SRI_PARSE(str, str.charAt(prevIndex), prevIndex); } // Avoid setters being fired @@ -56,7 +56,7 @@ const parse = (str) => { if (prevIndex !== str.length) { if (!RegExpPrototype.test(kAllWSP, StringPrototype.slice(str, prevIndex))) { - throw new ERR_SRI_PARSE(str, prevIndex); + throw new ERR_SRI_PARSE(str, str.charAt(prevIndex), prevIndex); } } return entries; diff --git a/lib/internal/process/policy.js b/lib/internal/process/policy.js index 215e1420af1d1d..d2501403c39951 100644 --- a/lib/internal/process/policy.js +++ b/lib/internal/process/policy.js @@ -52,6 +52,6 @@ module.exports = Object.freeze({ }, assertIntegrity(moduleURL, content) { - this.manifest.matchesIntegrity(moduleURL, content); + this.manifest.assertIntegrity(moduleURL, content); } });