feat(immutable-arraybuffer): sliceToImmutable Hermes ponyfill and shim#2785
Conversation
438d05e to
a69fb24
Compare
| ...asyncArrowEliminator, | ||
| ...asyncGeneratorDestroyer, | ||
| ...removeImmutableArrayBufferShim, | ||
| ...immutableArrayBufferPonyfier, |
There was a problem hiding this comment.
we're not able to replace import '@endo/immutable-arraybuffer/shim.js'; in ses/lockdown.js with e.g.
const ab = new ArrayBuffer(0);
if (typeof ab.transferToImmutable === 'function') {
import('@endo/immutable-arraybuffer/shim.js');
} else {
import('@endo/immutable-arraybuffer/shim-hermes.js');
}so this transform is needed
There was a problem hiding this comment.
@kriskowal , I'm fine with the ponyfier. But do you now have some module magic that lets us conditionally synchronously import one of the other depending on platforms? Are we doing something similar to adapt to XS?
@kriskowal you're also the right person to ask: should it be "ponyfier" or "ponifier". FWIW, I prefer it as is, with the "y".
There was a problem hiding this comment.
module magic that lets us conditionally synchronously import one of the other depending on platforms? Are we doing something similar to adapt to XS?
happy to follow-up and replace this transform with it if we do ^ or if we're doing something similar with XS
|
@erights ready on my side, if you have any further feedback edit: all discussions addressed |
| ...asyncArrowEliminator, | ||
| ...asyncGeneratorDestroyer, | ||
| ...removeImmutableArrayBufferShim, | ||
| ...immutableArrayBufferPonyfier, |
There was a problem hiding this comment.
@kriskowal , I'm fine with the ponyfier. But do you now have some module magic that lets us conditionally synchronously import one of the other depending on platforms? Are we doing something similar to adapt to XS?
@kriskowal you're also the right person to ask: should it be "ponyfier" or "ponifier". FWIW, I prefer it as is, with the "y".
|
Please also fill out the PR form at the top before merging. |
f525105 to
316ea46
Compare
Co-authored-by: Mark S. Miller <erights@users.noreply.github.com>
Co-authored-by: Mark S. Miller <erights@users.noreply.github.com>
316ea46 to
d81dca0
Compare
# `ses` v1.13.0 - Two new `stackFiltering:` options are added - `'omit-frames'` -- Only omit likely uninteresting frames. Keep original paths. - `'shorten-paths'` -- Only shorten paths to text likely clickable in an IDE This fills out the matrix of what should have been orthogonal options. The existing `'concise'` setting both omits likely uninteresting frames and shortens their paths. The existing `'verbose'` setting does neither. - Uses the `@endo/immutable-arraybuffer` shim to add `ArrayBuffer.p.immutable`, `ArrayBuffer.p.transferToImmutable`, and `ArrayBuffer.p.sliceToImmutable` to ses, in order to emulate the [Immutable ArrayBuffer proposal](https://github.com/tc39/proposal-immutable-arraybuffer). These make an ArrayBuffer-like object whose contents cannot be mutated. However, due to limitations of the shim - Unlike `ArrayBuffer` and `SharedArrayBuffer` this shim's ArrayBuffer-like object cannot be transfered or cloned between JS threads. - Unlike `ArrayBuffer` and `SharedArrayBuffer`, this shim's ArrayBuffer-like object cannot be used as the backing store of TypeArrays or DataViews. - The shim depends on the platform providing either `structuredClone` or `Array.prototype.transfer`. Node <= 16 and provides neither, causing the shim to fail to initialize, and therefore SES to fail to initialize on such platforms. - Current Hermes has even stronger constraints, lacking `structuredClone`, `transfer`, private fields, and even `class` syntax. This requires other coping strategies. See #2785 - Even after the upcoming `transferToImmutable` proposal is implemented by the platform, the current code will still replace it with the shim implementation, in accord with shim best practices. See #2311 (comment) . It will require a later manual step to delete the shim or have it avoid overriting a platform implementation, after manual analysis of the compat implications. - The [evalTaming](https://github.com/endojs/endo/blob/master/packages/ses/docs/lockdown.md#evaltaming-options) option `'safe-eval'` now can only throw error `SES_DIRECT_EVAL`. This allows SES to initialize with `'unsafe-eval'` or `'no-eval'` on hosts with no direct eval available such as Hermes for a successful lockdown that tolerates it's language [features](https://github.com/facebook/hermes/blob/main/doc/Features.md). The module name `ses/hermes` can now be required to call `lockdown` and `repairIntrinsics` only, `Compartment` is not yet available. It is currently compatible with Hermes [v0.12.0](https://www.npmjs.com/package/hermes-engine-cli/v/0.12.0), we plan to support [v0.13.0](https://github.com/facebook/hermes/releases/tag/v0.13.0) then subsequent Hermes [tags](https://github.com/facebook/hermes/tags) or side-by-side versions built for React Native depending on ecosystem usage and official [support](https://github.com/reactwg/react-native-releases/blob/main/docs/support.md), then Static Hermes when released. Also `ses/hermes` can now be hooked into bundlers such as Metro to run Hardened JS. # `@endo/compartment-mapper` v1.6.1 - The `dev` flag for `mapNodeModules()` is no longer deprecated. The concept of a "condition" ([conditional exports](https://nodejs.org/api/packages.html#conditional-exports)) is disinct from the flag's original meaning (instructs `mapNodeModules()` to consider `devDependencies` when graphing packages). Users who have switched to using a `development` condition for `dev`'s purpose are encouraged to _switch back_ to using the `dev` flag instead. **In a future release, the presence of a `development` condition will no longer mimic an enabled `dev` flag** and will only be considered when evaluating conditional exports. # `@endo/evasive-transform` v2.0.0 - The `sourceType` option is now restricted to `script` and `module` only. Function signature types have changed to be more precise. # `@endo/bundle-source` v4.1.0 - The `'endoZipBase64'` moduleFormat now utilizes the `importHook` option to exit dependencies whose specifiers return a truthy value. # `@endo/import-bundle` v1.5.0 - The `'endoZipBase64'` moduleFormat now utilizes the `importHook` option.
Closes: #XXXX Refs: #2785 ## Description The Immutable ArrayBuffer shim, on non-Hermes, relies on the existence of *either* a global `structuredClone` or `Array.prototype.transfer`. However, I forgot to capture early the global `structuredClone` into a module-local one, and to then use that repeatedly. MetaMask scuttling deleted the global `structuredClone`, running headlong into my mistake on platforms (Node 18 and Node 20) that do not also have `Array.prototype.transfer`. This PR fixes that mistake, capturing `structuredClone` early, and then using the resulting module-local one. ### Security Considerations The bug this PR fixes was a security hazard. ### Scaling Considerations none ### Documentation Considerations none ### Testing Considerations In theory, we should add scuttling as an orthogonal dimension of our platform testing matrix. However that is rather expensive for minor benefit. ### Compatibility Considerations The bug created compat problems that this PR fixes. ### Upgrade Considerations none
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>
Refs: (#2399) #2400 #2798
Description
Rather than excluding the
'@endo/immutable-arraybuffer/shim.js'import in the Hermes SES shim(since Hermes lacks
structuredClone,transfer, private fields andclasssyntax),here we introduce
'@endo/immutable-arraybuffer/shim-hermes.js'- a limited version achieved without those.TODO
Security Considerations
Documented in source code
Scaling Considerations
Documentation Considerations
Documented in source code
Testing Considerations
Separate tests added for Hermes
Compatibility Considerations
See #2785 (comment) RE expanding the transform to support more than side effect imports if one day required
Upgrade Considerations
SES NEWS.md bullet already added for next release