Skip to content
This repository was archived by the owner on Oct 24, 2023. It is now read-only.

ArrayBuffer.prototype.resize - detached buffer handling? #116

Closed
phoddie opened this issue Dec 27, 2022 · 9 comments · May be fixed by #120
Closed

ArrayBuffer.prototype.resize - detached buffer handling? #116

phoddie opened this issue Dec 27, 2022 · 9 comments · May be fixed by #120

Comments

@phoddie
Copy link

phoddie commented Dec 27, 2022

The argument to resize can cause user-code to execute. That code could call transfer, detaching the buffer. The algorithm steps don't check for [[IsDetachedBuffer]] after coercing the argument in Step 5:

  1. If IsDetachedBuffer(O) is true, throw a TypeError exception.
  2. Let newByteLength be ? ToIntegerOrInfinity(newLength).
  3. If newByteLength < 0 or newByteLength > O.[[ArrayBufferMaxByteLength]], throw a RangeError exception.
  4. Let hostHandled be ? HostResizeArrayBuffer(O, newByteLength).

This turned up as a crash in testing the implementation in XS. One solution is to repeat Step 4 after Step 6. Whatever the solution, it should probably be normatively defined so engines behave consistently.

@phoddie
Copy link
Author

phoddie commented Jun 27, 2023

@syg – With this proposal up for Stage 4 next month, is there a resolution to this? (Apologies if there was a change that I missed)

@syg
Copy link
Collaborator

syg commented Jun 27, 2023

Oh man I missed this issue, thanks for the ping. My preferred fix is to transpose steps 4 and 5 in the resize algorithm. I'll tag on a quick normative item to the agenda. If there's discussion I'll defer the Stage 4 ask. @phoddie WDYT about transposition instead of repeating the check?

Edit: The transposition fix would be in line with what we did in #99

@syg
Copy link
Collaborator

syg commented Jun 27, 2023

Chrome already has the transposed semantics. At a quick glance I can't quite tell what Safari would do.

@phoddie
Copy link
Author

phoddie commented Jun 27, 2023

@syg – I agree that performing the detached buffer check only once is best. And it should absolutely be normatively defined. My preference would be pull ToIntegerOrInfinity all the way back to step 2, so any changes a script might make would be before validating the ArrayBuffer. Practically speaking it may not matter now, but perhaps in the future.

FWIW – Step 22 of ArrayBuffer.prototype.slice helpfully notes that preceding steps may have caused chaos. That's nice for implementors. Perhaps appropriate here, depending on the change.

@syg
Copy link
Collaborator

syg commented Jun 28, 2023

Putting it as Step 2 is an option, though slightly inconsistent with previous choice in #99, which I think is characterized by "mostly do coercions and checks in receiver, then left-to-right parameter order, but the detach check can be delayed to avoid rechecking". Whereas putting the ToIntegerOrInfinity to Step 2 would be more characterized as "do coercions and checks of all the parameters that might cause detachment, then do receiver checks". Both are valid choices and I think effect on ergonomics is negligible. And given the choice we made in #99, I prefer to just stick with it.

@syg
Copy link
Collaborator

syg commented Jun 28, 2023

@phoddie I've added this item to the agenda for July.

@phoddie
Copy link
Author

phoddie commented Jun 28, 2023

@syg – I don't have a very strong opinion on this, so let's see what comes out of plenary.

From an implementer perspective, safely checking the arguments can be challenging to get right, so I'm happy if the spec provides safe guidance. The order of checks seems unimportant. How many developers have an intuition about that and would then depend on it? As long as engines implement the spec steps consistently, it should be enough.

@phoddie phoddie closed this as completed Jun 28, 2023
@syg
Copy link
Collaborator

syg commented Jun 28, 2023

How many developers have an intuition about that and would then depend on it? As long as engines implement the spec steps consistently, it should be enough.

I don't disagree! There are other delegates who in the past have expressed strong opinions that argument checking order ought to be preserved as much as possible though.

@bakkot
Copy link
Contributor

bakkot commented Jun 28, 2023

As a data point, I am a delegate who likes to preserve argument checking order, but if there's implementation reasons to do things in a different order I am OK with violating the usual order.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants