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

Implement (un)wrapping through WeakMap or private class properties #248

Open
TimothyGu opened this issue Jun 27, 2021 · 1 comment
Open

Comments

@TimothyGu
Copy link
Member

Right now, we implement wrapping/unwrapping of IDL objects through a symbol:

function wrapperForImpl(impl) {
return impl ? impl[wrapperSymbol] : null;
}
function implForWrapper(wrapper) {
return wrapper ? wrapper[implSymbol] : null;
}
function tryWrapperForImpl(impl) {
const wrapper = wrapperForImpl(impl);
return wrapper ? wrapper : impl;
}
function tryImplForWrapper(wrapper) {
const impl = implForWrapper(wrapper);
return impl ? impl : wrapper;
}

However, this is not only allows client scripts to escape the jsdom environment, it also fools brand checks if prototypes are used:

const url = new URL("...");
const notURL = Object.create(url);
console.log(notURL.href);  // should throw TypeError but doesn't

On the other hand, private class properties are immune to this, due to their so-called WeakMap semantics. We could very well use the so-called "super-return trick" to implement IDL objects:

class Returner {
  constructor(obj) {
    return obj;
  }
}
class Brander extends Returner {
  #wrapped;
  static getWrapped(obj) {
    return obj.#wrapped;
  }
  static setWrapped(obj, wrapped) {
    new Brander(obj);
    obj.#wrapped = wrapped;
    return obj
  }
}

const urlImpl = new URLImpl();
const url = Object.create(URL.prototype);
Brander.setWrapped(url, urlImpl);

const roundtrippedImpl = Brander.getWrapped(url);
console.assert(urlImpl === roundtrippedImpl);

Private properties are supported since Node.js v12.x. They are supposed to be faster than WeakMap, but we should still do some performance investigations. (I expect it to be slower than symbol properties still.)

@Sebmaster
Copy link
Member

Oh I just recently saw this trick on Twitter and thought it might work for our object branding purposes. I was a bit worried about backwards compat, but node v12 sounds perfectly fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants