policy: minor perf opts and cleanup

bmeck · targos · commit 95431eace9fc · 2019-09-20T09:30:19.000+02:00
PR-URL: #29322 Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1124,7 +1124,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_CANNOT_PIPE', 'Cannot pipe, not readable', Error);
 E('ERR_STREAM_DESTROYED', 'Cannot call %s after a stream was destroyed', Error);
diff --git 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
@@ -848,7 +848,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
@@ -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
@@ -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
@@ -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
@@ -52,6 +52,6 @@ module.exports = Object.freeze({
   },
 
   assertIntegrity(moduleURL, content) {
-    this.manifest.matchesIntegrity(moduleURL, content);
+    this.manifest.assertIntegrity(moduleURL, content);
   }
 });

Original file line number	Diff line number	Diff line change
`@@ -848,7 +848,7 @@ Module.prototype._compile = function(content, filename) {`
`848`	`848`	`let redirects;`
`849`	`849`	`if (manifest) {`
`850`	`850`	`moduleURL = pathToFileURL(filename);`
`851`		`- redirects = manifest.getRedirects(moduleURL);`
	`851`	`+ redirects = manifest.getRedirector(moduleURL);`
`852`	`852`	`manifest.assertIntegrity(moduleURL, content);`
`853`	`853`	`}`
`854`	`854`
Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,6 @@ module.exports = Object.freeze({`
`52`	`52`	`},`
`53`	`53`
`54`	`54`	`assertIntegrity(moduleURL, content) {`
`55`		`- this.manifest.matchesIntegrity(moduleURL, content);`
	`55`	`+ this.manifest.assertIntegrity(moduleURL, content);`
`56`	`56`	`}`
`57`	`57`	`});`