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.

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

TypedArray ArrayBuffer constructor allowing negative length? #468

Closed
littledan opened this issue Mar 10, 2016 · 6 comments
Closed

TypedArray ArrayBuffer constructor allowing negative length? #468

littledan opened this issue Mar 10, 2016 · 6 comments

Comments

@littledan
Copy link
Member

Was this intended? In https://tc39.github.io/ecma262/#sec-typedarray-buffer-byteoffset-length , a number of the arguments are checked to be greater than or equal to 0, but the length simply has ToLength applied to it, without this check. V8 currently checks that the argument is >= 0. Unlike other places where the ES2017 draft specification asks us to be more strict than we currently are, this asks us to be more lenient. I'd be fine to implement it, but before weakening the restrictions and releasing this to the web, I have to ask, was this intentional? When it is web-compatible, I like the idea of making/keeping interfaces strict in their input validation. The negative length is checked to be possible by the test262 test built-ins/TypedArrays/buffer-arg-defined-negative-length @allenwb

@domenic
Copy link
Member

domenic commented Mar 10, 2016

I don't have any specific input on this issue but I wanted to share a strategy that we often use for similar things to the typed array situation, where we're investigating how well a new-ish spec matches a long legacy of not-so-robust implementations. What we do is construct a quick test case in jsbin or live dom viewer, then create a Markdown table and fill in what we can with the test results and ask others to fill on cells for browsers we don't have on our computer (like Safari or Edge).

You can see some examples at whatwg/html#823 (comment) and whatwg/html#775 (comment).

This might be helpful for finding out if V8's check is a minority among browsers and the spec matches the others, or if all browsers agree and the spec mismatches reality, or what.

@allenwb
Copy link
Member

allenwb commented Mar 10, 2016

This appears to be a bug, that took a couple to steps to introduce.

Prior to ES6 Rev17 ToNumber rather than ToLength was applied to the length argument and a RangeError was thrown if the result number was < 0. See 15.13.6.1.4 of Rev 17.

Bug 2171 pointed out that the test was unnecessary because ToLength never returns a value < 0. So the check was deleted in Rev 21 22.2.1.4

The original bug was the simple replacement of the ToNumber with the ToLength. The length argument should probably be handled similarly to what is currently done in 22.2.4.2

@littledan
Copy link
Member Author

@allenwb Based on the rest of the TypedArray specification, that sounds entirely reasonable. However, I'm still not sure about the web compatibility of the general strategy of throwing an exception on non-integer arguments to various TypedArray/DataView/ArrayBuffer-related functions, which is under discussion at #265 . (I believe @zloirock has found some compatibility issues when he tried to ship these semantics in core.js.) For now, I'll hold off on allowing negative lengths in V8, and I'd be in favor of the spec change that @allenwb is suggesting.

@littledan
Copy link
Member Author

The compat issue: karma-runner/karma#1768

@leobalter Would you be interested in writing up the spec fix that @allenwb is suggesting?

@leobalter
Copy link
Member

Sure, I'll check it tomorrow.

@bakkot
Copy link
Contributor

bakkot commented Aug 17, 2016

I believe this is fixed after #410: length is now passed through ToIndex, which throws on negative inputs.

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

No branches or pull requests

6 participants