fix: use string instead of symbol for getter property
MODIFY
packages/ses/src/enable-property-overrides.js
packages/ses/src/enable-property-overrides.js
// Adapted from SES/Caja
// Copyright (C) 2011 Google Inc.
+// @ts-check
+
import {
defineProperty,
getOwnPropertyNames,
getOwnPropertyDescriptor,
getOwnPropertyDescriptors,
+ objectHasOwnProperty,
} from './commons.js';
-// 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
import enablements from './enablements.js';
const { ownKeys } = Reflect;
@@ -19,35 +20,56 @@ function isObject(obj) {
}
/**
- * This symbol names a property that we're adding to getters that
- * * identifies that the getter alleges that it was created to suppress the
- * override mistake.
- * * provides the original data property value as its value.
- *
- * This is intended for expert use and avoidance of accidental collision with
- * normal code. But we are not trying to hide the symbol beyond that. Thus, we
- * make it a registered symbol that can be reconstructed anywhere using the code
- * `Symbol.for('originalValue')`. We no longer export it, so that's the most
- * convenient way to obtain the symbol.
- */
-const originalValueSymbol = Symbol.for('originalValue');
-
-/**
- * For a special set of properties (defined in the enablement plan), it ensures
- * that the effect of freezing does not suppress the ability to override
- * these properties on derived objects by simple assignment.
+ * For a special set of properties defined in the `enablement` whitelist,
+ * `enablePropertyOverrides` 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.
+ * it would override a non-writable data property of the same name. In
+ * retrospect, this was a mistake, the so-called "override 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.
+ *
+ * For the enabled properties, `enablePropertyOverrides` effectively shims what
+ * the assignment behavior would have been in the absence of the override
+ * mistake. However, the shim produces an imperfect emulation. It shims the
+ * behavior by turning these data properties into accessor properties, where
+ * the accessor's getter and setter provide the desired behavior. For
+ * non-reflective operations, the illusion is perfect. However, reflective
+ * operations like `getOwnPropertyDescriptor` see the descriptor of an accessor
+ * property rather than the descriptor of a data property. At the time of this
+ * writing, this is the best we know how to do.
+ *
+ * To the getter of the accessor we add a property named
+ * `'originalDataPropertyValue'` whose value is, as it says, the value that the
+ * data property had before being converted to an accessor property. We add
+ * this extra property to the getter for two reason:
+ *
+ * The harden algorithm walks the own properties reflectively, i.e., with
+ * `getOwnPropertyDescriptor` semantics, rather than `[[Get]]` semantics. When
+ * it sees an accessor property, it does not invoke the getter. Rather, it
+ * proceeds to walk both the getter and setter as part of its transitive
+ * traversal. Without this extra property, `enablePropertyOverrides` would have
+ * hidden the original data property value from `harden`, which would be bad.
+ * Instead, by exposing that value in an own data property on the getter,
+ * `harden` finds and walks it anyway.
+ *
+ * We enable a form of cooperative emulation, giving reflective code an
+ * opportunity to cooperate in upholding the illusion. When such cooperative
+ * reflective code sees an accessor property, where the accessor's getter
+ * has an `originalDataPropertyValue` property, it knows that the getter is
+ * alleging that it is the result of the `enablePropertyOverrides` conversion
+ * pattern, so it can decide to cooperatively "pretend" that it sees a data
+ * property with that value.
+ *
+ * @param {Record<string, any>} intrinsics
*/
-
-// TODO exmplain parameters
export default function enablePropertyOverrides(intrinsics) {
function enable(path, obj, prop, desc) {
if ('value' in desc && desc.configurable) {
@@ -56,13 +78,7 @@ export default function enablePropertyOverrides(intrinsics) {
function getter() {
return value;
}
- // The presence of this symbol on an accessor's getter alleges that
- // this accessor is a converted data property whose original value
- // is the value of that property. By exposing it, the harden algorithm
- // on encountering the getter will harden the getter function and also
- // this value. Thereby preserving the same transitivity that harden would
- // have had on the original data-property graph.
- getter[originalValueSymbol] = value;
+ getter.originalDataPropertyValue = value;
function setter(newValue) {
if (obj === this) {
@@ -70,7 +86,7 @@ export default function enablePropertyOverrides(intrinsics) {
`Cannot assign to read only property '${prop}' of '${path}'`,
);
}
- if (hasOwnProperty.call(this, prop)) {
+ if (objectHasOwnProperty(this, prop)) {
this[prop] = newValue;
} else {
defineProperty(this, prop, {

MODIFY
packages/ses/test/frozen-primordials.test.js
packages/ses/test/frozen-primordials.test.js
@@ -2,20 +2,15 @@ import test from 'tape';
import '../lockdown.js';
import { getOwnPropertyDescriptor } from '../src/commons.js';
-const originalValueSymbol = Symbol.for('originalValue');
-
test('check if override-protected primordials are frozen', t => {
lockdown();
- // After removing the detachedProperties mechanism and before adding
- // the originalValueSymbol mechanism, this test failed.
+ // After removing the detachedProperties mechanism and without
+ // the originalDataPropertyValue mechanism, this test failed.
t.ok(Object.isFrozen(Object.prototype.toString));
- // Just checking that reconstruction produces the *same* symbol
- t.equals(originalValueSymbol, Symbol.for('originalValue'));
-
const desc = getOwnPropertyDescriptor(Object.prototype, 'toString');
- t.equals(desc.get[originalValueSymbol], Object.prototype.toString);
+ t.equals(desc.get.originalDataPropertyValue, Object.prototype.toString);
t.end();
});

fix: use string instead of symbol for getter property