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

feat(pass-style,marshal): ByteArray, a new binary Passable type #1538

Draft
wants to merge 1 commit into
base: markm-to-immutable-3
Choose a base branch
from
Draft

feat(pass-style,marshal): ByteArray, a new binary Passable type #1538

wants to merge 1 commit into from

Conversation

erights
Copy link
Contributor

@erights erights commented Apr 4, 2023 •
edited
Loading

Staged on #2311

Still very drafty, but a start

Fixes #1331
See plan at ocapn/ocapn#5 (comment)

@erights erights requested review from dckc and gibson042 April 4, 2023 03:44
@erights erights self-assigned this Apr 4, 2023
@erights erights mentioned this pull request Apr 4, 2023
Closed
mhofman
mhofman reviewed Apr 4, 2023
View reviewed changes
Copy link
Contributor

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

I think we should try to align copyBytes with the prior art of the Web platform's Blob.

packages/pass-style/src/types.js Outdated Show resolved Hide resolved
packages/pass-style/src/copyBytes.js Outdated Show resolved Hide resolved
packages/pass-style/src/copyBytes.js Outdated Show resolved Hide resolved
packages/pass-style/src/copyBytes.js Outdated Show resolved Hide resolved
packages/pass-style/src/copyBytes.js Outdated Show resolved Hide resolved
@erights erights mentioned this pull request Apr 13, 2023
Open
@erights erights mentioned this pull request Jun 25, 2023
Open
6 tasks
@erights erights force-pushed the markm-binary branch 2 times, most recently from 80cc7f2 to fca467e Compare September 16, 2023 03:35
@erights erights force-pushed the markm-binary branch 4 times, most recently from 15cbf4f to a6a6ca0 Compare September 26, 2023 05:51
@erights erights force-pushed the markm-binary branch 2 times, most recently from 4bf3d8c to ce5f3bf Compare November 7, 2023 19:22
@erights erights changed the title WIP fix: CopyBytes new binary Passable type WIP feat(pass-style,marshal)!: ByteArray, a new binary Passable type Jan 11, 2024
@erights erights force-pushed the markm-binary branch 3 times, most recently from b25c593 to fe561d2 Compare January 11, 2024 06:24
@erights erights added the ocapn label Jun 8, 2024
mhofman
mhofman reviewed Jun 8, 2024
View reviewed changes
Copy link
Contributor

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

I actually like the direction this is going. It's relying on a global to have certain semantics, which we can shim, either in ses or in a separate independent shim. We mainly have to hope the committee would be accepting this or we'll end up with some weird compat issues

packages/pass-style/src/byteArray.js Show resolved Hide resolved
@erights erights force-pushed the markm-binary branch 2 times, most recently from 371431a to 177f45f Compare June 13, 2024 13:29
@erights erights force-pushed the markm-byte-array-2 branch from 5eb079d to 2043b76 Compare July 11, 2024 20:22
@erights erights force-pushed the markm-byte-array-2 branch from 2043b76 to 4dc9583 Compare July 13, 2024 22:59
@erights erights mentioned this pull request Aug 7, 2024
Merged
erights added a commit that referenced this pull request Aug 13, 2024
@erights
feat(immutable-arraybuffer): transferToImmutable ponyfill and shim (#…
dd3f5c2 
…2399)

Closes: #XXXX
Refs:  #1538 #1331 #2309 #2311 

## Description

Introduces the `@endo/immutable-arraybuffer` package, the ponyfill
exports of `@endo/immutable-arraybuffer`, and the shim obtained by
importing `@endo/immutable-arraybuffer/shim.js`.

Alternative to #2309 as suggested by @phoddie at
#2309 (comment)

