Skip to content

feat(immutable-arraybuffer): transferToImmutable ponyfill and shim#2399

Merged
erights merged 3 commits into
masterfrom
markm-to-immutable-2
Aug 13, 2024
Merged

feat(immutable-arraybuffer): transferToImmutable ponyfill and shim#2399
erights merged 3 commits into
masterfrom
markm-to-immutable-2

Conversation

@erights

@erights erights commented Aug 6, 2024

Copy link
Copy Markdown
Contributor

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

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 self-assigned this Aug 6, 2024
@erights erights force-pushed the markm-to-immutable-2 branch 4 times, most recently from d40f56f to d610950 Compare August 7, 2024 02:10
@erights erights marked this pull request as ready for review August 7, 2024 02:10
@erights erights marked this pull request as draft August 7, 2024 02:11
@erights erights force-pushed the markm-to-immutable-2 branch from d610950 to e4be967 Compare August 7, 2024 03:02
@erights erights marked this pull request as ready for review August 7, 2024 03:03
@erights erights force-pushed the markm-to-immutable-2 branch 2 times, most recently from b71e84c to c1d8ab4 Compare August 7, 2024 03:34
@erights erights changed the title feat(immutable-arraybuffer): transferToImmutable shim feat(immutable-arraybuffer): transferToImmutable ponyfill and shim Aug 7, 2024
@erights erights force-pushed the markm-to-immutable-2 branch from c1d8ab4 to f531bc8 Compare August 7, 2024 18:43
@erights

erights commented Aug 10, 2024

Copy link
Copy Markdown
Contributor Author

OCapN meeting coming up on Tuesday. So if there's no problem with this, I'd like to make progress before then. Ping ;)

Comment thread packages/immutable-arraybuffer/package.json Outdated
Comment thread packages/immutable-arraybuffer/test/index.test.js Outdated
Comment thread packages/immutable-arraybuffer/index.js
Comment thread packages/immutable-arraybuffer/README.md Outdated

@gibson042 gibson042 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The code looks reasonable, but I feel pretty strongly that we should not have a polyfill at all... liveslots will need to use a new endo to take advantage of this anyway, and it IMO should explicitly propagate transferBufferToImmutable and isBufferImmutable as globals into all descendant compartments rather than manipulating a built-in prototype and creating compatibility concerns.

Comment thread packages/immutable-arraybuffer/README.md Outdated
Comment thread packages/immutable-arraybuffer/SECURITY.md Outdated
Comment thread packages/immutable-arraybuffer/SECURITY.md Outdated
Comment thread packages/immutable-arraybuffer/README.md
Comment thread packages/immutable-arraybuffer/package.json Outdated
Comment thread packages/immutable-arraybuffer/index.js Outdated
Comment thread packages/immutable-arraybuffer/index.js Outdated
Comment thread packages/immutable-arraybuffer/index.js Outdated
Comment thread packages/immutable-arraybuffer/index.js
@erights erights force-pushed the markm-to-immutable-2 branch from f531bc8 to 262bcf0 Compare August 13, 2024 17:44
@erights erights enabled auto-merge (squash) August 13, 2024 18:51
@erights erights merged commit dd3f5c2 into master Aug 13, 2024
@erights erights deleted the markm-to-immutable-2 branch August 13, 2024 18:59
erights added a commit that referenced this pull request Jun 18, 2025
Closes: #XXXX
Refs: #2785 #2399

## Description

The Immutable ArrayBuffers shim can only emulate `transferToImmutable`
on platforms with either `structuredClone` or
`ArrayBuffer.prototype.transfer`. Otherwise, the shim would refuse to
initialize. Without Immutable ArrayBuffers, we cannot initialize `ses`,
and `pass-style` cannot implement the new `byteArray` Passable type,
which `marshal` and `patterns` now depends on.

Unfortunately, it turns out that some platforms we want to support do
not have `structuredClone` or `ArrayBuffer.prototype.transfer`. The
first we encountered was Hermes, that also does not have classes with
private fields. #2785 makes all these work on Hermes, by supporting
`sliceToImmutable` instead, and using WeakMaps rather than private
fields. On the theory that Hermes was a one-off, #2785 sniffed to see if
it was on Hermes, only only then used the alternate shim.

In turns out Hermes was not a one-off. Some version of JSC of concern
(TODO which ones?) also have neither `structuredClone` or
`ArrayBuffer.prototype.transfer`. So this PR unifies the two shim
strategies into one which uses the WeakMap technique, which works
everywhere, and emulating `sliceToImmutable`, which we can do
everywhere. It still sniffs for `structuredClone` or
`ArrayBuffer.prototype.transfer`, and only emulates
`transferToImmutable` if one is present. Thus, we sniff only for
features not platforms, and emulate `sliceToImmutable` on all platforms.

Other PRs (TODO: which ones?) already ensure that `pass-style`,
`marshal`, and `patterns` are fine with the absence of
`transferToImmutable`.

### Security Considerations

A singe piece of code with small feature detects is simpler, and thus
more secure, than maintaining two alternate pieces of code, each for a
different set of platforms.

### Scaling Considerations

On most platforms, a class with private fields is probably more
efficient than a WeakMap. This PR sacrifices that optimization in
exchange for unification.

### Documentation Considerations

Less to document.

### Testing Considerations

The "unified" tests are just the union of the old tests, and so have two
problems
- [ ] redundancy, which needs to be removed
- [ ] contains unconditional `transferToImmutable` tests that won't work
on some platforms. These specific tests need to be made conditional,
probably using the same `test.skip` trick some tests use for
`Array.prototype.transfer`.

### Compatibility Considerations

The goal is to make the shim compat with more platforms. On platforms
that worked before this PR, this PR should be a pure refactor, and so
without any compat hazards.

### Upgrade Considerations

- [x] Update `NEWS.md` for user-facing changes.

---------

Co-authored-by: Christopher Hiller <boneskull@boneskull.com>
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

Successfully merging this pull request may close these issues.

Support passable immutable ArrayBuffer as copy-data

3 participants