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

Update boxes to work in SES environments #257

Closed
wants to merge 4 commits into from
Closed

Update boxes to work in SES environments #257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4f3fb3d
Fix copy-paste error in box objects description
nicolo-ribaudo Sep 30, 2021
b0301e3
`Box.prototype.unbox()` -> `Box.unbox(value)`
nicolo-ribaudo Sep 30, 2021
6268496
Add a realm check when unboxing boxes
nicolo-ribaudo Sep 30, 2021
8578a3c
Don't unwrap boxes in `JSON.stringify` and `String(...)`
nicolo-ribaudo Oct 1, 2021
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
5 changes: 2 additions & 3 deletions spec/abstract-operations.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,16 +769,15 @@ <h1>
<emu-clause id="sec-samevaluenongeneric" type="abstract operation" oldids="sec-samevaluenonnumeric sec-samevaluenonnumber">
<h1>
<del>SameValueNonNumeric</del><ins>SameValueNonGeneric</ins> (
_x_: an ECMAScript language value, but not a Number or a BigInt,
_y_: an ECMAScript language value, but not a Number or a BigInt,
_x_: an ECMAScript language value, but not a Number, a BigInt, a Record, a Tuple, or a Box,
_y_: an ECMAScript language value, but not a Number, a BigInt, a Record, a Tuple, or a Box,
nicolo-ribaudo marked this conversation as resolved.
Show resolved Hide resolved
)
</h1>
<dl class="header">
<dt>description</dt>
<dd>It returns a completion record whose [[Type]] is ~normal~ and whose [[Value]] is a Boolean.</dd>
</dl>
<emu-alg>
1. Assert: Type(_x_) is not Number or BigInt.
1. Assert: Type(_x_) is the same as Type(_y_).
1. If Type(_x_) is Undefined, return *true*.
1. If Type(_x_) is Null, return *true*.
Expand Down
8 changes: 4 additions & 4 deletions spec/data-types-and-values.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ <h1>

<emu-clause id="sec-ecmascript-language-types-box-type">
<h1>The Box Type</h1>
<p>The Box type is the set of all the possible singleton primitive wrappers around any ECMAScript value. Each box value holds an associated [[Value]] containing an ECMAScript value. The [[Value]] internal slot is never modified.</p>

<p>The Box type is the set of all the possible singleton primitive wrappers around any ECMAScript value. Each box value holds an associated [[Value]] containing an ECMAScript value and a [[Realm]] containing the Realm of the Box function which created the Box value. The [[Value]] and [[Realm]] internal slots are never modified.</p>

<emu-clause id="sec-ecmascript-language-types-box-tostring" type="abstract operation">
<h1>
Expand All @@ -170,8 +169,7 @@ <h1>
</h1>
<dl class="header"></dl>
<emu-alg>
1. Let _valueString_ be ? ToString(_argument_.[[Value]]).
1. Return the string-concatenation of *"Box("*, _valueString_, and *")"*.
1. Return *"[object Box]"*.
Copy link
Member

Choose a reason for hiding this comment

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

Since boxes are not objects, should it return Box() ?

Suggested change
1. Return *"[object Box]"*.
1. Return *"Box()"*.

</emu-alg>
</emu-clause>

Expand All @@ -184,6 +182,7 @@ <h1>
</h1>
<dl class="header"></dl>
<emu-alg>
1. If _x_.[[Realm]] is not _y_.[[Realm]], return *false*.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why explicit realm checks are necessary. If the values are objects, then SameValue shouldn't already return false? And if they're primitives, it shouldn't matter what realm they were created in.

Copy link
Member

Choose a reason for hiding this comment

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

if they're primitives, it shouldn't matter what realm they were created in.

this is my stance, but this PR is explicitly trying to change it so that you can't unbox a Box primitive unless you have the Box constructor from the realm it was created in.

Copy link
Member

Choose a reason for hiding this comment

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

To keep object graphs separate for ShadowRealm, and enforce existing security-critical assumptions of membranes between legacy realms, a Box of object cannot be unboxable across a Realm boundary.

The model is then to say that a Box is realm-local primitive opaque wrapper for a value. I originally wanted to allow primitives to be unboxed cross-realm as they wouldn't break the above realm constraints, but @erights pointed out that users may assume that if a box requires permission (through access to Box.unbox) to open, it should apply regardless of the content, as a user may wish to put a sensitive primitive value in it (e.g. unique symbol).

If a box can only be unboxed using the local realm's Box.unbox, it has to be different from another realm's box containing the same value.

