Skip to content
Permalink
Browse files

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: #728
PR-URL: #6092
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information...
mscdex committed Apr 7, 2016
1 parent dba245f commit e38bade82801645d8cc1010e3f4e9826052244cb
@@ -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;
}
@@ -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<Object> events_obj = Object::New(env->isolate());
events_obj->SetPrototype(env->context(), Null(env->isolate()));

This comment has been minimized.

Copy link
@jasnell

jasnell Apr 19, 2016

Member

@mscdex ... fyi... this is causing a compiler warning when building

../src/node.cc:3232:3: warning: ignoring return value of function declared with
      warn_unused_result attribute [-Wunused-result]
  events_obj->SetPrototype(env->context(), Null(env->isolate()));
  ^~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
process->Set(env->events_string(), events_obj);
}


This file was deleted.

@@ -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');
@@ -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);

0 comments on commit e38bade

Please sign in to comment.
You can’t perform that action at this time.