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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: mscdex

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;
       }

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<Object> events_obj = Object::New(env->isolate());
+  events_obj->SetPrototype(env->context(), Null(env->isolate()));
+  process->Set(env->events_string(), events_obj);
 }
 
 

test/known_issues/test-events-known-properties.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');

test/parallel/test-event-emitter-listeners-side-effects.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);
+