From 85d20629154a55aadf96ca62d1b2622524acfb0e Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Wed, 9 Dec 2020 13:23:15 -0800 Subject: [PATCH] fix: simplify harden to climb prototype chain (#541) --- packages/make-hardener/src/main.js | 53 +++---------------- .../make-hardener/test/makeHardener.test.js | 20 ++----- 2 files changed, 9 insertions(+), 64 deletions(-) diff --git a/packages/make-hardener/src/main.js b/packages/make-hardener/src/main.js index 8c66805e63..a63207c875 100644 --- a/packages/make-hardener/src/main.js +++ b/packages/make-hardener/src/main.js @@ -26,20 +26,11 @@ const { ownKeys } = Reflect; * Create a `harden` function. */ function makeHardener() { - if (arguments.length >= 1) { - // TODO Just a transitional test. Remove when safe to do so. - throw new TypeError('makeHardener no longer takes any options'); - } - - // Objects that we won't freeze, either because we've frozen them already, - // or they were one of the initial roots (terminals). These objects form - // the "fringe" of the hardened object graph. - const fringeSet = new WeakSet(); + const hardened = new WeakSet(); const { harden } = { harden(root) { const toFreeze = new Set(); - const prototypes = new Map(); const paths = new WeakMap(); // If val is something we should be freezing but aren't yet, @@ -54,7 +45,7 @@ function makeHardener() { // future proof: break until someone figures out what it should do throw new TypeError(`Unexpected typeof: ${type}`); } - if (fringeSet.has(val) || toFreeze.has(val)) { + if (hardened.has(val) || toFreeze.has(val)) { // Ignore if this is an exit, or we've already visited it return; } @@ -77,15 +68,10 @@ function makeHardener() { // get stable/immutable outbound links before a Proxy has a chance to do // something sneaky. - const proto = getPrototypeOf(obj); - const descs = getOwnPropertyDescriptors(obj); const path = paths.get(obj) || 'unknown'; - - // console.log(`adding ${proto} to prototypes under ${path}`); - if (proto !== null && !prototypes.has(proto)) { - prototypes.set(proto, path); - paths.set(proto, `${path}.__proto__`); - } + const descs = getOwnPropertyDescriptors(obj); + const proto = getPrototypeOf(obj); + enqueue(proto, `${path}.__proto__`); ownKeys(descs).forEach(name => { const pathname = `${path}.${String(name)}`; @@ -113,44 +99,17 @@ function makeHardener() { toFreeze.forEach(freezeAndTraverse); // todo curried forEach } - function checkPrototypes() { - prototypes.forEach((path, p) => { - if (!(toFreeze.has(p) || fringeSet.has(p))) { - // all reachable properties have already been frozen by this point - let msg; - try { - msg = `prototype ${p} of ${path} is not already in the fringeSet`; - } catch (e) { - // `${(async _=>_).__proto__}` fails in most engines - msg = - 'a prototype of something is not already in the fringeset (and .toString failed)'; - try { - console.log(msg); - console.log('the prototype:', p); - console.log('of something:', path); - } catch (_e) { - // console.log might be missing in restrictive SES realms - } - } - throw new TypeError(msg); - } - }); - } - function commit() { // todo curried forEach // we capture the real WeakSet.prototype.add above, in case someone // changes it. The two-argument form of forEach passes the second // argument as the 'this' binding, so we add to the correct set. - toFreeze.forEach(fringeSet.add, fringeSet); + toFreeze.forEach(hardened.add, hardened); } enqueue(root); dequeue(); - // console.log("fringeSet", fringeSet); - // console.log("prototype set:", prototypes); // console.log("toFreeze set:", toFreeze); - checkPrototypes(); commit(); return root; diff --git a/packages/make-hardener/test/makeHardener.test.js b/packages/make-hardener/test/makeHardener.test.js index eeadb4ecab..4b54275fa6 100644 --- a/packages/make-hardener/test/makeHardener.test.js +++ b/packages/make-hardener/test/makeHardener.test.js @@ -1,14 +1,8 @@ import test from 'ava'; import makeHardener from '../src/main.js'; -// `harden` is only intended to work after `lockdown`. However, -// to test it standalone, we need to freeze at least these ahead -// of time. -const hardenFirst = [Array.prototype, Object.prototype, Function.prototype]; - test('makeHardener', t => { const h = makeHardener(); - h(hardenFirst); const o = { a: {} }; t.is(h(o), o); t.truthy(Object.isFrozen(o)); @@ -17,7 +11,6 @@ test('makeHardener', t => { test('harden the same thing twice', t => { const h = makeHardener(); - h(hardenFirst); const o = { a: {} }; t.is(h(o), o); t.is(h(o), o); @@ -27,7 +20,6 @@ test('harden the same thing twice', t => { test('harden objects with cycles', t => { const h = makeHardener(); - h(hardenFirst); const o = { a: {} }; o.a.foo = o; t.is(h(o), o); @@ -37,7 +29,6 @@ test('harden objects with cycles', t => { test('harden overlapping objects', t => { const h = makeHardener(); - h(hardenFirst); const o1 = { a: {} }; const o2 = { a: o1.a }; t.is(h(o1), o1); @@ -48,23 +39,18 @@ test('harden overlapping objects', t => { t.truthy(Object.isFrozen(o2)); }); -test('do not commit early', t => { - // refs #4 +test('harden up prototype chain', t => { const h = makeHardener(); - h(hardenFirst); const a = { a: 1 }; const b = { b: 1, __proto__: a }; const c = { c: 1, __proto__: b }; - t.throws(() => h(b), { instanceOf: TypeError }); - // the bug is that 'b' is marked as hardened. If that happens, harden(c) - // will pass when it was supposed to throw. - t.throws(() => h(c), { instanceOf: TypeError }); + h(c); + t.truthy(Object.isFrozen(a)); }); test('harden() tolerates objects with null prototypes', t => { const h = makeHardener(); - h(hardenFirst); const o = { a: 1 }; Object.setPrototypeOf(o, null); t.is(h(o), o);