Skip to content

Commit a7ee0c2

Browse files
committed
fix(urlpattern): match capture groups in test()/exec()
v8_regex_provider::regex_search had two bugs that made URLPattern matching fail (or crash) for any pattern with capture groups: - The ToLocal(&item) success check was inverted, so the first successfully read match element bailed out as "no match" and test()/exec() always returned false/null. - It included match element 0 (the whole-match string). ada's create_component_match_result pairs exec_result[i] with group_name_list[i] and expects only the capture groups, so the extra element overran group_name_list and crashed once the inverted check was fixed. Iterate from index 1 and bail only when reading an element fails. Add URLPattern regression tests covering test() match/non-match and exec() named capture-group extraction and null-on-no-match.
1 parent 268178c commit a7ee0c2

2 files changed

Lines changed: 31 additions & 2 deletions

File tree

NativeScript/runtime/URLPatternImpl.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,19 @@ v8_regex_provider::regex_search(
7373
auto array = matches.As<v8::Array>();
7474
auto len = array->Length();
7575
ret.reserve(len);
76-
for (int i = 0; i < len; i++) {
76+
// Skip element 0 (the whole-match string). ada's
77+
// url_pattern_component::create_component_match_result pairs exec_result[i]
78+
// with group_name_list[i], i.e. it expects only the capture groups.
79+
// Including element 0 makes exec_result one longer than group_name_list and
80+
// indexes it out of bounds (crash). ada documents this: the provider must
81+
// drop the full match ("start from 1").
82+
for (int i = 1; i < len; i++) {
7783
v8::Local<v8::Value> item;
78-
if (array->Get(isolate->GetCurrentContext(), i).ToLocal(&item)) {
84+
// ToLocal returns true on success; bail only when reading the element
85+
// fails. (This condition was inverted, which made every regex match
86+
// report no match, so URLPattern test()/exec() always failed for patterns
87+
// with capture groups.)
88+
if (!array->Get(isolate->GetCurrentContext(), i).ToLocal(&item)) {
7989
return std::nullopt;
8090
}
8191

TestRunner/app/tests/URLPattern.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,23 @@ describe("URLPattern", function () {
4646
expect(pattern.hostname).toBe("google.com");
4747
});
4848

49+
it("test() matches a URL against a pattern with a capture group", function () {
50+
const pattern = new URLPattern("https://example.com/books/:id");
51+
expect(pattern.test("https://example.com/books/123")).toBe(true);
52+
expect(pattern.test("https://example.com/movies/123")).toBe(false);
53+
});
54+
55+
it("exec() extracts named capture groups", function () {
56+
const pattern = new URLPattern("https://example.com/books/:id");
57+
const result = pattern.exec("https://example.com/books/123");
58+
expect(result).not.toBeNull();
59+
expect(result.pathname.input).toBe("/books/123");
60+
expect(result.pathname.groups.id).toBe("123");
61+
});
62+
63+
it("exec() returns null when the URL does not match", function () {
64+
const pattern = new URLPattern("https://example.com/books/:id");
65+
expect(pattern.exec("https://example.com/movies/123")).toBeNull();
66+
});
67+
4968
});

0 commit comments

Comments
 (0)