-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Comments
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. |
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 |
@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. |
The compat issue: karma-runner/karma#1768 @leobalter Would you be interested in writing up the spec fix that @allenwb is suggesting? |
Sure, I'll check it tomorrow. |
I believe this is fixed after #410: |
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
@allenwbThe text was updated successfully, but these errors were encountered: