console: prevent constructing console methods

Ref: https://github.com/nodejs/node/issues/25987

PR-URL: https://github.com/nodejs/node/pull/26096
Refs: https://github.com/nodejs/node/issues/25987
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>

lib/internal/console/constructor.js

@@ -279,29 +279,26 @@ Console.prototype[kFormatForStderr] = function(args) {
   return util.formatWithOptions(opts, ...args);
 };
 
-Console.prototype.log = function log(...args) {
+const consoleMethods = {
+  log(...args) {
     this[kWriteToConsole](kUseStdout, this[kFormatForStdout](args));
-};
+  },
 
-Console.prototype.debug = Console.prototype.log;
-Console.prototype.info = Console.prototype.log;
-Console.prototype.dirxml = Console.prototype.log;
 
-Console.prototype.warn = function warn(...args) {
+  warn(...args) {
     this[kWriteToConsole](kUseStderr, this[kFormatForStderr](args));
-};
+  },
 
-Console.prototype.error = Console.prototype.warn;
 
-Console.prototype.dir = function dir(object, options) {
+  dir(object, options) {
     this[kWriteToConsole](kUseStdout, util.inspect(object, {
       customInspect: false,
       ...this[kGetInspectOptions](this._stdout),
       ...options
     }));
-};
+  },
 
-Console.prototype.time = function time(label = 'default') {
+  time(label = 'default') {
     // Coerces everything other than Symbol to a string
     label = `${label}`;
     if (this._times.has(label)) {
@@ -310,9 +307,9 @@ Console.prototype.time = function time(label = 'default') {
     }
     trace(kTraceBegin, kTraceConsoleCategory, `time::${label}`, 0);
     this._times.set(label, process.hrtime());
-};
+  },
 
-Console.prototype.timeEnd = function timeEnd(label = 'default') {
+  timeEnd(label = 'default') {
     // Coerces everything other than Symbol to a string
     label = `${label}`;
     const hasWarned = timeLogImpl(this, 'timeEnd', label);
@@ -320,50 +317,33 @@ Console.prototype.timeEnd = function timeEnd(label = 'default') {
     if (!hasWarned) {
       this._times.delete(label);
     }
-};
+  },
 
-Console.prototype.timeLog = function timeLog(label = 'default', ...data) {
+  timeLog(label = 'default', ...data) {
     // Coerces everything other than Symbol to a string
     label = `${label}`;
     timeLogImpl(this, 'timeLog', label, data);
     trace(kTraceInstant, kTraceConsoleCategory, `time::${label}`, 0);
-};
+  },
 
-// Returns true if label was not found
-function timeLogImpl(self, name, label, data) {
-  const time = self._times.get(label);
-  if (!time) {
-    process.emitWarning(`No such label '${label}' for console.${name}()`);
-    return true;
-  }
-  const duration = process.hrtime(time);
-  const ms = duration[0] * 1000 + duration[1] / 1e6;
-  if (data === undefined) {
-    self.log('%s: %sms', label, ms.toFixed(3));
-  } else {
-    self.log('%s: %sms', label, ms.toFixed(3), ...data);
-  }
-  return false;
-}
-
-Console.prototype.trace = function trace(...args) {
+  trace(...args) {
     const err = {
       name: 'Trace',
       message: this[kFormatForStderr](args)
     };
     Error.captureStackTrace(err, trace);
     this.error(err.stack);
-};
+  },
 
-Console.prototype.assert = function assert(expression, ...args) {
+  assert(expression, ...args) {
     if (!expression) {
       args[0] = `Assertion failed${args.length === 0 ? '' : `: ${args[0]}`}`;
       this.warn(...args);  // The arguments will be formatted in warn() again
     }
-};
+  },
 
   // Defined by: https://console.spec.whatwg.org/#clear
-Console.prototype.clear = function clear() {
+  clear() {
     // It only makes sense to clear if _stdout is a TTY.
     // Otherwise, do nothing.
     if (this._stdout.isTTY) {
@@ -373,10 +353,10 @@ Console.prototype.clear = function clear() {
       cursorTo(this._stdout, 0, 0);
       clearScreenDown(this._stdout);
     }
-};
+  },
 
   // Defined by: https://console.spec.whatwg.org/#count
-Console.prototype.count = function count(label = 'default') {
+  count(label = 'default') {
     // Ensures that label is a string, and only things that can be
     // coerced to strings. e.g. Symbol is not allowed
     label = `${label}`;
@@ -389,10 +369,10 @@ Console.prototype.count = function count(label = 'default') {
     counts.set(label, count);
     trace(kTraceCount, kTraceConsoleCategory, `count::${label}`, 0, count);
     this.log(`${label}: ${count}`);
-};
+  },
 
   // Defined by: https://console.spec.whatwg.org/#countreset
-Console.prototype.countReset = function countReset(label = 'default') {
+  countReset(label = 'default') {
     const counts = this[kCounts];
     if (!counts.has(label)) {
       process.emitWarning(`Count for '${label}' does not exist`);
@@ -400,30 +380,22 @@ Console.prototype.countReset = function countReset(label = 'default') {
     }
     trace(kTraceCount, kTraceConsoleCategory, `count::${label}`, 0, 0);
     counts.delete(`${label}`);
-};
+  },
 
-Console.prototype.group = function group(...data) {
+  group(...data) {
     if (data.length > 0) {
       this.log(...data);
     }
     this[kGroupIndent] += '  ';
-};
-Console.prototype.groupCollapsed = Console.prototype.group;
+  },
 
-Console.prototype.groupEnd = function groupEnd() {
+  groupEnd() {
     this[kGroupIndent] =
       this[kGroupIndent].slice(0, this[kGroupIndent].length - 2);
-};
-
-const keyKey = 'Key';
-const valuesKey = 'Values';
-const indexKey = '(index)';
-const iterKey = '(iteration index)';
-
-const isArray = (v) => ArrayIsArray(v) || isTypedArray(v) || isBuffer(v);
+  },
 
   // https://console.spec.whatwg.org/#table
-Console.prototype.table = function(tabularData, properties) {
+  table(tabularData, properties) {
     if (properties !== undefined && !ArrayIsArray(properties))
       throw new ERR_INVALID_ARG_TYPE('properties', 'Array', properties);
 
@@ -445,7 +417,8 @@ Console.prototype.table = function(tabularData, properties) {
       };
       return util.inspect(v, opt);
     };
-  const getIndexArray = (length) => ArrayFrom({ length }, (_, i) => inspect(i));
+    const getIndexArray = (length) => ArrayFrom(
+      { length }, (_, i) => inspect(i));
 
     const mapIter = isMapIterator(tabularData);
     let isKeyValue = false;
@@ -535,10 +508,44 @@ Console.prototype.table = function(tabularData, properties) {
     values.unshift(indexKeyArray);
 
     return final(keys, values);
+  },
 };
 
+// Returns true if label was not found
+function timeLogImpl(self, name, label, data) {
+  const time = self._times.get(label);
+  if (!time) {
+    process.emitWarning(`No such label '${label}' for console.${name}()`);
+    return true;
+  }
+  const duration = process.hrtime(time);
+  const ms = duration[0] * 1000 + duration[1] / 1e6;
+  if (data === undefined) {
+    self.log('%s: %sms', label, ms.toFixed(3));
+  } else {
+    self.log('%s: %sms', label, ms.toFixed(3), ...data);
+  }
+  return false;
+}
+
+const keyKey = 'Key';
+const valuesKey = 'Values';
+const indexKey = '(index)';
+const iterKey = '(iteration index)';
+
+const isArray = (v) => ArrayIsArray(v) || isTypedArray(v) || isBuffer(v);
+
 function noop() {}
 
+for (const method of Reflect.ownKeys(consoleMethods))
+  Console.prototype[method] = consoleMethods[method];
+
+Console.prototype.debug = Console.prototype.log;
+Console.prototype.info = Console.prototype.log;
+Console.prototype.dirxml = Console.prototype.log;
+Console.prototype.error = Console.prototype.warn;
+Console.prototype.groupCollapsed = Console.prototype.group;
+
 module.exports = {
   Console,
   kBindStreamsLazy,

src/inspector_js_api.cc

@@ -272,7 +272,17 @@ void Initialize(Local<Object> target, Local<Value> unused,
   Environment* env = Environment::GetCurrent(context);
 
   Agent* agent = env->inspector_agent();
-  env->SetMethod(target, "consoleCall", InspectorConsoleCall);
+
+  v8::Local<v8::Function> consoleCallFunc =
+      env->NewFunctionTemplate(InspectorConsoleCall, v8::Local<v8::Signature>(),
+                               v8::ConstructorBehavior::kThrow,
+                               v8::SideEffectType::kHasSideEffect)
+          ->GetFunction(context)
+          .ToLocalChecked();
+  auto name_string = FIXED_ONE_BYTE_STRING(env->isolate(), "consoleCall");
+  target->Set(context, name_string, consoleCallFunc).FromJust();
+  consoleCallFunc->SetName(name_string);
+
   env->SetMethod(
       target, "setConsoleExtensionInstaller", SetConsoleExtensionInstaller);
   if (agent->WillWaitForConnect())

test/parallel/test-console-methods.js

@@ -0,0 +1,40 @@
+'use strict';
+require('../common');
+
+// This test ensures that console methods
+// cannot be invoked as constructors
+
+const assert = require('assert');
+
+const { Console } = console;
+const newInstance = new Console(process.stdout);
+const err = TypeError;
+
+const methods = [
+  'log',
+  'warn',
+  'dir',
+  'time',
+  'timeEnd',
+  'timeLog',
+  'trace',
+  'assert',
+  'clear',
+  'count',
+  'countReset',
+  'group',
+  'groupEnd',
+  'table',
+  'debug',
+  'info',
+  'dirxml',
+  'error',
+  'groupCollapsed',
+];
+
+for (const method of methods) {
+  assert.throws(() => new console[method](), err);
+  assert.throws(() => new newInstance[method](), err);
+  assert.throws(() => Reflect.construct({}, [], console[method]), err);
+  assert.throws(() => Reflect.construct({}, [], newInstance[method]), err);
+}