bootstrap: --frozen-intrinsics override problem workaround

PR-URL: nodejs#28254 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Trott · Jun 22, 2019 · 554ffa3 · 554ffa3
1 parent 3fd5451
commit 554ffa3
Show file tree

Hide file tree

Showing 3 changed files with 225 additions and 73 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -209,44 +209,7 @@ Enable experimental frozen intrinsics like `Array` and `Object`.
 
 Support is currently only provided for the root context and no guarantees are
 currently provided that `global.Array` is indeed the default intrinsic
-reference.
-
-**Code breakage is highly likely with this flag**, since redefining any
-builtin properties on a subclass will throw in strict mode due to the ECMA-262
-issue https://github.com/tc39/ecma262/pull/1307. This flag may still change
-or be removed in the future.
-
-To avoid these cases, any builtin function overrides should be defined upfront:
-
-```js
-const o = {};
-// THROWS: Cannot assign read only property 'toString' of object
-o.toString = () => 'string';
-
-class X {
-  constructor() {
-    this.toString = () => 'string';
-  }
-}
-// THROWS: Cannot assign read only property 'toString' of object
-new X();
-```
-
-```js
-// OK
-const o = { toString: () => 'string' };
-
-class X {
-  toString = undefined;
-  constructor() {
-    this.toString = () => 'string';
-  }
-}
-// OK
-new X();
-```
-
-
+reference. Code may break under this flag.
 
 ### `--heapsnapshot-signal=signal`
 <!-- YAML

diff --git a/lib/internal/freeze_intrinsics.js b/lib/internal/freeze_intrinsics.js
@@ -12,18 +12,29 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-// SPDX-License-Identifier: MIT
+// SPDX-License-Identifier: Apache-2.0
 
 // Based upon:
 // https://github.com/google/caja/blob/master/src/com/google/caja/ses/startSES.js
 // https://github.com/google/caja/blob/master/src/com/google/caja/ses/repairES5.js
-// https://github.com/tc39/proposal-frozen-realms/blob/91ac390e3451da92b5c27e354b39e52b7636a437/shim/src/deep-freeze.js
+// https://github.com/tc39/proposal-ses/blob/e5271cc42a257a05dcae2fd94713ed2f46c08620/shim/src/freeze.js
 
