From bcd1d28c789d4646407c3a3f8b83d79bc2c78fa1 Mon Sep 17 00:00:00 2001 From: Ravi Jayaramappa Date: Tue, 21 Aug 2018 16:44:20 -0700 Subject: [PATCH 1/2] test: Tests for issue #20 Test to demostrate that membrane containment can be bypassed using getOwnPropertyDescriptor() --- src/reactive-handler.ts | 2 +- test/reactive-handler.spec.ts | 39 ++++++++++++++++++++++++++++++++++ test/read-only-handler.spec.ts | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/reactive-handler.ts b/src/reactive-handler.ts index a0e0160..80f7f30 100644 --- a/src/reactive-handler.ts +++ b/src/reactive-handler.ts @@ -43,7 +43,7 @@ function lockShadowTarget(membrane: ReactiveMembrane, shadowTarget: ReactiveMemb targetKeys.forEach((key: PropertyKey) => { let descriptor = getOwnPropertyDescriptor(originalTarget, key) as PropertyDescriptor; - // We do not need to wrap the descriptor if not configurable + // We do not need to wrap the descriptor if configurable // Because we can deal with wrapping it when user goes through // Get own property descriptor. There is also a chance that this descriptor // could change sometime in the future, so we can defer wrapping diff --git a/test/reactive-handler.spec.ts b/test/reactive-handler.spec.ts index 2a991b4..da16d6d 100644 --- a/test/reactive-handler.spec.ts +++ b/test/reactive-handler.spec.ts @@ -593,4 +593,43 @@ describe('ReactiveHandler', () => { expect(accessSpy).toHaveBeenCalledTimes(2); expect(accessSpy).toHaveBeenLastCalledWith(obj.foo, 'bar'); }); + + describe.skip('issue#20 - getOwnPropertyDescriptor', () => { + it('should return reactive proxy when property accessed via raw getter', () => { + const target = new ReactiveMembrane(); + const todos = {}; + const observable = {}; + Object.defineProperty(todos, 'foo', { + get() { + return observable; + }, + configurable: true + }); + const expected = target.getProxy(observable); + + const proxy = target.getProxy(todos); + expect(proxy.foo).toBe(expected); + + const desc = Object.getOwnPropertyDescriptor(proxy, 'foo'); + const { get } = desc; + expect(get()).toBe(expected); + }); + it('should return reactive proxy when property accessed via descriptor', () => { + const target = new ReactiveMembrane(); + const todos = {}; + const observable = {}; + Object.defineProperty(todos, 'foo', { + value : observable, + configurable: true + }); + const expected = target.getProxy(observable); + + const proxy = target.getProxy(todos); + expect(proxy.foo).toBe(expected); + + const desc = Object.getOwnPropertyDescriptor(proxy, 'foo'); + const { value } = desc; + expect(value).toBe(expected); + }); + }) }); diff --git a/test/read-only-handler.spec.ts b/test/read-only-handler.spec.ts index 8894949..b590bf2 100644 --- a/test/read-only-handler.spec.ts +++ b/test/read-only-handler.spec.ts @@ -271,4 +271,41 @@ describe('ReadOnlyHandler', () => { doNothing(readOnly.foo); }).not.toThrow(); }); + + describe.skip('issue#20 - getOwnPropertyDescriptor', () => { + it('should return reactive proxy when property accessed via raw getter', () => { + const target = new ReactiveMembrane(); + const todos = {}; + Object.defineProperty(todos, 'entry', { + get() { + return { foo: 'bar' }; + }, + configurable: true + }); + + const proxy = target.getReadOnlyProxy(todos); + const desc = Object.getOwnPropertyDescriptor(proxy, 'entry'); + const { get } = desc; + expect(() => { + get().foo = ''; + }).toThrow(); + expect(todos['entry'].foo).toEqual('bar'); + }); + it('should return reactive proxy when property accessed via descriptor', () => { + const target = new ReactiveMembrane(); + const todos = {}; + Object.defineProperty(todos, 'entry', { + value : { foo: 'bar' }, + configurable: true + }); + + const proxy = target.getReadOnlyProxy(todos); + const desc = Object.getOwnPropertyDescriptor(proxy, 'entry'); + const { value } = desc; + expect( () => { + value.foo = ''; + }).toThrow(); + expect(todos['entry'].foo).toEqual('bar'); + }); + }) }); From 67c16cdfb6bb1f1fba6e845c1e1349a4363aaea6 Mon Sep 17 00:00:00 2001 From: Ravi Jayaramappa Date: Fri, 24 Aug 2018 10:00:23 -0700 Subject: [PATCH 2/2] Update test case names to reflect the assertions --- test/reactive-handler.spec.ts | 4 ++-- test/read-only-handler.spec.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/reactive-handler.spec.ts b/test/reactive-handler.spec.ts index da16d6d..1204733 100644 --- a/test/reactive-handler.spec.ts +++ b/test/reactive-handler.spec.ts @@ -595,7 +595,7 @@ describe('ReactiveHandler', () => { }); describe.skip('issue#20 - getOwnPropertyDescriptor', () => { - it('should return reactive proxy when property accessed via raw getter', () => { + it('should return reactive proxy when property value accessed via accessor descriptor', () => { const target = new ReactiveMembrane(); const todos = {}; const observable = {}; @@ -614,7 +614,7 @@ describe('ReactiveHandler', () => { const { get } = desc; expect(get()).toBe(expected); }); - it('should return reactive proxy when property accessed via descriptor', () => { + it('should return reactive proxy when property value accessed via data descriptor', () => { const target = new ReactiveMembrane(); const todos = {}; const observable = {}; diff --git a/test/read-only-handler.spec.ts b/test/read-only-handler.spec.ts index b590bf2..e46325d 100644 --- a/test/read-only-handler.spec.ts +++ b/test/read-only-handler.spec.ts @@ -273,7 +273,7 @@ describe('ReadOnlyHandler', () => { }); describe.skip('issue#20 - getOwnPropertyDescriptor', () => { - it('should return reactive proxy when property accessed via raw getter', () => { + it('readonly proxy prevents mutation when value accessed via accessor descriptor', () => { const target = new ReactiveMembrane(); const todos = {}; Object.defineProperty(todos, 'entry', { @@ -291,7 +291,7 @@ describe('ReadOnlyHandler', () => { }).toThrow(); expect(todos['entry'].foo).toEqual('bar'); }); - it('should return reactive proxy when property accessed via descriptor', () => { + it('readonly proxy prevents mutation when value accessed via data descriptor', () => { const target = new ReactiveMembrane(); const todos = {}; Object.defineProperty(todos, 'entry', {