[duplicate] url: fix URLPattern.exec() capture group property insertion order#64765
Closed
kairosci wants to merge 1 commit into
Closed
[duplicate] url: fix URLPattern.exec() capture group property insertion order#64765kairosci wants to merge 1 commit into
kairosci wants to merge 1 commit into
Conversation
Author
|
Closing in favour of #64735 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
URLPattern.exec()builds thegroupsobject for each component result, it iterated overurl_pattern_component_result::groups, which is typed asstd::unordered_map<std::string, std::optional<std::string>>. Becausestd::unordered_mapprovides no guaranteed iteration order, the property insertion order on the resulting JavaScript object was arbitrary and non-deterministic across runs and platforms — contradicting the WebIDL specification requirement that named capture groups appear in the order they were declared in the pattern string.The fix threads the
group_name_listfield from the correspondingada::url_pattern_componentthrough to the serialization layer. That field is astd::vector<std::string>whose entries are populated in declaration order by the ada library when the pattern is compiled. TheURLPatternComponentResult::ToJSObjectfunction now accepts this vector and drives the loop over capture groups using it, looking up each value by name from the unordered map. TheURLPatternResult::ToJSValuefunction receives theada::url_patternreference (which owns the components with their ordered name lists) alongside the match result, and passes the correctgroup_name_listfor each URL component toToJSObject. The instance-levelExecmethod already owns theurl_pattern_member and passes it toToJSValue.A regression test in
test/parallel/test-urlpattern-exec-groups-order.jscovers four scenarios: a three-group pathname where names are in alphabetical order, a three-group pathname where names are in reverse-alphabetical order (to distinguish declaration order from sorted order), a two-component pattern where hostname and pathname each carry independent groups, and a pattern with no named groups to confirm that the empty case is unaffected.Closes #64734