assert: do not read Node.js modules

Prevent reading a Node.js module. That could theoretically lead to
false errors being thrown otherwise.

PR-URL: https://github.com/nodejs/node/pull/18322
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

lib/assert.js

@@ -25,13 +25,13 @@ const {
   isDeepEqual,
   isDeepStrictEqual
 } = require('internal/util/comparisons');
-const { AssertionError, TypeError } = require('internal/errors');
+const { AssertionError, TypeError, errorCache } = require('internal/errors');
 const { openSync, closeSync, readSync } = require('fs');
 const { parseExpressionAt } = require('internal/deps/acorn/dist/acorn');
 const { inspect } = require('util');
 const { EOL } = require('os');
+const nativeModule = require('native_module');
 
-const codeCache = new Map();
 // Escape control characters but not \n and \t to keep the line breaks and
 // indentation intact.
 // eslint-disable-next-line no-control-regex
@@ -146,6 +146,60 @@ function getBuffer(fd, assertLine) {
   return buffers;
 }
 
+function getErrMessage(call) {
+  const filename = call.getFileName();
+  const line = call.getLineNumber() - 1;
+  const column = call.getColumnNumber() - 1;
+  const identifier = `${filename}${line}${column}`;
+
+  if (errorCache.has(identifier)) {
+    return errorCache.get(identifier);
+  }
+
+  // Skip Node.js modules!
+  if (filename.endsWith('.js') && nativeModule.exists(filename.slice(0, -3))) {
+    errorCache.set(identifier, undefined);
+    return;
+  }
+
+  var fd;
+  try {
+    fd = openSync(filename, 'r', 0o666);
+    const buffers = getBuffer(fd, line);
+    const code = Buffer.concat(buffers).toString('utf8');
+    const nodes = parseExpressionAt(code, column);
+    // Node type should be "CallExpression" and some times
+    // "SequenceExpression".
+    const node = nodes.type === 'CallExpression' ? nodes : nodes.expressions[0];
+    const name = node.callee.name;
+    // Calling `ok` with .apply or .call is uncommon but we use a simple
+    // safeguard nevertheless.
+    if (name !== 'apply' && name !== 'call') {
+    // Only use `assert` and `assert.ok` to reference the "real API" and
+    // not user defined function names.
+      const ok = name === 'ok' ? '.ok' : '';
+      const args = node.arguments;
+      var message = code
+        .slice(args[0].start, args[args.length - 1].end)
+        .replace(escapeSequencesRegExp, escapeFn);
+      message = 'The expression evaluated to a falsy value:' +
+        `${EOL}${EOL}  assert${ok}(${message})${EOL}`;
+    }
+    // Make sure to always set the cache! No matter if the message is
+    // undefined or not
+    errorCache.set(identifier, message);
+
+    return message;
+
+  } catch (e) {
+  // Invalidate cache to prevent trying to read this part again.
+    errorCache.set(identifier, undefined);
+  } finally {
+    if (fd !== undefined)
+      closeSync(fd);
+  }
+}
+
 function innerOk(args, fn) {
   var [value, message] = args;
 
@@ -168,54 +222,12 @@ function innerOk(args, fn) {
       const call = err.stack[0];
       Error.prepareStackTrace = tmpPrepare;
 
-      const filename = call.getFileName();
+      // TODO(BridgeAR): fix the "generatedMessage property"
-      const line = call.getLineNumber() - 1;
+      // Since this is actually a generated message, it has to be
-      const column = call.getColumnNumber() - 1;
+      // determined differently from now on.
-      const identifier = `${filename}${line}${column}`;
 
-      if (codeCache.has(identifier)) {
+      // Make sure it would be "null" in case that is used.
-        message = codeCache.get(identifier);
+      message = getErrMessage(call) || message;
-      } else {
-        var fd;
-        try {
-          fd = openSync(filename, 'r', 0o666);
-          const buffers = getBuffer(fd, line);
-          const code = Buffer.concat(buffers).toString('utf8');
-          const nodes = parseExpressionAt(code, column);
-          // Node type should be "CallExpression" and some times
-          // "SequenceExpression".
-          const node = nodes.type === 'CallExpression' ?
-            nodes :
-            nodes.expressions[0];
-          // TODO: fix the "generatedMessage property"
-          // Since this is actually a generated message, it has to be
-          // determined differently from now on.
-
-          const name = node.callee.name;
-          // Calling `ok` with .apply or .call is uncommon but we use a simple
-          // safeguard nevertheless.
-          if (name !== 'apply' && name !== 'call') {
-            // Only use `assert` and `assert.ok` to reference the "real API" and
-            // not user defined function names.
-            const ok = name === 'ok' ? '.ok' : '';
-            const args = node.arguments;
-            message = code
-              .slice(args[0].start, args[args.length - 1].end)
-              .replace(escapeSequencesRegExp, escapeFn);
-            message = 'The expression evaluated to a falsy value:' +
-              `${EOL}${EOL}  assert${ok}(${message})${EOL}`;
-          }
-          // Make sure to always set the cache! No matter if the message is
-          // undefined or not
-          codeCache.set(identifier, message);
-        } catch (e) {
-          // Invalidate cache to prevent trying to read this part again.
-          codeCache.set(identifier, undefined);
-        } finally {
-          if (fd !== undefined)
-            closeSync(fd);
-        }
-      }
     }
     innerFail({
       actual: value,

lib/internal/errors.js

@@ -398,7 +398,8 @@ module.exports = exports = {
   URIError: makeNodeError(URIError),
   AssertionError,
   SystemError,
-  E // This is exported only to facilitate testing.
+  E, // This is exported only to facilitate testing.
+  errorCache: new Map() // This is in here only to facilitate testing.
 };
 
 // To declare an error message, use the E(sym, val) function above. The sym

test/parallel/test-assert.js

@@ -19,6 +19,8 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+// Flags: --expose-internals
+
 'use strict';
 
 /* eslint-disable prefer-common-expectserror */
@@ -26,6 +28,9 @@
 const common = require('../common');
 const assert = require('assert');
 const { EOL } = require('os');
+const EventEmitter = require('events');
+const { errorCache } = require('internal/errors');
+const { writeFileSync, unlinkSync } = require('fs');
 const a = assert;
 
 function makeBlock(f) {
@@ -1019,6 +1024,38 @@ common.expectsError(
   }
 );
 
+// Do not try to check Node.js modules.
+{
+  const e = new EventEmitter();
+
+  e.on('hello', assert);
+
+  let threw = false;
+  try {
+    e.emit('hello', false);
+  } catch (err) {
+    const frames = err.stack.split('\n');
+    const [, filename, line, column] = frames[1].match(/\((.+):(\d+):(\d+)\)/);
+    // Reset the cache to check again
+    errorCache.delete(`${filename}${line - 1}${column - 1}`);
+    const data = `${'\n'.repeat(line - 1)}${' '.repeat(column - 1)}` +
+                 'ok(failed(badly));';
+    try {
+      writeFileSync(filename, data);
+      assert.throws(
+        () => e.emit('hello', false),
+        {
+          message: 'false == true'
+        }
+      );
+      threw = true;
+    } finally {
+      unlinkSync(filename);
+    }
+  }
+  assert(threw);
+}
+
 common.expectsError(
   // eslint-disable-next-line no-restricted-syntax
   () => assert.throws(() => {}, 'Error message', 'message'),