We plan to fix #1331 in a stack of PRs starting with this one
- This PR implements a ponyfill and shim for an upcoming *Immutable
ArrayBuffer* proposal, along the lines suggested by @phoddie at
#2309 (comment) + the
suggestions on an earlier state of #2311 . This is a pure JavaScript
ponyfill/shim, leaving it to #2311 to bring it into Hardened JavaScript.
- #2311 imports the #2399 shim, treating the new objects it introduces
as if they are new primordials, to be permitted and hardened.
- #1538 uses the Hardened JavaScript Immutable ArrayBuffers to define a
new `Passable` type, `ByteArray`, corresponding to the
[OCapN](https://ocapn.org/) `ByteArray`.
- Some future PR extending the various marshal formats to encode and
decode the `ByteArray` objects.

See the README.md in this PR for more.

### Security Considerations

Better support for immutability generally helps security. The
imperfections of the shim are a leaky abstraction in all the ways
explained in the Caveats section of the README.md. For example, objects
that are purely artifacts of the emulation, like the
`immutableArrayBufferPrototype`, are easily discoverable, revealing the
emulation's mechanisms.

As a pure JavaScript polyfill/shim, this `@endo/immutable-arraybuffer`
package does not harden the objects it exposes. Thus, by itself it does
not provide much security -- like the initial state of JavaScript does
not by itself provide much security. Rather, both provide securability,
depending on Hardened JavaScript to harden early as needed to provide
the security. See #2311

Once hardened early, the abstraction will still be leaky as above, but
the immutability of the buffer contents are robustly enforced.

### Scaling Considerations

This ponyfill/shim is a zero-copy implementation, meaning that it does
no more buffer copying than expected of a native implementation.

### Compatibility and Documentation Considerations

This ponyfill/shim implements zero-copy by relying on the platform to
provide one of two primitives: `structuredClone` or
`ArrayBuffer.prototype.transfer`. Neither exist on Node <= 16. Without
either, this ponyfill/shim will fail to initialize.

This PR sets the stage for writing an Immutable ArrayBuffer proposal,
proposing it to tc39, and including it in our own documentation.

### Testing Considerations

Ideally, we should identify the subset of test262 `ArrayBuffer` tests
that should be applicable to immutable ArrayBuffers, and duplicate them
for that purpose.

### Upgrade Considerations

Nothing breaking.
@erights erights changed the base branch from markm-byte-array-2 to markm-to-immutable-3 August 16, 2024 22:01
@erights erights mentioned this pull request Aug 19, 2024
Draft
3 tasks
@erights erights force-pushed the markm-to-immutable-3 branch from c9c9300 to 19de016 Compare September 2, 2024 21:24
@erights erights force-pushed the markm-to-immutable-3 branch from 19de016 to 5972da3 Compare September 7, 2024 20:31
@erights erights force-pushed the markm-to-immutable-3 branch from 5972da3 to 1e582fb Compare October 14, 2024 19:30
erights
erights commented Oct 14, 2024
View reviewed changes
packages/marshal/src/rankOrder.js
// 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change to shortlex order. See https://en.wikipedia.org/wiki/Shortlex_order

Shortlex order would enable encodePassable (aka compactOrdered) to encode buffers with a prefix followed by the unescaped bytes. Avoiding escaping would be great.

Copy link
Contributor

Choose a reason for hiding this comment

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

So this makes ByteArray different than string for ordering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. For strings, normal lexicographic ordering is overwhelmingly more expected than shortlex. For binaries of different length, I don't think there is any one overwhelming expectation.

FWIW, if I did think we could get away with shortlex for strings too, I would go for it, for the same reason. Needing to insert escapes into bulk data sucks. Attn @gibson042

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, if I did think we could get away with shortlex for strings too, I would go for it

That would be a breaking change that would have to be opted into at definition of the stores and other components that use the order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. That's another reason we cannot get away with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, because of this breaking change issue, we haven't even moved forward on #2008

erights
erights commented Oct 14, 2024
View reviewed changes
packages/patterns/src/keys/compareKeys.js
// 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If both are byteArrays, then we should delegate the whole thing to compareRank. Which, btw, we'll be switching to shortlex order.

@erights erights force-pushed the markm-to-immutable-3 branch from 1e582fb to b32ab45 Compare October 29, 2024 00:01
@erights erights force-pushed the markm-to-immutable-3 branch from b32ab45 to 544a213 Compare November 17, 2024 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dckc dckc dckc left review comments

@mhofman mhofman mhofman left review comments

@gibson042 gibson042 Awaiting requested review from gibson042

Assignees

@erights erights

Labels
ocapn
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support passable immutable ArrayBuffer as copy-data
3 participants
@erights @mhofman @dckc