From e38bade82801645d8cc1010e3f4e9826052244cb Mon Sep 17 00:00:00 2001 From: Brian White Date: Wed, 6 Apr 2016 20:33:28 -0400 Subject: [PATCH] events: don't inherit from Object.prototype This commit safely allows event names that are named the same as properties that are ordinarily inherited from Object.prototype such as __proto__. Fixes: https://github.com/nodejs/node/issues/728 PR-URL: https://github.com/nodejs/node/pull/6092 Reviewed-By: Evan Lucas Reviewed-By: James M Snell --- lib/events.js | 20 +++++++---- src/node.cc | 4 ++- .../test-events-known-properties.js | 12 ------- ...st-event-emitter-listeners-side-effects.js | 1 - .../test-event-emitter-special-event-names.js | 36 +++++++++++++++++++ 5 files changed, 52 insertions(+), 21 deletions(-) delete mode 100644 test/known_issues/test-events-known-properties.js create mode 100644 test/parallel/test-event-emitter-special-event-names.js diff --git a/lib/events.js b/lib/events.js index a67de552f1..669c0d70ea 100644 --- a/lib/events.js +++ b/lib/events.js @@ -2,6 +2,12 @@ var domain; +// This constructor is used to store event handlers. Instantiating this is +// faster than explicitly calling `Object.create(null)` to get a "clean" empty +// object (tested with v8 v4.9). +function EventHandlers() {} +EventHandlers.prototype = Object.create(null); + function EventEmitter() { EventEmitter.init.call(this); } @@ -44,7 +50,7 @@ EventEmitter.init = function() { } if (!this._events || this._events === Object.getPrototypeOf(this)._events) { - this._events = {}; + this._events = new EventHandlers(); this._eventsCount = 0; } @@ -211,7 +217,7 @@ EventEmitter.prototype.addListener = function addListener(type, listener) { events = this._events; if (!events) { - events = this._events = {}; + events = this._events = new EventHandlers(); this._eventsCount = 0; } else { // To avoid recursion in the case that type === "newListener"! Before @@ -296,7 +302,7 @@ EventEmitter.prototype.removeListener = if (list === listener || (list.listener && list.listener === listener)) { if (--this._eventsCount === 0) - this._events = {}; + this._events = new EventHandlers(); else { delete events[type]; if (events.removeListener) @@ -319,7 +325,7 @@ EventEmitter.prototype.removeListener = if (list.length === 1) { list[0] = undefined; if (--this._eventsCount === 0) { - this._events = {}; + this._events = new EventHandlers(); return this; } else { delete events[type]; @@ -346,11 +352,11 @@ EventEmitter.prototype.removeAllListeners = // not listening for removeListener, no need to emit if (!events.removeListener) { if (arguments.length === 0) { - this._events = {}; + this._events = new EventHandlers(); this._eventsCount = 0; } else if (events[type]) { if (--this._eventsCount === 0) - this._events = {}; + this._events = new EventHandlers(); else delete events[type]; } @@ -366,7 +372,7 @@ EventEmitter.prototype.removeAllListeners = this.removeAllListeners(key); } this.removeAllListeners('removeListener'); - this._events = {}; + this._events = new EventHandlers(); this._eventsCount = 0; return this; } diff --git a/src/node.cc b/src/node.cc index 081905cfc0..dd427c792e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3228,7 +3228,9 @@ void SetupProcessObject(Environment* env, env->SetMethod(process, "_setupDomainUse", SetupDomainUse); // pre-set _events object for faster emit checks - process->Set(env->events_string(), Object::New(env->isolate())); + Local events_obj = Object::New(env->isolate()); + events_obj->SetPrototype(env->context(), Null(env->isolate())); + process->Set(env->events_string(), events_obj); } diff --git a/test/known_issues/test-events-known-properties.js b/test/known_issues/test-events-known-properties.js deleted file mode 100644 index 944fa3da2b..0000000000 --- a/test/known_issues/test-events-known-properties.js +++ /dev/null @@ -1,12 +0,0 @@ -'use strict'; -// Refs: https://github.com/nodejs/node/issues/728 -const common = require('../common'); -const assert = require('assert'); -const EventEmitter = require('events'); -const ee = new EventEmitter(); - -ee.on('__proto__', common.mustCall((data) => { - assert.strictEqual(data, 42); -})); - -ee.emit('__proto__', 42); diff --git a/test/parallel/test-event-emitter-listeners-side-effects.js b/test/parallel/test-event-emitter-listeners-side-effects.js index 355ec5262c..a30d1d3d81 100644 --- a/test/parallel/test-event-emitter-listeners-side-effects.js +++ b/test/parallel/test-event-emitter-listeners-side-effects.js @@ -21,7 +21,6 @@ assert(fl.length === 1); assert(fl[0] === assert.fail); e.listeners('bar'); -assert(!e._events.hasOwnProperty('bar')); e.on('foo', assert.ok); fl = e.listeners('foo'); diff --git a/test/parallel/test-event-emitter-special-event-names.js b/test/parallel/test-event-emitter-special-event-names.js new file mode 100644 index 0000000000..fb3f1b52d9 --- /dev/null +++ b/test/parallel/test-event-emitter-special-event-names.js @@ -0,0 +1,36 @@ +'use strict'; + +const common = require('../common'); +const EventEmitter = require('events'); +const assert = require('assert'); + +const ee = new EventEmitter(); +const handler = () => {}; + +assert.strictEqual(ee._events.hasOwnProperty, undefined); +assert.strictEqual(ee._events.toString, undefined); + +ee.on('__proto__', handler); +ee.on('__defineGetter__', handler); +ee.on('toString', handler); + +assert.deepStrictEqual(ee.eventNames(), [ + '__proto__', + '__defineGetter__', + 'toString' +]); + +assert.deepStrictEqual(ee.listeners('__proto__'), [handler]); +assert.deepStrictEqual(ee.listeners('__defineGetter__'), [handler]); +assert.deepStrictEqual(ee.listeners('toString'), [handler]); + +ee.on('__proto__', common.mustCall(function(val) { + assert.strictEqual(val, 1); +})); +ee.emit('__proto__', 1); + +process.on('__proto__', common.mustCall(function(val) { + assert.strictEqual(val, 1); +})); +process.emit('__proto__', 1); +