From a1778e0dc6b8c3c41d06aa48b3bd72db51873b6d Mon Sep 17 00:00:00 2001 From: Paul Taylor Date: Mon, 16 Jan 2017 09:28:37 -0800 Subject: [PATCH] fix(SafeSubscriber): SafeSubscriber shouldn't mutate incoming Observers. 2237 --- spec/Observable-spec.ts | 12 ++++++++---- src/Subscriber.ts | 10 ++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/spec/Observable-spec.ts b/spec/Observable-spec.ts index 49280149f9..cbb677f73c 100644 --- a/spec/Observable-spec.ts +++ b/spec/Observable-spec.ts @@ -476,8 +476,9 @@ describe('Observable', () => { ' of the anonymous observer', (done: MochaDone) => { //intentionally not using lambda to avoid typescript's this context capture const o = { + myValue: 'foo', next: function next(x) { - expect(this).to.equal(o); + expect(this.myValue).to.equal('foo'); expect(x).to.equal(1); done(); } @@ -490,8 +491,9 @@ describe('Observable', () => { ' of the anonymous observer', (done: MochaDone) => { //intentionally not using lambda to avoid typescript's this context capture const o = { + myValue: 'foo', error: function error(err) { - expect(this).to.equal(o); + expect(this.myValue).to.equal('foo'); expect(err).to.equal('bad'); done(); } @@ -504,8 +506,9 @@ describe('Observable', () => { ' context of the anonymous observer', (done: MochaDone) => { //intentionally not using lambda to avoid typescript's this context capture const o = { + myValue: 'foo', complete: function complete() { - expect(this).to.equal(o); + expect(this.myValue).to.equal('foo'); done(); } }; @@ -527,8 +530,9 @@ describe('Observable', () => { //intentionally not using lambda to avoid typescript's this context capture const o = { + myValue: 'foo', next: function next(x) { - expect(this).to.equal(o); + expect(this.myValue).to.equal('foo'); throw x; } }; diff --git a/src/Subscriber.ts b/src/Subscriber.ts index 88c900fdf9..dc31a46114 100644 --- a/src/Subscriber.ts +++ b/src/Subscriber.ts @@ -167,14 +167,16 @@ class SafeSubscriber extends Subscriber { if (isFunction(observerOrNext)) { next = (<((value: T) => void)> observerOrNext); } else if (observerOrNext) { - context = observerOrNext; next = (> observerOrNext).next; error = (> observerOrNext).error; complete = (> observerOrNext).complete; - if (isFunction(context.unsubscribe)) { - this.add(<() => void> context.unsubscribe.bind(context)); + if (observerOrNext !== emptyObserver) { + context = Object.create(observerOrNext); + if (isFunction(context.unsubscribe)) { + this.add(<() => void> context.unsubscribe.bind(context)); + } + context.unsubscribe = this.unsubscribe.bind(this); } - context.unsubscribe = this.unsubscribe.bind(this); } this._context = context;