Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propose to tc39 without override mistake #105

Open
erights opened this issue Sep 29, 2019 · 5 comments
Open

Propose to tc39 without override mistake #105

erights opened this issue Sep 29, 2019 · 5 comments
Assignees
Labels
kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 tc39

Comments

@erights
Copy link
Contributor

erights commented Sep 29, 2019

We should propose harden to tc39, together with whatever other primitive hardening/freezing/snapshotting primitives we need to cover our needs, including those of tc53 and Moddable.

Should we propose that properties made non-writable,non-configurable by harden also act as if the override mistake were absent? As we've seen, shimming this everywhere with accessors is expensive. But done via a standard mechanism, the platform could fix this cheaply.

Note: There was already an attempt to just fix the override mistake in the language itself. That failed under testing --- a little old code was incompatible with fixing this, but enough code that no one was willing to ratify it. The best we can get is probably that new hardening/freezing/snapshotting mechanisms fix this. Since these mechanisms would be new, the fix would effectively be opt-in and not break old code.

@erights
Copy link
Contributor Author

erights commented Sep 29, 2019

Attn @phoddie @bmeck @jfparadis @dckc

@jfparadis
Copy link
Contributor

Decoupling harden and fixing the override mistake make sense to me. Even in code, those steps are completely independent and are executed in sequence. The harden shim just need to become conditional to avoid overriding a native implementation.

@jfparadis jfparadis transferred this issue from Agoric/harden Feb 21, 2020
@kriskowal kriskowal added the tc39 label Aug 13, 2020
@mhofman mhofman self-assigned this Apr 15, 2022
@mhofman
Copy link
Contributor

mhofman commented Apr 15, 2022

One idea might be to add an option to freeze (and maybe seal) to request the integrity level be cached as a slot (e.g [[CachedIntegrity]] on the object. Then we could get 2 benefits:

  • isFrozen / TestIntegrityLevel would return immediately if the slot is set accordingly, without calling the [[IsExtensible]], [[OwnPropertyKeys]] and [[GetOwnProperty]] of the target object. This is only relevant in the case of exotics like proxies as an implementation can already cache this for regular objects.
  • OrdinarySetWithOwnDescriptor could be updated so that if the ancestor on which the existing property is found is "cached frozen", then it would revert to define semantics on the receiver. Probably change step 2.1 to something like If ownDesc.[[Writable]] is false and O.[[CachedIntegrity]] is not "frozen", return false., but I have to figure out if that'd be sufficient. (Btw, I'm surprised there isn't a same value check before returning false)
Object.freeze(Object.prototype, { cacheIntegrity: true });
const obj = {};
obj.toString = () => '[Overridden Object]'; // succeeds
const proxy = new Proxy({ foo: 42 }, {
  ownKeys(...args) {
    console.log('ownKeys triggered');
    return Reflect.ownKeys(...args);
  },
  isExtensible(...args) {
    console.log('isExtensible triggered');
    return Reflect.isExtensible(...args);
  },
  preventExtensions(...args) {
    console.log('preventExtensions triggered');
    return Reflect.preventExtensions(...args);
  },
});
Object.seal(proxy, { cacheIntegrity: true }); // prints 'preventExtensions triggered'
assert(Object.isSealed(proxy)); // prints nothing
assert(!Object.isFrozen(proxy)); // prints 'ownKeys triggered'
Object.freeze(proxy, { cacheIntegrity: true }); // prints 'ownKeys triggered'
assert(Object.isFrozen(proxy)); // prints nothing

@mhofman
Copy link
Contributor

mhofman commented Apr 17, 2022

If we propose a Object.create option to deal with the return override + private fields issue, then options for sealing/freezing and caching the integrity level at creation might also make sense:

The create options bag could include an option to create born frozen/sealed objects:

const frozenObject = Object.create(null, { foo: { value: 42, enumerable: true } }, { sealed: true });
assert.throws(() => { frozenObject.bar = 'baz'; });
assert(Object.isFrozen(frozenObject));

The use cases for caching the integrity level creation are less clear, but it would look like:

function Foo () {
  this.a = 42
}
Foo.prototype = Object.create(
  Object.prototype,
  {
    toString: {
      value() { return `[Foo ${this.a}]`; }
    },
  },
  {
    sealed: true,
    cachedIntegrity: true,
  },
);

@kriskowal kriskowal added the kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 label Jan 9, 2024
@kriskowal
Copy link
Member

When we were considering this change in 2019, I did not yet appreciate that a harden that transitively freezes both properties and prototypes interferes with lockdown if it is called on almost any object. So, for harden to be more generally useful, useful before lockdown, it would probably need to be reframed as a transitive freeze over properties only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 tc39
Projects
None yet
Development

No branches or pull requests

4 participants