-/* global WebAssembly, SharedArrayBuffer */
+/* global WebAssembly, SharedArrayBuffer, console */
 /* eslint-disable no-restricted-globals */
 'use strict';
 
 module.exports = function() {
+  const {
+    defineProperty,
+    freeze,
+    getOwnPropertyDescriptor,
+    getOwnPropertyDescriptors,
+    getOwnPropertyNames,
+    getOwnPropertySymbols,
+    getPrototypeOf
+  } = Object;
+  const objectHasOwnProperty = Object.prototype.hasOwnProperty;
+  const { ownKeys } = Reflect;
   const {
     clearImmediate,
     clearInterval,
@@ -33,30 +44,112 @@ module.exports = function() {
     setTimeout
   } = require('timers');
 
+  const intrinsicPrototypes = [
+    // Anonymous Intrinsics
+    // IteratorPrototype
+    getPrototypeOf(
+      getPrototypeOf(new Array()[Symbol.iterator]())
+    ),
+    // ArrayIteratorPrototype
+    getPrototypeOf(new Array()[Symbol.iterator]()),
+    // StringIteratorPrototype
+    getPrototypeOf(new String()[Symbol.iterator]()),
+    // MapIteratorPrototype
+    getPrototypeOf(new Map()[Symbol.iterator]()),
+    // SetIteratorPrototype
+    getPrototypeOf(new Set()[Symbol.iterator]()),
+    // GeneratorFunction
+    getPrototypeOf(function* () {}),
+    // AsyncFunction
+    getPrototypeOf(async function() {}),
+    // AsyncGeneratorFunction
+    getPrototypeOf(async function* () {}),
+    // TypedArray
+    getPrototypeOf(Uint8Array),
+
+    // 19 Fundamental Objects
+    Object.prototype, // 19.1
+    Function.prototype, // 19.2
+    Boolean.prototype, // 19.3
+
+    Error.prototype, // 19.5
+    EvalError.prototype,
+    RangeError.prototype,
+    ReferenceError.prototype,
+    SyntaxError.prototype,
+    TypeError.prototype,
+    URIError.prototype,
+
+    // 20 Numbers and Dates
+    Number.prototype, // 20.1
+    Date.prototype, // 20.3
+
+    // 21 Text Processing
+    String.prototype, // 21.1
+    RegExp.prototype, // 21.2
+
+    // 22 Indexed Collections
+    Array.prototype, // 22.1
+
+    Int8Array.prototype,
+    Uint8Array.prototype,
+    Uint8ClampedArray.prototype,
+    Int16Array.prototype,
+    Uint16Array.prototype,
+    Int32Array.prototype,
+    Uint32Array.prototype,
+    Float32Array.prototype,
+    Float64Array.prototype,
+    BigInt64Array.prototype,
+    BigUint64Array.prototype,
+
+    // 23 Keyed Collections
+    Map.prototype, // 23.1
+    Set.prototype, // 23.2
+    WeakMap.prototype, // 23.3
+    WeakSet.prototype, // 23.4
+
+    // 24 Structured Data
+    ArrayBuffer.prototype, // 24.1
+    DataView.prototype, // 24.3
+    Promise.prototype, // 25.4
+
+    // Other APIs / Web Compatibility
+    console.Console.prototype,
+    BigInt.prototype,
+    WebAssembly.Module.prototype,
+    WebAssembly.Instance.prototype,
+    WebAssembly.Table.prototype,
+    WebAssembly.Memory.prototype,
+    WebAssembly.CompileError.prototype,
+    WebAssembly.LinkError.prototype,
+    WebAssembly.RuntimeError.prototype,
+    SharedArrayBuffer.prototype
+  ];
   const intrinsics = [
     // Anonymous Intrinsics
     // ThrowTypeError
-    Object.getOwnPropertyDescriptor(Function.prototype, 'caller').get,
+    getOwnPropertyDescriptor(Function.prototype, 'caller').get,
     // IteratorPrototype
-    Object.getPrototypeOf(
-      Object.getPrototypeOf(new Array()[Symbol.iterator]())
+    getPrototypeOf(
+      getPrototypeOf(new Array()[Symbol.iterator]())
     ),
     // ArrayIteratorPrototype
-    Object.getPrototypeOf(new Array()[Symbol.iterator]()),
+    getPrototypeOf(new Array()[Symbol.iterator]()),
     // StringIteratorPrototype
-    Object.getPrototypeOf(new String()[Symbol.iterator]()),
+    getPrototypeOf(new String()[Symbol.iterator]()),
     // MapIteratorPrototype
-    Object.getPrototypeOf(new Map()[Symbol.iterator]()),
+    getPrototypeOf(new Map()[Symbol.iterator]()),
     // SetIteratorPrototype
-    Object.getPrototypeOf(new Set()[Symbol.iterator]()),
+    getPrototypeOf(new Set()[Symbol.iterator]()),
     // GeneratorFunction
-    Object.getPrototypeOf(function* () {}),
+    getPrototypeOf(function* () {}),
     // AsyncFunction
-    Object.getPrototypeOf(async function() {}),
+    getPrototypeOf(async function() {}),
     // AsyncGeneratorFunction
-    Object.getPrototypeOf(async function* () {}),
+    getPrototypeOf(async function* () {}),
     // TypedArray
-    Object.getPrototypeOf(Uint8Array),
+    getPrototypeOf(Uint8Array),
 
     // 18 The Global Object
     eval,
@@ -75,14 +168,13 @@ module.exports = function() {
     Boolean, // 19.3
     Symbol, // 19.4
 
-    // Disabled pending stack trace mutation handling
-    // Error, // 19.5
-    // EvalError,
-    // RangeError,
-    // ReferenceError,
-    // SyntaxError,
-    // TypeError,
-    // URIError,
+    Error, // 19.5
+    EvalError,
+    RangeError,
+    ReferenceError,
+    SyntaxError,
+    TypeError,
+    URIError,
 
     // 20 Numbers and Dates
     Number, // 20.1
@@ -128,36 +220,37 @@ module.exports = function() {
     escape,
     unescape,
 
-    // Web compatibility
+    // Other APIs / Web Compatibility
     clearImmediate,
     clearInterval,
     clearTimeout,
     setImmediate,
     setInterval,
     setTimeout,
-
-    // Other APIs
+    console,
     BigInt,
     Atomics,
     WebAssembly,
     SharedArrayBuffer
   ];
 
-  if (typeof Intl !== 'undefined')
+  if (typeof Intl !== 'undefined') {
+    intrinsicPrototypes.push(Intl.Collator.prototype);
+    intrinsicPrototypes.push(Intl.DateTimeFormat.prototype);
+    intrinsicPrototypes.push(Intl.ListFormat.prototype);
+    intrinsicPrototypes.push(Intl.NumberFormat.prototype);
+    intrinsicPrototypes.push(Intl.PluralRules.prototype);
+    intrinsicPrototypes.push(Intl.RelativeTimeFormat.prototype);
     intrinsics.push(Intl);
+  }
+
+  intrinsicPrototypes.forEach(enableDerivedOverrides);
 
+  const frozenSet = new WeakSet();
   intrinsics.forEach(deepFreeze);
 
+  // Objects that are deeply frozen.
   function deepFreeze(root) {
-
-    const { freeze, getOwnPropertyDescriptors, getPrototypeOf } = Object;
-    const { ownKeys } = Reflect;
-
-    // Objects that are deeply frozen.
-    // It turns out that Error is reachable from WebAssembly so it is
-    // explicitly added here to ensure it is not frozen
-    const frozenSet = new WeakSet([Error, Error.prototype]);
-
     /**
      * "innerDeepFreeze()" acts like "Object.freeze()", except that:
      *
@@ -246,4 +339,79 @@ module.exports = function() {
     innerDeepFreeze(root);
     return root;
   }
+
+  /**
+   * For a special set of properties (defined below), it ensures that the
+   * effect of freezing does not suppress the ability to override these
+   * properties on derived objects by simple assignment.
+   *
+   * Because of lack of sufficient foresight at the time, ES5 unfortunately
+   * specified that a simple assignment to a non-existent property must fail if
+   * it would override a non-writable data property of the same name. (In
+   * retrospect, this was a mistake, but it is now too late and we must live
+   * with the consequences.) As a result, simply freezing an object to make it
+   * tamper proof has the unfortunate side effect of breaking previously correct
+   * code that is considered to have followed JS best practices, if this
+   * previous code used assignment to override.
+   *
+   * To work around this mistake, deepFreeze(), prior to freezing, replaces
+   * selected configurable own data properties with accessor properties which
+   * simulate what we should have specified -- that assignments to derived
+   * objects succeed if otherwise possible.
+   */
+  function enableDerivedOverride(obj, prop, desc) {
+    if ('value' in desc && desc.configurable) {
+      const value = desc.value;
+
+      function getter() {
+        return value;
+      }
+
+      // Re-attach the data property on the object so
+      // it can be found by the deep-freeze traversal process.
+      getter.value = value;
+
+      function setter(newValue) {
+        if (obj === this) {
+          // eslint-disable-next-line no-restricted-syntax
+          throw new TypeError(
+            `Cannot assign to read only property '${prop}' of object '${obj}'`
+          );
+        }
+        if (objectHasOwnProperty.call(this, prop)) {
+          this[prop] = newValue;
+        } else {
+          defineProperty(this, prop, {
+            value: newValue,
+            writable: true,
+            enumerable: desc.enumerable,
+            configurable: desc.configurable
+          });
+        }
+      }
+
+      defineProperty(obj, prop, {
+        get: getter,
+        set: setter,
+        enumerable: desc.enumerable,
+        configurable: desc.configurable
+      });
+    }
+  }
+
+  function enableDerivedOverrides(obj) {
+    if (!obj) {
+      return;
+    }
+    const descs = getOwnPropertyDescriptors(obj);
+    if (!descs) {
+      return;
+    }
+    getOwnPropertyNames(obj).forEach((prop) => {
+      return enableDerivedOverride(obj, prop, descs[prop]);
+    });
+    getOwnPropertySymbols(obj).forEach((prop) => {
+      return enableDerivedOverride(obj, prop, descs[prop]);
+    });
+  }
 };
diff --git a/test/parallel/test-freeze-intrinsics.js b/test/parallel/test-freeze-intrinsics.js
@@ -7,3 +7,24 @@ assert.throws(
   () => Object.defineProperty = 'asdf',
   TypeError
 );
+
+// Ensure we can extend Console
+{
+  class ExtendedConsole extends console.Console {}
+
+  const s = new ExtendedConsole(process.stdout);
+  const logs = [];
+  s.log = (msg) => logs.push(msg);
+  s.log('custom');
+  s.log = undefined;
+  assert.strictEqual(s.log, undefined);
+  assert.strictEqual(logs.length, 1);
+  assert.strictEqual(logs[0], 'custom');
+}
+
+// Ensure we can write override Object prototype properties on instances
+{
+  const o = {};
+  o.toString = () => 'Custom toString';
+  assert.strictEqual(o + 'asdf', 'Custom toStringasdf');
+}