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.

Sign up for GitHub

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

Jump to bottom

Normative: Restrict the use of PrivateNames in class decorators #136

Closed
wants to merge 1 commit into from
Closed

Normative: Restrict the use of PrivateNames in class decorators #136

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,11 +438,12 @@ <h1>%PrivateName% ( )</h1>
</emu-clause>

<emu-clause id="sec-private-name-object" aoid=PrivateNameObject>
<h1>PrivateNameObject ( _name_ )</h1>
<h1>PrivateNameObject ( _name_, _restricted_ )</h1>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it might be better to name this "privileged" instead of "restricted", so that behavior can default to the safe thing instead of to the unsafe thing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting a change in runtime semantics, or just an editorial change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An editorial change, to invert the default from "privileged" to "restricted".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The design guideline used in ES5 when designing property descriptors:

Names should say what is allowed rather than what is denied. APIs should follow the security best practice of "deny by default".

Applying that guideline to this API the second parameter might be called readWriteAllowed and should default to false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, there is no public API proposed in this patch; we are talking about names for what goes on when decorators expose private fields.

How about getSetAllowed? Read/write would be sort of new terms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that it isn't a public API, but it is still an API used by spec. writers and readers.

I don't really care what name you chose as long as it describes what setting it to true enables.

<p>When PrivateNameObject is called with Private Name _name_, the following steps are taken:</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"and Boolean _restricted_"? (and then should there be an Assert that restricted is a boolean, or is that implied?)

<emu-alg>
1. Let _O_ be ? ObjectCreate(%PrivateNamePrototype%, &laquo; [[PrivateName]] &raquo;).
1. Let _O_ be ? ObjectCreate(%PrivateNamePrototype%, &laquo; [[PrivateNameData]], [[PrivateNameRestricted]] &raquo;).
1. Set _O_.[[PrivateNameData]] to _name_.
1. Set _O_.[[PrivateNameRestricted]] to _restricted_.
1. Perform ! SetIntegrityLevel(_O_, `"frozen"`).
1. Return _O_.
</emu-alg>
Expand All @@ -464,6 +465,7 @@ <h1>%PrivateName%.prototype.get ( _object_ )</h1>
<emu-alg>
1. Let _O_ be the *this* value.
1. Let _pn_ be ? GetPrivateName(_O_).
1. If _pn_.[[PrivateNameRestricted]] is *true*, throw a *TypeError* exception.
1. If Type(_object_) is not Object, throw a *TypeError* exception.
1. Return ? PrivateFieldGet(_pn_, _object_).
</emu-alg>
Expand All @@ -475,6 +477,7 @@ <h1>%PrivateName%.prototype.set ( _object_, _value_ )</h1>
<emu-alg>
1. Let _O_ be the *this* value.
1. Let _pn_ be ? GetPrivateName(_O_).
1. If _pn_.[[PrivateNameRestricted]] is *true*, throw a *TypeError* exception.
1. If Type(_object_) is not Object, throw a *TypeError* exception.
1. Return ? PrivateFieldSet(_pn_, _object_, _value_).
</emu-alg>
Expand Down Expand Up @@ -521,7 +524,7 @@ <h1>GetPrivateName ( _O_ )</h1>

<emu-clause id="sec-properties-of-private-name-instances">
<h1>Properties of PrivateName Instances</h1>
<p>PrivateName instances are ordinary objects that inherit properties from the PrivateName prototype object. PrivateName instances have a [[PrivateNameData]] internal slot. The [[PrivateNameData]] internal slot is the Private Name value represented by this Private Name object.</p>
<p>PrivateName instances are ordinary objects that inherit properties from the PrivateName prototype object. PrivateName instances have [[PrivateNameData]] and [[PrivateNameRestricted]] internal slots. The [[PrivateNameData]] internal slot is the Private Name value represented by this Private Name object. [[PrivateNameRestricted]] is a boolean, indicating whether `get` and `set` operations are permitted. </p>
</emu-clause>
</emu-clause>
</emu-clause>
Expand Down Expand Up @@ -657,7 +660,7 @@ <h1>DecorateElement ( _element_, _placements_ )</h1>
1. Let _finishers_ be a new empty List.
1. For each _decorator_ in _element_.[[Decorators]], in reverse list order do
1. Perform RemoveElementPlacement(_element_, _placements_).
1. Let _elementObject_ be ? FromElementDescriptor(_element_).
1. Let _elementObject_ be ? FromElementDescriptor(_element_, *false*).
1. Let _elementFinisherExtrasObject_ be ? Call(_decorator_, *undefined*, « _elementObject_ »).
1. If _elementFinisherExtrasObject_ is *undefined*,
1. Let _elementFinisherExtrasObject_ be _elementObject_.
Expand Down Expand Up @@ -731,21 +734,21 @@ <h1>FromElementDescriptors ( _elements_ )</h1>
1. Assert: _elements_ is a List of ElementDescriptor Records.
1. Let _elementObjects_ be a new empty List.
1. For each _element_ in _elements_, do
1. Append ! FromElementDescriptor(_element_) to _elementObjects_.
1. Append ! FromElementDescriptor(_element_, *true*) to _elementObjects_.
1. Return ! CreateArrayFromList(_elementObjects_).
</emu-alg>
</emu-clause>

<emu-clause id=sec-from-element-descriptor aoid=FromElementDescriptor>
<h1>FromElementDescriptor ( _element_ )</h1>
<h1>FromElementDescriptor ( _element_, _restricted_ )</h1>
<emu-alg>
1. Assert: _element_ is an ElementDescriptor Record.
1. Let _obj_ be ! ObjectCreate(%ObjectPrototype%).
1. Let _desc_ be PropertyDescriptor{ [[Value]]: `"Descriptor"`, [[Writable]]: *false*, [[Enumerable]]: *false*, [[Configurable]]: *true* }.
1. Perform ! DefinePropertyOrThrow(_obj_, @@toStringTag, _desc_).
1. Perform ! CreateDataPropertyOrThrow(_obj_, `"kind"`, _element_.[[Kind]]).
1. Let _key_ be _element_.[[Key]].
1. If _key_ is a Private Name, set _key_ to ? PrivateNameObject(_key_).
1. If _key_ is a Private Name, set _key_ to ? PrivateNameObject(_key_, _restricted_).
1. Perform ! CreateDataPropertyOrThrow(_obj_, `"key"`, _key_).
1. Perform ! CreateDataPropertyOrThrow(_obj_, `"placement"`, _element_.[[Placement]]).
1. Perform ! CreateDataPropertyOrThrow(_obj_, `"descriptor"`, ! FromPropertyDescriptor(_element_.[[Descriptor]])).
Expand Down