Copy link
Member

Choose a reason for hiding this comment

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

To keep object graphs separate for ShadowRealm,

How does a boxed object cross the realm boundary? Shouldn't that just throw?

enforce existing security-critical assumptions of membranes between legacy realms,

What's the failure case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

To keep object graphs separate for ShadowRealm, and enforce existing security-critical assumptions of membranes between legacy realms, a Box of object cannot be unboxable across a Realm boundary.

To further explain the two situations that have been presented as motivating cases for uniquely pairing each box constructor with its unbox function.

“Legacy realms”:

  • Existing code that is not patched in time, running in newer environments.
  • An application that is creating realms (e.g same-origin-iFrames)
  • with all values passed between them being checked for 'isObject', then all object values are secured (denied or wrapped in a Proxy), and all other values passing directly across.
  • And also not restricting globalThis with an allowList (that would remove the unrecognised Box constructor).

Here code in one realm could now give another realm direct access to its objects by putting one in a Tuple using a Box. And now when that value is passed across it will appear as a primitive: Object(tuple) !== tuple so it will pass through as is.

“ShadowRealms”:

Being able to call shadow-realm proxied functions with Records and Tuples would be very useful, e.g. the named argument pattern.

For protection shadowRealms do not allow directly passing objects.

What should happen if a record being passed to a shadowRealm contains a Box with an object inside it? One option is only boxless R&T can be used, otherwise they will throw. Another option is they can be passed across, but not unboxed. This second option could be useful in situations where it’s useful to preserve identity across the boundary but still restrict access.

1. Let _xValue_ be _x_.[[Value]].
1. Let _yValue_ be _y_.[[Value]].
1. Return ! SameValue(_xValue_, _yValue_.)
Expand All @@ -199,6 +198,7 @@ <h1>
</h1>
<dl class="header"></dl>
<emu-alg>
1. If _x_.[[Realm]] is not _y_.[[Realm]], return *false*.
1. Let _xValue_ be _x_.[[Value]].
1. Let _yValue_ be _y_.[[Value]].
1. Return ! SameValueZero(_xValue_, _yValue_.)
Expand Down
22 changes: 13 additions & 9 deletions spec/immutable-data-structures.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,8 @@ <h1>Box ( _arg_ )</h1>
<p>When the `Box` function is called, the following steps are taken:</p>
<emu-alg>
1. If NewTarget is not *undefined*, throw a *TypeError* exception.
1. Return a new Box value whose [[Value]] is _arg_.
1. Let _realm_ be the current Realm Record.
1. Return a new Box value whose [[Value]] is _arg_ and whose [[Realm]] is _realm_.
</emu-alg>
</emu-clause>
</emu-clause>
Expand Down Expand Up @@ -675,6 +676,17 @@ <h1>RecursiveContainsBoxes ( _value_ )</h1>
</emu-clause>
</emu-clause>

<emu-clause id="sec-box.unbox">
<h1>Box.unbox ( _value_ )</h1>
Copy link
Member

Choose a reason for hiding this comment

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

This is much less ergonomic to use. It's not clear to me why it needs to be a static method instead of a instance method.

Copy link
Member

Choose a reason for hiding this comment

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

Is Box.unbox(box) that much less ergonomic than box.unbox()?
Also nit, box being a primitive, it wouldn't per-se be an instance method, but a prototype method of the Box object wrapper. Users are still free to install their own:

Box.prototype.unbox = function unbox() {
  return Box.unbox(this);
};

Copy link
Member

Choose a reason for hiding this comment

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

Is Box.unbox(box) that much less ergonomic than box.unbox()?

In an OOP oriented language, considerably less.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Potentially a use case for pipeline |> ?

<p>The `unbox` function extracts the wrapped ECMAScript language value from _box_, throwing if it has been wrapped by a different Realm's Box constructor. When called, the following steps are taken:</p>
<emu-alg>
1. Let _box_ be ? thisBoxValue(_value_).
1. Let _realm_ be the current Realm Record.
1. If _box_.[[Realm]] is not _realm_, throw a TypeError exception.
1. Return _box_.[[Value]].
</emu-alg>
</emu-clause>

