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

"array index property name" doesn't guard against failure #346

Closed
annevk opened this issue Apr 18, 2017 · 6 comments · Fixed by #386
Closed

"array index property name" doesn't guard against failure #346

annevk opened this issue Apr 18, 2017 · 6 comments · Fixed by #386

Comments

@annevk
Copy link
Member

annevk commented Apr 18, 2017

In particular, ToUint32 can throw and the various callers we have of this check don't expect that. (At least not those in HTML.)

@bzbarsky
Copy link
Collaborator

I must be missing something.

https://heycam.github.io/webidl/#dfn-array-index-property-name says:

which is a property name P such that Type(P) is String and for which the following algorithm returns true

So we land in https://tc39.github.io/ecma262/#sec-touint32 which can only throw via ToNumber, and our argument is guaranteed to be a String. https://tc39.github.io/ecma262/#sec-tonumber in the string case says:

ToNumber applied to Strings applies the following grammar to the input String interpreted as a sequence of UTF-16 encoded code points (6.1.4). If the grammar cannot interpret the String as an expansion of StringNumericLiteral, then the result of ToNumber is NaN.

so how can this throw?

@annevk
Copy link
Member Author

annevk commented Apr 18, 2017

Is the Type(P) is String an assertion that the input must meet or a check on input that makes it return false if you pass a Symbol?

@bzbarsky
Copy link
Collaborator

The latter. Any property name for which Type(P) is not string or the algorithm returns false is not an array index property name.

If there's a clearer way of phrasing this, happy to do that.

@annevk
Copy link
Member Author

annevk commented Apr 18, 2017

Making the type check the first step of the algorithm and confidently asserting that ToUint32 and ToString cannot throw with the ! convention from JavaScript would do the trick.

@annevk
Copy link
Member Author

annevk commented Apr 18, 2017

I tried to find out if we could reuse a definition from JavaScript directly, but https://tc39.github.io/ecma262/#sec-array-exotic-objects seems to have the bug that IDL does not and it's defined somewhat informatively.

@annevk
Copy link
Member Author

annevk commented Apr 18, 2017

Filed tc39/ecma262#900.

tobie added a commit to tobie/webidl that referenced this issue Jul 27, 2017
Make it explicit that the algorithm only accepts strings as input and does not throw.

Fixes whatwg#346.
tobie added a commit that referenced this issue Jul 28, 2017
Make it explicit that the algorithm only accepts strings as input and does not throw.

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

Successfully merging a pull request may close this issue.

2 participants