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

WeakRef as specified violates tc39 consensus #2214

Closed
erights opened this issue Oct 27, 2020 · 4 comments
Closed

WeakRef as specified violates tc39 consensus #2214

erights opened this issue Oct 27, 2020 · 4 comments

Comments

@erights
Copy link

erights commented Oct 27, 2020

https://tc39.es/ecma262/#sec-weak-ref.prototype.constructor does not mention that the WeakRef.prototype.constructor property itself is normative optional. This is a bug. The tc39 consensus was that it is normative optional, i.e., that a conforming implementation may omit it.

Making it mandatory violates a safety property: The WeakRef constructor is designed to be separately deniable or virtualizable because it creates a huge side channel through the garbage collector. Unlike most side channels, this one does not rely on being able to measure duration. Access to the WeakRef constructor enables detecting of the collection of any object. A weakref instance should enable only the detection of whether that weakref's target is collected. A conforming platform must be able to omit the WeakRef.prototype.constructor property in order to prevent the privilege escalation from detecting the collection of one object to detecting the collection of any object.

During the process from consensus to final merging into the spec, it would be interesting to know when this got dropped. Adding it back in will not invalidate any currently valid engine implementations. Any implementation that conforms to the spec as currently written will also conform to the spec that we agreed on. (Since we need to add it back in anyway, there's a different configuration of WeakRef.prototype.constructor that I would now prefer, that would be closer to the semantics of the spec as currently written. But we should first restore the consensus state before considering alternatives.)

Attn @dtribble @kriskowal @FUDCo @kumavis @danfinlay @phoddie @patrick-soquet @warner @bmeck @guybedford @tschneidereit @syg @littledan @codehag

@littledan
Copy link
Member

The bug here is in CSS, not in intention. If you look in the source, you can see <emu-clause id="sec-weak-ref.prototype.constructor" normative-optional>. In the WeakRef repo, there is corresponding CSS to make this appear as normative optional:

[normative-optional] {
  border-left: 5px solid #ff6600;
  padding: .5em;
  display: block;
  background: #ffeedd;
}
[normative-optional]:before {
  display: block;
  color: #884400;
  content: "NORMATIVE OPTIONAL";
}

It seems like we forgot to copy over this CSS when merging WeakRefs into the main spec. cc @codehag

I'm not sure if this is the most accessible way to present normative optional text. If the editor group is available to look into what would be the best way to present inline normative optional, that would be great.

@codehag
Copy link
Contributor

codehag commented Oct 27, 2020

Yes, this looks like my mistake when I made the merging pr. Would the best thing to do for now be to add the css back in?

@erights
Copy link
Author

erights commented Oct 29, 2020

Grateful that this wasn't lost --- just accidentally invisible.

However, is it appropriate for a normative distinction to be represented only in style/rendering meta-data? Shouldn't the CSS be only about presentation, not content? Shouldn't the meaning of the content remain even if all CSS were erased?

@michaelficarra
Copy link
Member

@erights Yes, we're going to discuss an accessible way to indicate normative optionality in the editor call.

codehag added a commit to codehag/ecma262 that referenced this issue Dec 3, 2020
@ljharb ljharb closed this as completed in 31f3c2b Dec 5, 2020
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

4 participants