Skip to content

Commit 0d63acb

Browse files
authored
reproduce and fix urlpattern bug(s) (#992)
1 parent 8775060 commit 0d63acb

File tree

3 files changed

+20
-3
lines changed

3 files changed

+20
-3
lines changed

src/url_pattern.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,11 @@ tl::expected<std::string, errors> url_pattern_init::process_search(
341341
if (value.starts_with("?")) {
342342
value.remove_prefix(1);
343343
}
344-
ADA_ASSERT_TRUE(!value.starts_with("?"));
344+
// We cannot assert that the value is no longer starting with a single
345+
// question mark because technically it can start. The question is whether or
346+
// not we should remove the first question mark. Ref:
347+
// https://github.com/ada-url/ada/pull/992 The spec is not clear on this.
348+
345349
// If type is "pattern" then return strippedValue.
346350
if (type == process_type::pattern) {
347351
return std::string(value);

src/url_pattern_helpers.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,10 @@ tl::expected<std::string, errors> canonicalize_search(std::string_view input) {
410410
url->set_search(input);
411411
if (url->has_search()) {
412412
const auto search = url->get_search();
413-
return std::string(search.substr(1));
413+
if (!search.empty()) {
414+
return std::string(search.substr(1));
415+
}
416+
return "";
414417
}
415418
return tl::unexpected(errors::type_error);
416419
}
@@ -430,7 +433,10 @@ tl::expected<std::string, errors> canonicalize_hash(std::string_view input) {
430433
// Return dummyURL's fragment.
431434
if (url->has_hash()) {
432435
const auto hash = url->get_hash();
433-
return std::string(hash.substr(1));
436+
if (!hash.empty()) {
437+
return std::string(hash.substr(1));
438+
}
439+
return "";
434440
}
435441
return tl::unexpected(errors::type_error);
436442
}

tests/basic_tests.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,7 @@ TYPED_TEST(basic_tests, test_issue_970) {
524524
SUCCEED();
525525
}
526526

527+
// Ref: https://github.com/cloudflare/workerd/issues/5144
527528
TYPED_TEST(basic_tests, test_workerd_issue_5144_1) {
528529
auto url = ada::parse<TypeParam>("https://example.sub.com/??");
529530
ASSERT_TRUE(url);
@@ -533,6 +534,7 @@ TYPED_TEST(basic_tests, test_workerd_issue_5144_1) {
533534
SUCCEED();
534535
}
535536

537+
// Ref: https://github.com/cloudflare/workerd/issues/5144
536538
TYPED_TEST(basic_tests, test_workerd_issue_5144_2) {
537539
auto url = ada::parse<TypeParam>("https://example.sub.com/???");
538540
ASSERT_TRUE(url);
@@ -541,6 +543,7 @@ TYPED_TEST(basic_tests, test_workerd_issue_5144_2) {
541543
SUCCEED();
542544
}
543545

546+
// Ref: https://github.com/cloudflare/workerd/issues/5144
544547
TYPED_TEST(basic_tests, test_workerd_issue_5144_3) {
545548
auto url = ada::parse<TypeParam>("https://example.sub.com/????");
546549
ASSERT_TRUE(url);
@@ -549,6 +552,7 @@ TYPED_TEST(basic_tests, test_workerd_issue_5144_3) {
549552
SUCCEED();
550553
}
551554

555+
// Ref: https://github.com/cloudflare/workerd/issues/5144
552556
TYPED_TEST(basic_tests, test_workerd_issue_5144_4) {
553557
using regex_provider = ada::url_pattern_regex::std_regex_provider;
554558
auto init = ada::url_pattern_init{};
@@ -559,5 +563,8 @@ TYPED_TEST(basic_tests, test_workerd_issue_5144_4) {
559563
ASSERT_TRUE(pattern->match("https://example.com/?"));
560564
ASSERT_TRUE(pattern->match("https://example.com/??"));
561565

566+
auto dummy_init = ada::url_pattern_init{};
567+
dummy_init.search = "???";
568+
ASSERT_TRUE(pattern->exec(std::move(dummy_init)));
562569
SUCCEED();
563570
}

0 commit comments

Comments
 (0)