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

URLSearchParams constructor interprets an array as a record #24406

Closed
saschanaz opened this issue Oct 9, 2019 · 5 comments
Closed

URLSearchParams constructor interprets an array as a record #24406

saschanaz opened this issue Oct 9, 2019 · 5 comments

Comments

@saschanaz
Copy link
Contributor

@saschanaz saschanaz commented Oct 9, 2019

const params = new URLSearchParams(["ab"]);
document.write(params);

Expected: An error, because the constructor requires sequence<sequence<USVString>>.
Actual: Instead it interprets as a record, so the result is 0=ab&length=1. The spec says sequences and records are distinguishable, so this shouldn't happen.

See also #19776 which is also a distinguishablity problem.

@saschanaz
Copy link
Contributor Author

@saschanaz saschanaz commented Oct 15, 2019

@highfive assign me

BTW, is it expected that the bot just adds a label without actually adding me as an assignee?

@highfive
Copy link

@highfive highfive commented Oct 15, 2019

Hey @saschanaz! Thanks for your interest in working on this issue. It's now assigned to you!

@highfive highfive added the C-assigned label Oct 15, 2019
@CYBAI
Copy link
Collaborator

@CYBAI CYBAI commented Oct 15, 2019

BTW, is it expected that the bot just adds a label without actually adding me as an assignee?

Yes, highfive bot will only add the C-assigned label because GitHub didn't support assigning someone out of the organization when the bot was created.

@CYBAI
Copy link
Collaborator

@CYBAI CYBAI commented Oct 15, 2019

Btw, maybe it's good to add the testcase you gave into this one 👀?

test(function() {
var params = new URLSearchParams([]);
assert_true(params != null, 'constructor returned non-null value.');
params = new URLSearchParams([['a', 'b'], ['c', 'd']]);
assert_equals(params.get("a"), "b");
assert_equals(params.get("c"), "d");
assert_throws(new TypeError(), function() { new URLSearchParams([[1]]); });
assert_throws(new TypeError(), function() { new URLSearchParams([[1,2,3]]); });
}, "Constructor with sequence of sequences of strings");

@saschanaz
Copy link
Contributor Author

@saschanaz saschanaz commented Oct 15, 2019

This is a general binding problem so it's possible that there already is a failing test elsewhere. I might check after implementing a fix.

bors-servo added a commit that referenced this issue Oct 16, 2019
[WIP] Match sequence only when the value is iterable

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #24406

<!-- Either: -->
- [ ] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit to servo/rust-mozjs that referenced this issue Oct 16, 2019
Throw when failing to convert sequence inner values

Returning `Ok(ConversionResult::Failure)` allows the code to try other conversion in a union type, but [per the spec there is no such failover](https://heycam.github.io/webidl/#es-union) given that the type is iterable.

For servo/servo#24406.
bors-servo added a commit that referenced this issue Oct 17, 2019
Throw when failing to convert sequence inner values

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24406

<!-- Either: -->
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Oct 17, 2019
Throw when failing to convert sequence inner values

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24406

<!-- Either: -->
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
@jdm @saschanaz @highfive @CYBAI and others
You can’t perform that action at this time.