Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix: special EventEmitter keys leak information about other rules (#9328
) `Linter` uses Node's `EventEmitter` API to register listeners for rules. However, the `EventEmitter` API has a few problems for this use case: * `EventEmitter` has three "special" events (`newListener`, `removeListener`, and `error`) which are called when something happens with another listener. This is undesirable because `Linter` allows rules to register listeners for arbitrary string events, and we don't want rule listeners to be able to detect each other. * `EventEmitter` calls listeners with a `this` value of the event emitter itself. This is undesirable because this would allow rules to modify or tamper with listeners registered by other rules. This commit fixes the problem by updating `Linter` to use a custom event-emitting object with a similar API, rather than `EventEmitter` itself.
- Loading branch information
1 parent
d593e61
commit 1c6bc67
Showing
7 changed files
with
135 additions
and
33 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /** | ||
| * @fileoverview A variant of EventEmitter which does not give listeners information about each other | ||
| * @author Teddy Katz | ||
| */ | ||
|
|
||
| "use strict"; | ||
|
|
||
| //------------------------------------------------------------------------------ | ||
| // Typedefs | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| /** | ||
| * An object describing an AST selector | ||
| * @typedef {Object} SafeEmitter | ||
| * @property {function(eventName: string, listenerFunc: Function): void} on Adds a listener for a given event name | ||
| * @property {function(eventName: string, arg1?: any, arg2?: any, arg3?: any)} emit Emits an event with a given name. | ||
| * This calls all the listeners that were listening for that name, with `arg1`, `arg2`, and `arg3` as arguments. | ||
| * @property {function(): string[]} eventNames Gets the list of event names that have registered listeners. | ||
| */ | ||
|
|
||
| /** | ||
| * Creates an object which can listen for and emit events. | ||
| * This is similar to the EventEmitter API in Node's standard library, but it has a few differences. | ||
| * The goal is to allow multiple modules to attach arbitrary listeners to the same emitter, without | ||
| * letting the modules know about each other at all. | ||
| * 1. It has no special keys like `error` and `newListener`, which would allow modules to detect when | ||
| * another module throws an error or registers a listener. | ||
| * 2. It calls listener functions without any `this` value. (`EventEmitter` calls listeners with a | ||
| * `this` value of the emitter instance, which would give listeners access to other listeners.) | ||
| * 3. Events can be emitted with at most 3 arguments. (For example: when using `emitter.emit('foo', a, b, c)`, | ||
| * the arguments `a`, `b`, and `c` will be passed to the listener functions.) | ||
| * @returns {SafeEmitter} An emitter | ||
| */ | ||
| module.exports = () => { | ||
| const listeners = Object.create(null); | ||
|
|
||
| return Object.freeze({ | ||
| on(eventName, listener) { | ||
| if (eventName in listeners) { | ||
| listeners[eventName].push(listener); | ||
| } else { | ||
| listeners[eventName] = [listener]; | ||
| } | ||
| }, | ||
| emit(eventName, a, b, c) { | ||
| if (eventName in listeners) { | ||
| listeners[eventName].forEach(listener => listener(a, b, c)); | ||
| } | ||
| }, | ||
| eventNames() { | ||
| return Object.keys(listeners); | ||
| } | ||
| }); | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /** | ||
| * @fileoverview Tests for safe-emitter | ||
| * @author Teddy Katz | ||
| */ | ||
|
|
||
| "use strict"; | ||
|
|
||
| const createEmitter = require("../../../lib/util/safe-emitter"); | ||
| const assert = require("chai").assert; | ||
|
|
||
| describe("safe-emitter", () => { | ||
| describe("emit() and on()", () => { | ||
| it("allows listeners to be registered calls them when emitted", () => { | ||
| const emitter = createEmitter(); | ||
| const colors = []; | ||
|
|
||
| emitter.on("foo", () => colors.push("red")); | ||
| emitter.on("foo", () => colors.push("blue")); | ||
| emitter.on("bar", () => colors.push("green")); | ||
|
|
||
| emitter.emit("foo"); | ||
| assert.deepEqual(colors, ["red", "blue"]); | ||
|
|
||
| emitter.on("bar", color => colors.push(color)); | ||
| emitter.emit("bar", "yellow"); | ||
|
|
||
| assert.deepEqual(colors, ["red", "blue", "green", "yellow"]); | ||
| }); | ||
|
|
||
| it("calls listeners with no `this` value", () => { | ||
| const emitter = createEmitter(); | ||
| let called = false; | ||
|
|
||
| emitter.on("foo", function() { | ||
| assert.strictEqual(this, void 0); // eslint-disable-line no-invalid-this | ||
| called = true; | ||
| }); | ||
|
|
||
| emitter.emit("foo"); | ||
| assert(called); | ||
| }); | ||
| }); | ||
| }); |