From a70bafb3cc88b39cdf0fcfa7de6223f4435751a8 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 10 Feb 2019 12:39:23 +0100 Subject: [PATCH] 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 Reviewed-By: Gabriel Schulhof --- lib/internal/console/constructor.js | 451 +++++++++++++------------- src/inspector_js_api.cc | 12 +- test/parallel/test-console-methods.js | 40 +++ 3 files changed, 281 insertions(+), 222 deletions(-) create mode 100644 test/parallel/test-console-methods.js diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js index 6f1df1b6c15724..b33fe42891e853 100644 --- a/lib/internal/console/constructor.js +++ b/lib/internal/console/constructor.js @@ -279,55 +279,235 @@ Console.prototype[kFormatForStderr] = function(args) { return util.formatWithOptions(opts, ...args); }; -Console.prototype.log = function log(...args) { - this[kWriteToConsole](kUseStdout, this[kFormatForStdout](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) { - this[kWriteToConsole](kUseStderr, this[kFormatForStderr](args)); -}; + warn(...args) { + this[kWriteToConsole](kUseStderr, this[kFormatForStderr](args)); + }, -Console.prototype.error = Console.prototype.warn; + dir(object, options) { + this[kWriteToConsole](kUseStdout, util.inspect(object, { + customInspect: false, + ...this[kGetInspectOptions](this._stdout), + ...options + })); + }, -Console.prototype.dir = function dir(object, options) { - options = { - customInspect: false, - ...this[kGetInspectOptions](this._stdout), - ...options - }; - this[kWriteToConsole](kUseStdout, util.inspect(object, options)); -}; + time(label = 'default') { + // Coerces everything other than Symbol to a string + label = `${label}`; + if (this._times.has(label)) { + process.emitWarning(`Label '${label}' already exists for console.time()`); + return; + } + trace(kTraceBegin, kTraceConsoleCategory, `time::${label}`, 0); + this._times.set(label, process.hrtime()); + }, -Console.prototype.time = function time(label = 'default') { - // Coerces everything other than Symbol to a string - label = `${label}`; - if (this._times.has(label)) { - process.emitWarning(`Label '${label}' already exists for console.time()`); - return; - } - trace(kTraceBegin, kTraceConsoleCategory, `time::${label}`, 0); - this._times.set(label, process.hrtime()); -}; + timeEnd(label = 'default') { + // Coerces everything other than Symbol to a string + label = `${label}`; + const hasWarned = timeLogImpl(this, 'timeEnd', label); + trace(kTraceEnd, kTraceConsoleCategory, `time::${label}`, 0); + if (!hasWarned) { + this._times.delete(label); + } + }, -Console.prototype.timeEnd = function timeEnd(label = 'default') { - // Coerces everything other than Symbol to a string - label = `${label}`; - const hasWarned = timeLogImpl(this, 'timeEnd', label); - trace(kTraceEnd, kTraceConsoleCategory, `time::${label}`, 0); - if (!hasWarned) { - this._times.delete(label); - } -}; + 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); + }, + + trace(...args) { + const err = { + name: 'Trace', + message: this[kFormatForStderr](args) + }; + Error.captureStackTrace(err, trace); + this.error(err.stack); + }, + + 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 + clear() { + // It only makes sense to clear if _stdout is a TTY. + // Otherwise, do nothing. + if (this._stdout.isTTY) { + // The require is here intentionally to avoid readline being + // required too early when console is first loaded. + const { cursorTo, clearScreenDown } = require('readline'); + cursorTo(this._stdout, 0, 0); + clearScreenDown(this._stdout); + } + }, + + // Defined by: https://console.spec.whatwg.org/#count + 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}`; + const counts = this[kCounts]; + let count = counts.get(label); + if (count === undefined) + count = 1; + else + count++; + counts.set(label, count); + trace(kTraceCount, kTraceConsoleCategory, `count::${label}`, 0, count); + this.log(`${label}: ${count}`); + }, + + // Defined by: https://console.spec.whatwg.org/#countreset + countReset(label = 'default') { + const counts = this[kCounts]; + if (!counts.has(label)) { + process.emitWarning(`Count for '${label}' does not exist`); + return; + } + trace(kTraceCount, kTraceConsoleCategory, `count::${label}`, 0, 0); + counts.delete(`${label}`); + }, + + group(...data) { + if (data.length > 0) { + this.log(...data); + } + this[kGroupIndent] += ' '; + }, + + groupEnd() { + this[kGroupIndent] = + this[kGroupIndent].slice(0, this[kGroupIndent].length - 2); + }, + + // https://console.spec.whatwg.org/#table + table(tabularData, properties) { + if (properties !== undefined && !ArrayIsArray(properties)) + throw new ERR_INVALID_ARG_TYPE('properties', 'Array', properties); + + if (tabularData === null || typeof tabularData !== 'object') + return this.log(tabularData); + + if (cliTable === undefined) cliTable = require('internal/cli_table'); + const final = (k, v) => this.log(cliTable(k, v)); + + const inspect = (v) => { + const depth = v !== null && + typeof v === 'object' && + !isArray(v) && + ObjectKeys(v).length > 2 ? -1 : 0; + const opt = { + depth, + maxArrayLength: 3, + ...this[kGetInspectOptions](this._stdout) + }; + return util.inspect(v, opt); + }; + const getIndexArray = (length) => ArrayFrom( + { length }, (_, i) => inspect(i)); + + const mapIter = isMapIterator(tabularData); + let isKeyValue = false; + let i = 0; + if (mapIter) { + const res = previewEntries(tabularData, true); + tabularData = res[0]; + isKeyValue = res[1]; + } + + if (isKeyValue || isMap(tabularData)) { + const keys = []; + const values = []; + let length = 0; + if (mapIter) { + for (; i < tabularData.length / 2; ++i) { + keys.push(inspect(tabularData[i * 2])); + values.push(inspect(tabularData[i * 2 + 1])); + length++; + } + } else { + for (const [k, v] of tabularData) { + keys.push(inspect(k)); + values.push(inspect(v)); + length++; + } + } + return final([ + iterKey, keyKey, valuesKey + ], [ + getIndexArray(length), + keys, + values, + ]); + } + + const setIter = isSetIterator(tabularData); + if (setIter) + tabularData = previewEntries(tabularData); + + const setlike = setIter || (mapIter && !isKeyValue) || isSet(tabularData); + if (setlike) { + const values = []; + let length = 0; + for (const v of tabularData) { + values.push(inspect(v)); + length++; + } + return final([setlike ? iterKey : indexKey, valuesKey], [ + getIndexArray(length), + values, + ]); + } + + const map = {}; + let hasPrimitives = false; + const valuesKeyArray = []; + const indexKeyArray = ObjectKeys(tabularData); + + for (; i < indexKeyArray.length; i++) { + const item = tabularData[indexKeyArray[i]]; + const primitive = item === null || + (typeof item !== 'function' && typeof item !== 'object'); + if (properties === undefined && primitive) { + hasPrimitives = true; + valuesKeyArray[i] = inspect(item); + } else { + const keys = properties || ObjectKeys(item); + for (const key of keys) { + if (map[key] === undefined) + map[key] = []; + if ((primitive && properties) || !hasOwnProperty(item, key)) + map[key][i] = ''; + else + map[key][i] = item == null ? item : inspect(item[key]); + } + } + } + + const keys = ObjectKeys(map); + const values = ObjectValues(map); + if (hasPrimitives) { + keys.push(valuesKey); + values.push(valuesKeyArray); + } + keys.unshift(indexKey); + values.unshift(indexKeyArray); -Console.prototype.timeLog = function 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); + return final(keys, values); + }, }; // Returns true if label was not found @@ -347,75 +527,6 @@ function timeLogImpl(self, name, label, data) { return false; } -Console.prototype.trace = function 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) { - 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() { - // It only makes sense to clear if _stdout is a TTY. - // Otherwise, do nothing. - if (this._stdout.isTTY) { - // The require is here intentionally to avoid readline being - // required too early when console is first loaded. - const { cursorTo, clearScreenDown } = require('readline'); - cursorTo(this._stdout, 0, 0); - clearScreenDown(this._stdout); - } -}; - -// Defined by: https://console.spec.whatwg.org/#count -Console.prototype.count = function 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}`; - const counts = this[kCounts]; - let count = counts.get(label); - if (count === undefined) - count = 1; - else - count++; - 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') { - const counts = this[kCounts]; - if (!counts.has(label)) { - process.emitWarning(`Count for '${label}' does not exist`); - return; - } - trace(kTraceCount, kTraceConsoleCategory, `count::${label}`, 0, 0); - counts.delete(`${label}`); -}; - -Console.prototype.group = function group(...data) { - if (data.length > 0) { - this.log(...data); - } - this[kGroupIndent] += ' '; -}; -Console.prototype.groupCollapsed = Console.prototype.group; - -Console.prototype.groupEnd = function groupEnd() { - this[kGroupIndent] = - this[kGroupIndent].slice(0, this[kGroupIndent].length - 2); -}; - const keyKey = 'Key'; const valuesKey = 'Values'; const indexKey = '(index)'; @@ -423,118 +534,16 @@ 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) { - if (properties !== undefined && !ArrayIsArray(properties)) - throw new ERR_INVALID_ARG_TYPE('properties', 'Array', properties); - - if (tabularData === null || typeof tabularData !== 'object') - return this.log(tabularData); - - if (cliTable === undefined) cliTable = require('internal/cli_table'); - const final = (k, v) => this.log(cliTable(k, v)); - - const inspect = (v) => { - const opt = { depth: 0, maxArrayLength: 3 }; - if (v !== null && typeof v === 'object' && - !isArray(v) && ObjectKeys(v).length > 2) - opt.depth = -1; - Object.assign(opt, this[kGetInspectOptions](this._stdout)); - return util.inspect(v, opt); - }; - const getIndexArray = (length) => ArrayFrom({ length }, (_, i) => inspect(i)); - - const mapIter = isMapIterator(tabularData); - let isKeyValue = false; - let i = 0; - if (mapIter) { - const res = previewEntries(tabularData, true); - tabularData = res[0]; - isKeyValue = res[1]; - } - - if (isKeyValue || isMap(tabularData)) { - const keys = []; - const values = []; - let length = 0; - if (mapIter) { - for (; i < tabularData.length / 2; ++i) { - keys.push(inspect(tabularData[i * 2])); - values.push(inspect(tabularData[i * 2 + 1])); - length++; - } - } else { - for (const [k, v] of tabularData) { - keys.push(inspect(k)); - values.push(inspect(v)); - length++; - } - } - return final([ - iterKey, keyKey, valuesKey - ], [ - getIndexArray(length), - keys, - values, - ]); - } - - const setIter = isSetIterator(tabularData); - if (setIter) - tabularData = previewEntries(tabularData); - - const setlike = setIter || (mapIter && !isKeyValue) || isSet(tabularData); - if (setlike) { - const values = []; - let length = 0; - for (const v of tabularData) { - values.push(inspect(v)); - length++; - } - return final([setlike ? iterKey : indexKey, valuesKey], [ - getIndexArray(length), - values, - ]); - } - - const map = {}; - let hasPrimitives = false; - const valuesKeyArray = []; - const indexKeyArray = ObjectKeys(tabularData); - - for (; i < indexKeyArray.length; i++) { - const item = tabularData[indexKeyArray[i]]; - const primitive = item === null || - (typeof item !== 'function' && typeof item !== 'object'); - if (properties === undefined && primitive) { - hasPrimitives = true; - valuesKeyArray[i] = inspect(item); - } else { - const keys = properties || ObjectKeys(item); - for (const key of keys) { - if (map[key] === undefined) - map[key] = []; - if ((primitive && properties) || !hasOwnProperty(item, key)) - map[key][i] = ''; - else - map[key][i] = item == null ? item : inspect(item[key]); - } - } - } - - const keys = ObjectKeys(map); - const values = ObjectValues(map); - if (hasPrimitives) { - keys.push(valuesKey); - values.push(valuesKeyArray); - } - keys.unshift(indexKey); - values.unshift(indexKeyArray); +function noop() {} - return final(keys, values); -}; +for (const method of Reflect.ownKeys(consoleMethods)) + Console.prototype[method] = consoleMethods[method]; -function noop() {} +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, diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index 746cd68ac90c66..16116c9118fa96 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -272,7 +272,17 @@ void Initialize(Local target, Local unused, Environment* env = Environment::GetCurrent(context); Agent* agent = env->inspector_agent(); - env->SetMethod(target, "consoleCall", InspectorConsoleCall); + + v8::Local consoleCallFunc = + env->NewFunctionTemplate(InspectorConsoleCall, v8::Local(), + 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()) diff --git a/test/parallel/test-console-methods.js b/test/parallel/test-console-methods.js new file mode 100644 index 00000000000000..00dc144761cb57 --- /dev/null +++ b/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); +}