<emu-clause id="sec-box.prototype">
<h1>Box.prototype</h1>
<p>The initial value of *Box.prototype* is %Box.prototype%</p>
Expand Down Expand Up @@ -715,14 +727,6 @@ <h1>Box.prototype [ @@toStringTag ]</h1>
<p>The initial value of *Box.prototype[@@toStringTag]* is the String value *"Box"*.</p>
<p>This property has the attributes { [[Writable]]: *false*, [[Enumerable]]: *false*, [[Configurable]]: *true* }.</p>
</emu-clause>
<emu-clause id="sec-box.prototype.unbox">
<h1>Box.prototype.unbox ( )</h1>
<p>When the `unbox` function is called, the following steps are taken:</p>
<emu-alg>
1. Let _box_ be ? thisBoxValue(*this* value).
1. Return _box_.[[Value]]
</emu-alg>
</emu-clause>
</emu-clause>
</emu-clause>
</emu-clause>
2 changes: 1 addition & 1 deletion spec/ordinary-and-exotic-object-behaviours.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ <h1>

<emu-clause id="sec-box-exitic-objects">
<h1>Box Exotic Objects</h1>
<p>A Box object is an exotic object that encapsuates a Box value and exposes virtual integer-indexed data properties corresponding to the individual entries set on the underlying Box value. All keys properties are non-writable and non-configurable.</p>
<p>A Box object is an exotic object that encapsulates a Box value. It doesn't have any own key.</p>

<p>An object is a <dfn id="box-exotic-object">Box exotic object</dfn> (or simply, a Box object) if its following internal methods use the following implementations and it is an <emu-xref href="#sec-immutable-prototype-exotic-objects">Immutable Prototype Exotic Object</emu-xref>.</p>

Expand Down
30 changes: 2 additions & 28 deletions spec/structured-data.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,12 @@ <h1>
<dl class="header"></dl>
<emu-alg>
1. Let _value_ be ? Get(_holder_, _key_).
1. <ins>Let _toJSONCalled_ be *false*.</ins>
1. If Type(_value_) is Object or BigInt, then
1. If Type(_value_) is Object<ins>, Box,</ins> or BigInt, then
1. Let _toJSON_ be ? GetV(_value_, *"toJSON"*).
1. If IsCallable(_toJSON_) is *true*, then
1. Set _value_ to ? Call(_toJSON_, _value_, &laquo; _key_ &raquo;).
1. <ins>Set _toJSONCalled_ to *true*.</ins>
1. If _state_.[[ReplacerFunction]] is not *undefined*, then
1. Set _value_ to ? Call(_state_.[[ReplacerFunction]], _holder_, &laquo; _key_, _value_ &raquo;).
1. <ins>Set _value_ to ! MaybeUnwrapBox(_value_).</ins>
1. <ins>Else,</ins>
1. <ins>Set _value_ to ! MaybeUnwrapBox(_value_).</ins>
1. <ins>If _toJSONCalled_ is *false*, then</ins>
1. <ins>If Type(_value_) is Object or BigInt, then</ins>
1. <ins>Let _toJSON_ be ? GetV(_value_, *"toJSON"*).</ins>
1. <ins>If IsCallable(_toJSON_) is *true*, then</ins>
1. <ins>Set _value_ to ? Call(_toJSON_, _value_, &laquo; _key_ &raquo;).</ins>
1. <ins>Set _value_ to ! MaybeUnwrapBox(_value_).</ins>
1. If Type(_value_) is Object, then
1. If _value_ has a [[NumberData]] internal slot, then
1. Set _value_ to ? ToNumber(_value_).
Expand All @@ -169,7 +158,7 @@ <h1>
1. If Type(_value_) is Number, then
1. If _value_ is finite, return ! ToString(_value_).
1. Return *"null"*.
1. If Type(_value_) is BigInt, throw a *TypeError* exception.
1. If Type(_value_) is BigInt <ins>or Box</ins>, throw a *TypeError* exception.
1. If Type(_value_) is Object and IsCallable(_value_) is *false*, then
1. <del>Let _isArray_ be ? IsArray(_value_).</del>
1. <ins>If ! IsTuple(_value_) is *true*, then</ins>
Expand All @@ -181,21 +170,6 @@ <h1>
1. Return *undefined*.
</emu-alg>
</emu-clause>

<emu-clause id="sec-maybeunwrapbox" type="abstract operation">
<h1>
<ins>MaybeUnwrapBox</ins> (
_value_: an ECMAScript language value,
)
</h1>
<dl class="header"></dl>
<emu-alg>
1. If Type(_value_) is Object and _value_ has a [[BoxData]] internal slot,
1. Set _value_ to _value_.[[BoxData]].
1. If Type(_value_) is Box, return ! MaybeUnwrapBox(_value_.[[Value]]).
1. Return _value_.
</emu-alg>
</emu-clause>
</emu-clause>
</emu-clause>
</emu-clause>