-
Notifications
You must be signed in to change notification settings - Fork 72
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
feat(pass-style,marshal): ByteArray, a binary Passable #2414
base: master
Are you sure you want to change the base?
Conversation
eed07e2
to
1df60e9
Compare
835d70e
to
74007b3
Compare
4dd9bf8
to
d19e509
Compare
74007b3
to
6af60d7
Compare
d19e509
to
585042f
Compare
6af60d7
to
affa6b3
Compare
Converting to draft until more of the stubbed out cases are implemented, and until more of both are tested. |
8dce401
to
06e15fe
Compare
53ec91b
to
ce0fd14
Compare
Staged on #2419 Closes: #XXXX Refs: #2414 #2418 #2419 ## Description #2414 by itself does not work on Node 18 and Node 20 because - those platforms do not have `Array.prototype.transfer`, so #2414 must use `structuredClone` instead - `structuredClone` does exist on Node >= 18, so it should be on supported platforms (though see #2418 ). However, `structuredClone` itself is dangerous and so must not be added to the shared intrinsics. As a result, in #2414 , when `@endo/pass-style` is initialized in a created compartment, it fails to find either `Array.prototype.transfer` and `structuredClone To solve this, @kriskowal suggested that we also shim `Array.prototype.transfer` if needed during `lockdown`, along with other repairs. We are avoiding similarly shimming `Array.prototype.transferToImmutable` because it is not yet standard. But `Array.prototype.transfer` is standard, and so `lockdown` can globally shim it before hardening the shared intrinsics. This PR implements @kriskowal 's suggestion. ### Security Considerations none ### Scaling Considerations by itself, none ### Documentation Considerations nothing signicant. ### Testing Considerations See #2418 . Aside from that, none ### Compatibility and Upgrade Considerations On platforms with neither `Array.prototype.transfer` nor a global `structuredClone`, the ses-shim will *currently* not install an emulation of `Array.prototype.transfer`. However, once we verify that endo is not intended to support platforms without both, we may change lockdown to throw, failing to lock down.
ce0fd14
to
066637e
Compare
066637e
to
e696f45
Compare
// then according to their lengths. | ||
// Thus, if the data of ByteArray X is a prefix of | ||
// the data of ByteArray Y, then X is smaller than Y. | ||
return comparator(left.byteLength, right.byteLength); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should switch to shortlex. See https://en.wikipedia.org/wiki/Shortlex_order
That way, encodePassable could encode as prefix followed by the unescaped content.
// Thus, if the data of ByteArray X is a prefix of | ||
// the data of ByteArray Y, then X is smaller than Y. | ||
// @ts-expect-error narrowed | ||
return compareRank(left.byteLength, right.byteLength); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should delegate the entire compare to rankOrder, which, btw, will switch to shortlex order.
e696f45
to
6312dd7
Compare
6312dd7
to
566c4c8
Compare
Staged on #2414
Closes: #1331
Refs: #1538 ocapn/ocapn#5 (comment)
Description
Use the
@endo/immutable-arraybuffer
ponyfill directly, rather than the shim@endo/immutable-arraybuffer/shim.js
, to provide objects that act like the immutable ArrayBuffers that would result from thetransferToImmutable
proposal.Recognize these objects, when also frozen, as the new pass-style
byteArray
. Add a branch for each of the dispatches on pass-style that must now understand byte-arrays as well. As of this PR, many of those additional branches are stubbed out with "not yet implemented" errors as placeholders.Security Considerations
Does correctly enforce immutability. But by depending only on the ponyfill rather than the shim, this PR inherits the eval twin problem of the ponyfill. Each instantiation of
passStyleOf
will only recognize the byte-arrays from the instantiation of@endo/pass-style
that made that byte-array. For environment that, for example, use thepassStyleOf
endowed to them by liveslots, liveslots will also need to endow them with the corresponding byte-array maker.In theory, this is not much worse than the need to co-endow
passStyleOf
andProxy
, so thatpassStyleOf
can reject apparent copy structures that are also proxies.Scaling Considerations
The underlying ponyfill implementation helps us handle bulk binary data in a largely no-copy or minimal-copy manner.
Documentation Considerations
Will add a new pass-style, so everywhere we enumerate all the pass-styles, we'll need to add byte-array.
Testing Considerations
test.failing
test.Compatibility Considerations
Old code that dispatched only on the pass-styles it knew about will not correctly handle passables that contain byte-arrays.
Upgrade Considerations