From 845d1a61a3223d94f54c3e5687ecd8b6c1f5c17a Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Sat, 30 May 2026 17:32:28 -0400 Subject: [PATCH 1/4] Correctly handle trailing slashes in `URITemplateRouter` Signed-off-by: Juan Cruz Viotti --- src/core/uritemplate/uritemplate_router.cc | 12 +- .../uritemplate/uritemplate_router_view.cc | 17 +- test/uritemplate/uritemplate_router_test.cc | 190 ++++++++++++++++++ .../uritemplate_router_view_test.cc | 145 +++++++++++++ 4 files changed, 362 insertions(+), 2 deletions(-) diff --git a/src/core/uritemplate/uritemplate_router.cc b/src/core/uritemplate/uritemplate_router.cc index a1e61c94f1..cffca1e1be 100644 --- a/src/core/uritemplate/uritemplate_router.cc +++ b/src/core/uritemplate/uritemplate_router.cc @@ -491,6 +491,11 @@ auto URITemplateRouter::add(const std::string_view uri_template, current = &find_or_create_literal_child(literals, ""); } + if (!absorbed && current != nullptr && current != base_path_end && + uri_template.size() > 1 && uri_template.back() == '/') { + current = &find_or_create_literal_child(current->literals, ""); + } + if (!absorbed && current != nullptr) { const auto previous_identifier = current->identifier; if (previous_identifier == 0) { @@ -586,8 +591,13 @@ auto URITemplateRouter::match(const std::string_view path, const std::string_view segment{ segment_start, static_cast(position - segment_start)}; - // Empty segment (from double slash or trailing slash) doesn't match if (segment.empty()) { + if (position >= path_end) { + if (auto *empty_child = find_literal_child(*literal_children, "")) { + return finalize_match(this->otherwise_, empty_child->identifier, + empty_child->context); + } + } return finalize_match(this->otherwise_, 0, 0); } diff --git a/src/core/uritemplate/uritemplate_router_view.cc b/src/core/uritemplate/uritemplate_router_view.cc index 9025f7331c..58e29e4518 100644 --- a/src/core/uritemplate/uritemplate_router_view.cc +++ b/src/core/uritemplate/uritemplate_router_view.cc @@ -548,8 +548,23 @@ auto URITemplateRouterView::match( const auto segment_length = static_cast(position - segment_start); - // Empty segment (from double slash or trailing slash) doesn't match if (segment_length == 0) { + if (position >= path_end) { + const auto &node = nodes[current_node]; + if (node.first_literal_child != NO_CHILD && + node.first_literal_child < header->node_count && + node.literal_child_count <= + header->node_count - node.first_literal_child) { + const auto empty_match = binary_search_literal_children( + nodes, string_table, string_table_size, node.first_literal_child, + node.literal_child_count, "", 0); + if (empty_match != NO_CHILD) { + return finalize_match(otherwise_context, + nodes[empty_match].identifier, + nodes[empty_match].context); + } + } + } return finalize_match(otherwise_context, 0, 0); } diff --git a/test/uritemplate/uritemplate_router_test.cc b/test/uritemplate/uritemplate_router_test.cc index aa239c4c4f..1d6d02a782 100644 --- a/test/uritemplate/uritemplate_router_test.cc +++ b/test/uritemplate/uritemplate_router_test.cc @@ -2531,3 +2531,193 @@ TEST(URITemplateRouter, operation_id_after_overwrite) { EXPECT_TRUE(router.operation_id(1).empty()); EXPECT_EQ(router.operation_id(2), "list_users_v2"); } + +TEST(URITemplateRouter, trailing_slash_distinct_from_bare_match_bare) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_700", 1); + EXPECT_ROUTER_MATCH(router, "/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_distinct_from_bare_no_match_slashed) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_701", 1); + EXPECT_ROUTER_MATCH(router, "/foo/", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_only_registration_matches_slashed) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/", "op_702", 1); + EXPECT_ROUTER_MATCH(router, "/foo/", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_only_registration_no_match_bare) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/", "op_703", 1); + EXPECT_ROUTER_MATCH(router, "/foo", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_both_forms_registered_match_bare) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_704", 1); + router.add("/foo/", "op_705", 2); + EXPECT_ROUTER_MATCH(router, "/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_both_forms_registered_match_slashed) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_706", 1); + router.add("/foo/", "op_707", 2); + EXPECT_ROUTER_MATCH(router, "/foo/", 2, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_both_forms_registered_size_is_two) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_708", 1); + router.add("/foo/", "op_709", 2); + EXPECT_EQ(router.size(), 2); +} + +TEST(URITemplateRouter, + trailing_slash_both_forms_registered_lookup_by_operation_id) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_710", 1); + router.add("/foo/", "op_711", 2); + const auto bare = router.operation("op_710"); + const auto slashed = router.operation("op_711"); + EXPECT_EQ(bare.first, 1); + EXPECT_EQ(slashed.first, 2); +} + +TEST(URITemplateRouter, + trailing_slash_both_forms_registered_path_reconstruction) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_712", 1); + router.add("/foo/", "op_713", 2); + EXPECT_EQ(router.path(1), "/foo"); + EXPECT_EQ(router.path(2), "/foo/"); +} + +TEST(URITemplateRouter, trailing_slash_multi_segment_match_slashed) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/bar/", "op_714", 1); + EXPECT_ROUTER_MATCH(router, "/foo/bar/", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_multi_segment_no_match_bare) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/bar/", "op_715", 1); + EXPECT_ROUTER_MATCH(router, "/foo/bar", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_after_variable_match_slashed) { + sourcemeta::core::URITemplateRouter router; + router.add("/users/{id}/", "op_716", 1); + EXPECT_ROUTER_MATCH(router, "/users/42/", 1, 0, captures); + EXPECT_EQ(captures.size(), 1); + EXPECT_ROUTER_CAPTURE(captures, 0, "id", "42"); +} + +TEST(URITemplateRouter, trailing_slash_after_variable_no_match_bare) { + sourcemeta::core::URITemplateRouter router; + router.add("/users/{id}/", "op_717", 1); + EXPECT_ROUTER_MATCH(router, "/users/42", 0, 0, captures); +} + +TEST(URITemplateRouter, trailing_slash_both_forms_with_variable) { + sourcemeta::core::URITemplateRouter router; + router.add("/users/{id}", "op_718", 1); + router.add("/users/{id}/", "op_719", 2); + EXPECT_ROUTER_MATCH(router, "/users/42", 1, 0, captures_bare); + EXPECT_EQ(captures_bare.size(), 1); + EXPECT_ROUTER_CAPTURE(captures_bare, 0, "id", "42"); + EXPECT_ROUTER_MATCH(router, "/users/42/", 2, 0, captures_slashed); + EXPECT_EQ(captures_slashed.size(), 1); + EXPECT_ROUTER_CAPTURE(captures_slashed, 0, "id", "42"); +} + +TEST(URITemplateRouter, trailing_slash_does_not_relax_internal_double_slash) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/bar", "op_720", 1); + EXPECT_ROUTER_MATCH(router, "/foo//bar", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_with_base_path_match_slashed) { + sourcemeta::core::URITemplateRouter router{"/api"}; + router.add("/foo/", "op_721", 1); + EXPECT_ROUTER_MATCH(router, "/api/foo/", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_with_base_path_no_match_bare) { + sourcemeta::core::URITemplateRouter router{"/api"}; + router.add("/foo/", "op_722", 1); + EXPECT_ROUTER_MATCH(router, "/api/foo", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_with_base_path_both_forms) { + sourcemeta::core::URITemplateRouter router{"/api"}; + router.add("/foo", "op_723", 1); + router.add("/foo/", "op_724", 2); + EXPECT_ROUTER_MATCH(router, "/api/foo", 1, 0, captures_bare); + EXPECT_EQ(captures_bare.size(), 0); + EXPECT_ROUTER_MATCH(router, "/api/foo/", 2, 0, captures_slashed); + EXPECT_EQ(captures_slashed.size(), 0); +} + +TEST(URITemplateRouter, + trailing_slash_multiple_trailing_slashes_in_template_collapse_to_one) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo///", "op_725", 1); + EXPECT_ROUTER_MATCH(router, "/foo/", 1, 0, captures_one); + EXPECT_EQ(captures_one.size(), 0); + EXPECT_ROUTER_MATCH(router, "/foo", 0, 0, captures_bare); + EXPECT_EQ(captures_bare.size(), 0); +} + +TEST(URITemplateRouter, + trailing_slash_multiple_trailing_slashes_in_request_no_match) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/", "op_726", 1); + EXPECT_ROUTER_MATCH(router, "/foo//", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, trailing_slash_both_forms_each_has_distinct_arguments) { + sourcemeta::core::URITemplateRouter router; + const std::array bare_args{ + sourcemeta::core::URITemplateRouter::Argument{ + "kind", sourcemeta::core::URITemplateRouter::ArgumentValue{ + std::string_view{"bare"}}}}; + const std::array + slashed_args{sourcemeta::core::URITemplateRouter::Argument{ + "kind", sourcemeta::core::URITemplateRouter::ArgumentValue{ + std::string_view{"slashed"}}}}; + router.add("/foo", "op_727", 1, 0, bare_args); + router.add("/foo/", "op_728", 2, 0, slashed_args); + std::string bare_kind; + router.arguments( + 1, [&bare_kind](const std::string_view name, const auto &value) { + if (name == "kind" && std::holds_alternative(value)) { + bare_kind = std::get(value); + } + }); + std::string slashed_kind; + router.arguments( + 2, [&slashed_kind](const std::string_view name, const auto &value) { + if (name == "kind" && std::holds_alternative(value)) { + slashed_kind = std::get(value); + } + }); + EXPECT_EQ(bare_kind, "bare"); + EXPECT_EQ(slashed_kind, "slashed"); +} diff --git a/test/uritemplate/uritemplate_router_view_test.cc b/test/uritemplate/uritemplate_router_view_test.cc index 66560992d9..bc9a734dc5 100644 --- a/test/uritemplate/uritemplate_router_view_test.cc +++ b/test/uritemplate/uritemplate_router_view_test.cc @@ -3479,3 +3479,148 @@ TEST_F(URITemplateRouterViewTest, listing_path_excludes_base_path) { const sourcemeta::core::URITemplateRouterView restored{this->path}; EXPECT_EQ(restored.path(1), "/users/{id}"); } + +TEST_F(URITemplateRouterViewTest, + trailing_slash_distinct_from_bare_match_bare) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_900", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, + trailing_slash_distinct_from_bare_no_match_slashed) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_901", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo/", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, + trailing_slash_only_registration_matches_slashed) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/", "op_902", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo/", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, + trailing_slash_only_registration_no_match_bare) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/", "op_903", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, + trailing_slash_both_forms_registered_match_bare) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_904", 1); + router.add("/foo/", "op_905", 2); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, + trailing_slash_both_forms_registered_match_slashed) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_906", 1); + router.add("/foo/", "op_907", 2); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo/", 2, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, trailing_slash_after_variable_match_slashed) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/users/{id}/", "op_908", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/users/42/", 1, 0, captures); + EXPECT_EQ(captures.size(), 1); + EXPECT_ROUTER_CAPTURE(captures, 0, "id", "42"); +} + +TEST_F(URITemplateRouterViewTest, trailing_slash_after_variable_no_match_bare) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/users/{id}/", "op_909", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/users/42", 0, 0, captures); +} + +TEST_F(URITemplateRouterViewTest, + trailing_slash_does_not_relax_internal_double_slash) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo/bar", "op_910", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo//bar", 0, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, trailing_slash_with_base_path_both_forms) { + { + sourcemeta::core::URITemplateRouter router{"/api"}; + router.add("/foo", "op_911", 1); + router.add("/foo/", "op_912", 2); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/api/foo", 1, 0, captures_bare); + EXPECT_EQ(captures_bare.size(), 0); + EXPECT_ROUTER_MATCH(restored, "/api/foo/", 2, 0, captures_slashed); + EXPECT_EQ(captures_slashed.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, trailing_slash_path_reconstruction) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_913", 1); + router.add("/foo/", "op_914", 2); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_EQ(restored.path(1), "/foo"); + EXPECT_EQ(restored.path(2), "/foo/"); +} + +TEST_F(URITemplateRouterViewTest, trailing_slash_size_is_two) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo", "op_915", 1); + router.add("/foo/", "op_916", 2); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_EQ(restored.size(), 2); +} From 0b44ab970fd82cb38d3a8300cc85a8ea480df8d9 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Sat, 30 May 2026 19:21:34 -0400 Subject: [PATCH 2/4] More Signed-off-by: Juan Cruz Viotti --- src/core/uritemplate/uritemplate_router.cc | 178 ++++++++---------- .../uritemplate/uritemplate_router_view.cc | 90 ++------- test/uritemplate/uritemplate_router_test.cc | 176 ++++++++++++----- .../uritemplate_router_view_test.cc | 112 +++++++---- 4 files changed, 294 insertions(+), 262 deletions(-) diff --git a/src/core/uritemplate/uritemplate_router.cc b/src/core/uritemplate/uritemplate_router.cc index cffca1e1be..a8f60cde88 100644 --- a/src/core/uritemplate/uritemplate_router.cc +++ b/src/core/uritemplate/uritemplate_router.cc @@ -124,19 +124,6 @@ URITemplateRouter::URITemplateRouter(const std::string_view base_path, const std::string_view base_url) : base_path_{base_path}, base_url_{base_url} { assert(this->base_path_.empty() || this->base_path_.front() == '/'); - const auto base_path_last = this->base_path_.find_last_not_of('/'); - if (base_path_last == std::string::npos) { - this->base_path_.clear(); - } else { - this->base_path_.erase(base_path_last + 1); - } - - const auto base_url_last = this->base_url_.find_last_not_of('/'); - if (base_url_last == std::string::npos) { - this->base_url_.clear(); - } else { - this->base_url_.erase(base_url_last + 1); - } } auto URITemplateRouter::base_path() const noexcept -> std::string_view { @@ -241,18 +228,19 @@ auto URITemplateRouter::add(const std::string_view uri_template, throw URITemplateRouterDuplicateOperationIdError{operation_id}; } - // Walk base path segments to establish the trie prefix + if (!uri_template.empty() && uri_template.front() != '/' && + !(uri_template.size() >= 2 && uri_template[0] == '{' && + uri_template[1] == '/')) { + throw URITemplateRouterInvalidSegmentError{"Template must start with '/'", + uri_template}; + } + Node *current = nullptr; if (!this->base_path_.empty()) { - const char *base_position = this->base_path_.data(); - const char *const base_end = base_position + this->base_path_.size(); - while (base_position < base_end) { - while (base_position < base_end && *base_position == '/') { - ++base_position; - } - if (base_position >= base_end) { - break; - } + const char *base_position = this->base_path_.data() + 1; + const char *const base_end = + this->base_path_.data() + this->base_path_.size(); + while (true) { const char *segment_start = base_position; while (base_position < base_end && *base_position != '/') { ++base_position; @@ -262,6 +250,10 @@ auto URITemplateRouter::add(const std::string_view uri_template, static_cast(base_position - segment_start)}; auto &literals = current ? current->literals : this->root_.literals; current = &find_or_create_literal_child(literals, segment); + if (base_position >= base_end) { + break; + } + ++base_position; } } @@ -300,18 +292,23 @@ auto URITemplateRouter::add(const std::string_view uri_template, return; } - Node *base_path_end = current; bool absorbed = false; const char *position = uri_template.data(); const char *const end = position + uri_template.size(); - while (position < end && !absorbed) { - while (position < end && *position == '/') { - ++position; - } + if (position < end && *position == '/') { + ++position; + } - if (position >= end) { - break; + while (true) { + if (position >= end || *position == '/') { + auto &literals = current ? current->literals : this->root_.literals; + current = &find_or_create_literal_child(literals, ""); + if (position >= end) { + break; + } + ++position; + continue; } const char *segment_start = position; @@ -425,14 +422,15 @@ auto URITemplateRouter::add(const std::string_view uri_template, const std::string_view varname{ varname_start, static_cast(varname_end - varname_start)}; - ++position; // skip '}' + ++position; + + const bool followed_by_path_operator = + position + 1 < end && *position == '{' && *(position + 1) == '/'; - if (position < end && *position != '/') { - if (*position != '{' || position + 1 >= end || *(position + 1) != '/') { - throw URITemplateRouterInvalidSegmentError{ - "Path segment cannot mix literals and variables", - extract_segment(expression_start, end)}; - } + if (position < end && *position != '/' && !followed_by_path_operator) { + throw URITemplateRouterInvalidSegmentError{ + "Path segment cannot mix literals and variables", + extract_segment(expression_start, end)}; } if (is_expansion_type(type) && position < end) { @@ -448,52 +446,54 @@ auto URITemplateRouter::add(const std::string_view uri_template, } else { current = result; } - } else { - while (position < end && *position != '/' && *position != '{') { - if (*position == '}') { - throw URITemplateRouterInvalidSegmentError{ - "Unmatched closing brace", extract_segment(segment_start, end)}; - } - ++position; + + if (absorbed || position >= end) { + break; + } + if (followed_by_path_operator) { + continue; } + ++position; + continue; + } - if (position < end && *position == '{') { - if (position + 1 < end && *(position + 1) == '/') { - const std::string_view segment{ - segment_start, - static_cast(position - segment_start)}; - auto &literals = current ? current->literals : this->root_.literals; - current = &find_or_create_literal_child(literals, segment); - continue; - } - const char *expr_end = find_expression_end(position, end); - const char *seg_end = expr_end; - while (seg_end < end && *seg_end != '/') { - ++seg_end; - } + while (position < end && *position != '/' && *position != '{') { + if (*position == '}') { throw URITemplateRouterInvalidSegmentError{ - "Path segment cannot mix literals and variables", - std::string_view{segment_start, static_cast( - seg_end - segment_start)}}; + "Unmatched closing brace", extract_segment(segment_start, end)}; } + ++position; + } - const std::string_view segment{ - segment_start, static_cast(position - segment_start)}; - - auto &literals = current ? current->literals : this->root_.literals; - current = &find_or_create_literal_child(literals, segment); + if (position < end && *position == '{') { + if (position + 1 < end && *(position + 1) == '/') { + const std::string_view segment{ + segment_start, static_cast(position - segment_start)}; + auto &literals = current ? current->literals : this->root_.literals; + current = &find_or_create_literal_child(literals, segment); + continue; + } + const char *expr_end = find_expression_end(position, end); + const char *seg_end = expr_end; + while (seg_end < end && *seg_end != '/') { + ++seg_end; + } + throw URITemplateRouterInvalidSegmentError{ + "Path segment cannot mix literals and variables", + std::string_view{segment_start, + static_cast(seg_end - segment_start)}}; } - } - if (current == base_path_end && uri_template.size() == 1 && - uri_template[0] == '/') { + const std::string_view segment{ + segment_start, static_cast(position - segment_start)}; + auto &literals = current ? current->literals : this->root_.literals; - current = &find_or_create_literal_child(literals, ""); - } + current = &find_or_create_literal_child(literals, segment); - if (!absorbed && current != nullptr && current != base_path_end && - uri_template.size() > 1 && uri_template.back() == '/') { - current = &find_or_create_literal_child(current->literals, ""); + if (position >= end) { + break; + } + ++position; } if (!absorbed && current != nullptr) { @@ -560,17 +560,13 @@ auto URITemplateRouter::match(const std::string_view path, this->root_.context); } - if (path.size() == 1 && path[0] == '/') { - if (auto *child = find_literal_child(this->root_.literals, "")) { - return finalize_match(this->otherwise_, child->identifier, - child->context); - } + if (path.front() != '/') { return finalize_match(this->otherwise_, 0, 0); } const Node *current = nullptr; - const char *position = path.data(); - const char *const path_end = position + path.size(); + const char *position = path.data() + 1; + const char *const path_end = path.data() + path.size(); const std::vector> *literal_children = &this->root_.literals; @@ -578,11 +574,6 @@ auto URITemplateRouter::match(const std::string_view path, std::size_t variable_index = 0; - // Skip leading slash - if (position < path_end && *position == '/') { - ++position; - } - while (true) { const char *segment_start = position; while (position < path_end && *position != '/') { @@ -591,19 +582,9 @@ auto URITemplateRouter::match(const std::string_view path, const std::string_view segment{ segment_start, static_cast(position - segment_start)}; - if (segment.empty()) { - if (position >= path_end) { - if (auto *empty_child = find_literal_child(*literal_children, "")) { - return finalize_match(this->otherwise_, empty_child->identifier, - empty_child->context); - } - } - return finalize_match(this->otherwise_, 0, 0); - } - if (auto *literal_match = find_literal_child(*literal_children, segment)) { current = literal_match; - } else if (*variable_child) { + } else if (!segment.empty() && *variable_child) { assert(variable_index <= std::numeric_limits::max()); if (is_expansion_type((*variable_child)->type)) { @@ -625,12 +606,9 @@ auto URITemplateRouter::match(const std::string_view path, literal_children = ¤t->literals; variable_child = ¤t->variable; - // Check if there's more path if (position >= path_end) { break; } - - // Skip the slash and continue to next segment ++position; } diff --git a/src/core/uritemplate/uritemplate_router_view.cc b/src/core/uritemplate/uritemplate_router_view.cc index 58e29e4518..ad17300bc1 100644 --- a/src/core/uritemplate/uritemplate_router_view.cc +++ b/src/core/uritemplate/uritemplate_router_view.cc @@ -497,102 +497,46 @@ auto URITemplateRouterView::match( const auto string_table_size = header->arguments_offset - header->string_table_offset; - // Empty path matches empty template if (path.empty()) { return finalize_match(otherwise_context, nodes[0].identifier, nodes[0].context); } - // Root path "/" is stored as an empty literal segment - if (path.size() == 1 && path[0] == '/') { - const auto &root = nodes[0]; - if (root.first_literal_child == NO_CHILD) { - return finalize_match(otherwise_context, 0, 0); - } - - if (root.first_literal_child >= header->node_count || - root.literal_child_count > - header->node_count - root.first_literal_child) { - return finalize_match(otherwise_context, 0, 0); - } - - const auto match = binary_search_literal_children( - nodes, string_table, string_table_size, root.first_literal_child, - root.literal_child_count, "", 0); - if (match == NO_CHILD) { - return finalize_match(otherwise_context, 0, 0); - } - return finalize_match(otherwise_context, nodes[match].identifier, - nodes[match].context); + if (path.front() != '/') { + return finalize_match(otherwise_context, 0, 0); } - // Walk the trie, matching each path segment std::uint32_t current_node = 0; - const char *position = path.data(); - const char *const path_end = position + path.size(); + const char *position = path.data() + 1; + const char *const path_end = path.data() + path.size(); std::size_t variable_index = 0; - // Skip leading slash - if (position < path_end && *position == '/') { - ++position; - } - while (true) { - // Extract segment const char *segment_start = position; while (position < path_end && *position != '/') { ++position; } - const auto segment_length = static_cast(position - segment_start); - if (segment_length == 0) { - if (position >= path_end) { - const auto &node = nodes[current_node]; - if (node.first_literal_child != NO_CHILD && - node.first_literal_child < header->node_count && - node.literal_child_count <= - header->node_count - node.first_literal_child) { - const auto empty_match = binary_search_literal_children( - nodes, string_table, string_table_size, node.first_literal_child, - node.literal_child_count, "", 0); - if (empty_match != NO_CHILD) { - return finalize_match(otherwise_context, - nodes[empty_match].identifier, - nodes[empty_match].context); - } - } - } - return finalize_match(otherwise_context, 0, 0); - } - const auto &node = nodes[current_node]; const auto node_count = header->node_count; - // Try literal children first + std::uint32_t literal_match = NO_CHILD; if (node.first_literal_child != NO_CHILD) { if (node.first_literal_child >= node_count || node.literal_child_count > node_count - node.first_literal_child) { return finalize_match(otherwise_context, 0, 0); } - - const auto literal_match = binary_search_literal_children( + literal_match = binary_search_literal_children( nodes, string_table, string_table_size, node.first_literal_child, node.literal_child_count, segment_start, segment_length); - if (literal_match != NO_CHILD) { - current_node = literal_match; - if (position >= path_end) { - break; - } - ++position; - continue; - } } - // Fall back to variable child - if (node.variable_child != NO_CHILD) { + if (literal_match != NO_CHILD) { + current_node = literal_match; + } else if (segment_length > 0 && node.variable_child != NO_CHILD) { if (node.variable_child >= node_count || variable_index > std::numeric_limits::max()) { @@ -607,8 +551,6 @@ auto URITemplateRouterView::match( return finalize_match(otherwise_context, 0, 0); } - // Both Expansion and OptionalExpansion consume the rest of the path - // verbatim if (is_expansion_type(variable_node.type)) { const auto remaining_length = static_cast(path_end - segment_start); @@ -620,22 +562,20 @@ auto URITemplateRouterView::match( variable_node.context); } - // Regular variable - match single segment callback(static_cast(variable_index), {string_table + variable_node.string_offset, variable_node.string_length}, {segment_start, segment_length}); ++variable_index; current_node = node.variable_child; - if (position >= path_end) { - break; - } - ++position; - continue; + } else { + return finalize_match(otherwise_context, 0, 0); } - // No match - return finalize_match(otherwise_context, 0, 0); + if (position >= path_end) { + break; + } + ++position; } const auto &final_node = nodes[current_node]; diff --git a/test/uritemplate/uritemplate_router_test.cc b/test/uritemplate/uritemplate_router_test.cc index 1d6d02a782..c874cc8b7c 100644 --- a/test/uritemplate/uritemplate_router_test.cc +++ b/test/uritemplate/uritemplate_router_test.cc @@ -1118,30 +1118,6 @@ TEST(URITemplateRouter, base_path_with_empty_template) { EXPECT_EQ(captures.size(), 0); } -TEST(URITemplateRouter, base_path_slash_only_is_no_base_path) { - sourcemeta::core::URITemplateRouter router{"/"}; - EXPECT_TRUE(router.base_path().empty()); - router.add("/foo", "op_134", 1); - EXPECT_ROUTER_MATCH(router, "/foo", 1, 0, captures); - EXPECT_EQ(captures.size(), 0); -} - -TEST(URITemplateRouter, base_path_trailing_slash_normalized) { - sourcemeta::core::URITemplateRouter router{"/prefix/"}; - EXPECT_EQ(router.base_path(), "/prefix"); - router.add("/foo", "op_135", 1); - EXPECT_ROUTER_MATCH(router, "/prefix/foo", 1, 0, captures); - EXPECT_EQ(captures.size(), 0); -} - -TEST(URITemplateRouter, base_path_multiple_trailing_slashes_normalized) { - sourcemeta::core::URITemplateRouter router{"/prefix///"}; - EXPECT_EQ(router.base_path(), "/prefix"); - router.add("/foo", "op_136", 1); - EXPECT_ROUTER_MATCH(router, "/prefix/foo", 1, 0, captures); - EXPECT_EQ(captures.size(), 0); -} - TEST(URITemplateRouter, base_path_expansion) { sourcemeta::core::URITemplateRouter router{"/api"}; EXPECT_EQ(router.base_path(), "/api"); @@ -1205,21 +1181,6 @@ TEST(URITemplateRouter, base_url_set_without_base_path) { EXPECT_EQ(router.base_url(), "https://api.example.com"); } -TEST(URITemplateRouter, base_url_trailing_slash_normalized) { - sourcemeta::core::URITemplateRouter router{"", "https://api.example.com/"}; - EXPECT_EQ(router.base_url(), "https://api.example.com"); -} - -TEST(URITemplateRouter, base_url_multiple_trailing_slashes_normalized) { - sourcemeta::core::URITemplateRouter router{"", "https://api.example.com///"}; - EXPECT_EQ(router.base_url(), "https://api.example.com"); -} - -TEST(URITemplateRouter, base_url_only_slash_collapses_to_empty) { - sourcemeta::core::URITemplateRouter router{"", "/"}; - EXPECT_TRUE(router.base_url().empty()); -} - TEST(URITemplateRouter, base_url_not_used_for_matching) { sourcemeta::core::URITemplateRouter router{"/v1", "https://api.example.com"}; router.add("/foo", "op_base_url_match", 1); @@ -2674,16 +2635,6 @@ TEST(URITemplateRouter, trailing_slash_with_base_path_both_forms) { EXPECT_EQ(captures_slashed.size(), 0); } -TEST(URITemplateRouter, - trailing_slash_multiple_trailing_slashes_in_template_collapse_to_one) { - sourcemeta::core::URITemplateRouter router; - router.add("/foo///", "op_725", 1); - EXPECT_ROUTER_MATCH(router, "/foo/", 1, 0, captures_one); - EXPECT_EQ(captures_one.size(), 0); - EXPECT_ROUTER_MATCH(router, "/foo", 0, 0, captures_bare); - EXPECT_EQ(captures_bare.size(), 0); -} - TEST(URITemplateRouter, trailing_slash_multiple_trailing_slashes_in_request_no_match) { sourcemeta::core::URITemplateRouter router; @@ -2721,3 +2672,130 @@ TEST(URITemplateRouter, trailing_slash_both_forms_each_has_distinct_arguments) { EXPECT_EQ(bare_kind, "bare"); EXPECT_EQ(slashed_kind, "slashed"); } + +TEST(URITemplateRouter, + strict_internal_double_slash_registers_and_matches_only_itself) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo//bar", "op_800", 1); + EXPECT_ROUTER_MATCH(router, "/foo//bar", 1, 0, captures_verbatim); + EXPECT_EQ(captures_verbatim.size(), 0); + EXPECT_ROUTER_MATCH(router, "/foo/bar", 0, 0, captures_canonical); +} + +TEST(URITemplateRouter, + strict_only_slashes_template_registers_four_empty_segments) { + sourcemeta::core::URITemplateRouter router; + router.add("////", "op_801", 1); + EXPECT_EQ(router.size(), 1); + EXPECT_ROUTER_MATCH(router, "////", 1, 0, captures_match); + EXPECT_EQ(captures_match.size(), 0); + EXPECT_ROUTER_MATCH(router, "///", 0, 0, captures_short); + EXPECT_ROUTER_MATCH(router, "/////", 0, 0, captures_long); +} + +TEST(URITemplateRouter, + strict_three_slashes_template_registers_three_empty_segments) { + sourcemeta::core::URITemplateRouter router; + router.add("///", "op_802", 1); + EXPECT_ROUTER_MATCH(router, "///", 1, 0, captures_match); + EXPECT_EQ(captures_match.size(), 0); + EXPECT_ROUTER_MATCH(router, "//", 0, 0, captures_short); + EXPECT_ROUTER_MATCH(router, "////", 0, 0, captures_long); +} + +TEST(URITemplateRouter, + strict_double_slash_template_registers_two_empty_segments) { + sourcemeta::core::URITemplateRouter router; + router.add("//", "op_803", 1); + EXPECT_ROUTER_MATCH(router, "//", 1, 0, captures_match); + EXPECT_EQ(captures_match.size(), 0); + EXPECT_ROUTER_MATCH(router, "/", 0, 0, captures_short); +} + +TEST(URITemplateRouter, strict_internal_empty_with_variable_after) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo//{id}", "op_804", 1); + EXPECT_ROUTER_MATCH(router, "/foo//42", 1, 0, captures); + EXPECT_EQ(captures.size(), 1); + EXPECT_ROUTER_CAPTURE(captures, 0, "id", "42"); +} + +TEST(URITemplateRouter, strict_variable_does_not_bind_empty_segment) { + sourcemeta::core::URITemplateRouter router; + router.add("/users/{id}", "op_805", 1); + EXPECT_ROUTER_MATCH(router, "/users/", 0, 0, captures); +} + +TEST(URITemplateRouter, strict_missing_leading_slash_throws) { + sourcemeta::core::URITemplateRouter router; + EXPECT_THROW(router.add("foo", "op_806", 1), + sourcemeta::core::URITemplateRouterInvalidSegmentError); +} + +TEST(URITemplateRouter, strict_empty_template_still_special) { + sourcemeta::core::URITemplateRouter router; + router.add("", "op_807", 1); + EXPECT_ROUTER_MATCH(router, "", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, strict_base_path_preserves_trailing_slash) { + sourcemeta::core::URITemplateRouter router{"/api/"}; + EXPECT_EQ(router.base_path(), "/api/"); +} + +TEST(URITemplateRouter, + strict_base_path_with_trailing_slash_requires_empty_segment_in_request) { + sourcemeta::core::URITemplateRouter router{"/api/"}; + router.add("/foo", "op_808", 1); + EXPECT_ROUTER_MATCH(router, "/api//foo", 1, 0, captures_strict); + EXPECT_EQ(captures_strict.size(), 0); + EXPECT_ROUTER_MATCH(router, "/api/foo", 0, 0, captures_lenient); +} + +TEST(URITemplateRouter, + strict_base_path_without_trailing_slash_matches_canonical_request) { + sourcemeta::core::URITemplateRouter router{"/api"}; + router.add("/foo", "op_809", 1); + EXPECT_ROUTER_MATCH(router, "/api/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, strict_base_url_preserves_trailing_slash) { + sourcemeta::core::URITemplateRouter router{"", "https://api.example.com/"}; + EXPECT_EQ(router.base_url(), "https://api.example.com/"); +} + +TEST(URITemplateRouter, strict_base_url_preserves_multiple_trailing_slashes) { + sourcemeta::core::URITemplateRouter router{"", "https://api.example.com///"}; + EXPECT_EQ(router.base_url(), "https://api.example.com///"); +} + +TEST(URITemplateRouter, strict_internal_empty_then_literal) { + sourcemeta::core::URITemplateRouter router; + router.add("/a", "op_810", 1); + router.add("/a//b", "op_811", 2); + EXPECT_ROUTER_MATCH(router, "/a", 1, 0, captures_a); + EXPECT_EQ(captures_a.size(), 0); + EXPECT_ROUTER_MATCH(router, "/a//b", 2, 0, captures_aab); + EXPECT_EQ(captures_aab.size(), 0); + EXPECT_ROUTER_MATCH(router, "/a/b", 0, 0, captures_ab); + EXPECT_ROUTER_MATCH(router, "/a/", 0, 0, captures_a_slash); +} + +TEST(URITemplateRouter, strict_root_template_still_works) { + sourcemeta::core::URITemplateRouter router; + router.add("/", "op_812", 1); + EXPECT_ROUTER_MATCH(router, "/", 1, 0, captures_match); + EXPECT_EQ(captures_match.size(), 0); + EXPECT_ROUTER_MATCH(router, "", 0, 0, captures_empty); + EXPECT_ROUTER_MATCH(router, "//", 0, 0, captures_double); +} + +TEST(URITemplateRouter, strict_path_reconstruction_preserves_input) { + sourcemeta::core::URITemplateRouter router; + router.add("/foo//bar", "op_813", 1); + router.add("////", "op_814", 2); + EXPECT_EQ(router.path(1), "/foo//bar"); + EXPECT_EQ(router.path(2), "////"); +} diff --git a/test/uritemplate/uritemplate_router_view_test.cc b/test/uritemplate/uritemplate_router_view_test.cc index bc9a734dc5..95c95a3cba 100644 --- a/test/uritemplate/uritemplate_router_view_test.cc +++ b/test/uritemplate/uritemplate_router_view_test.cc @@ -2124,33 +2124,6 @@ TEST_F(URITemplateRouterViewTest, base_path_expansion) { EXPECT_ROUTER_CAPTURE(captures, 0, "path", "a/b/c"); } -TEST_F(URITemplateRouterViewTest, base_path_trailing_slash_normalized) { - { - sourcemeta::core::URITemplateRouter router{"/prefix/"}; - router.add("/foo", "op_152", 1); - sourcemeta::core::URITemplateRouterView::save(router, this->path); - } - - const sourcemeta::core::URITemplateRouterView restored{this->path}; - EXPECT_EQ(restored.base_path(), "/prefix"); - EXPECT_ROUTER_MATCH(restored, "/prefix/foo", 1, 0, captures); - EXPECT_EQ(captures.size(), 0); -} - -TEST_F(URITemplateRouterViewTest, - base_path_multiple_trailing_slashes_normalized) { - { - sourcemeta::core::URITemplateRouter router{"/prefix///"}; - router.add("/foo", "op_153", 1); - sourcemeta::core::URITemplateRouterView::save(router, this->path); - } - - const sourcemeta::core::URITemplateRouterView restored{this->path}; - EXPECT_EQ(restored.base_path(), "/prefix"); - EXPECT_ROUTER_MATCH(restored, "/prefix/foo", 1, 0, captures); - EXPECT_EQ(captures.size(), 0); -} - TEST_F(URITemplateRouterViewTest, base_url_empty_by_default) { { sourcemeta::core::URITemplateRouter router; @@ -2206,17 +2179,6 @@ TEST_F(URITemplateRouterViewTest, base_url_round_trip_without_base_path) { EXPECT_EQ(captures.size(), 0); } -TEST_F(URITemplateRouterViewTest, base_url_trailing_slash_normalized) { - { - sourcemeta::core::URITemplateRouter router{"", "https://api.example.com/"}; - router.add("/foo", "op_base_url_trailing", 1); - sourcemeta::core::URITemplateRouterView::save(router, this->path); - } - - const sourcemeta::core::URITemplateRouterView restored{this->path}; - EXPECT_EQ(restored.base_url(), "https://api.example.com"); -} - TEST_F(URITemplateRouterViewTest, base_url_arguments_still_resolve) { { sourcemeta::core::URITemplateRouter router{"/v1", @@ -3624,3 +3586,77 @@ TEST_F(URITemplateRouterViewTest, trailing_slash_size_is_two) { const sourcemeta::core::URITemplateRouterView restored{this->path}; EXPECT_EQ(restored.size(), 2); } + +TEST_F(URITemplateRouterViewTest, + strict_internal_double_slash_matches_only_itself) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo//bar", "op_950", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/foo//bar", 1, 0, captures_verbatim); + EXPECT_EQ(captures_verbatim.size(), 0); + EXPECT_ROUTER_MATCH(restored, "/foo/bar", 0, 0, captures_canonical); +} + +TEST_F(URITemplateRouterViewTest, + strict_only_slashes_template_matches_only_itself) { + { + sourcemeta::core::URITemplateRouter router; + router.add("////", "op_951", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "////", 1, 0, captures_match); + EXPECT_EQ(captures_match.size(), 0); + EXPECT_ROUTER_MATCH(restored, "///", 0, 0, captures_short); + EXPECT_ROUTER_MATCH(restored, "/////", 0, 0, captures_long); +} + +TEST_F(URITemplateRouterViewTest, strict_variable_does_not_bind_empty_segment) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/users/{id}", "op_952", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/users/", 0, 0, captures); +} + +TEST_F(URITemplateRouterViewTest, strict_base_path_preserves_trailing_slash) { + { + sourcemeta::core::URITemplateRouter router{"/api/"}; + router.add("/foo", "op_953", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_EQ(restored.base_path(), "/api/"); + EXPECT_ROUTER_MATCH(restored, "/api//foo", 1, 0, captures_strict); + EXPECT_EQ(captures_strict.size(), 0); + EXPECT_ROUTER_MATCH(restored, "/api/foo", 0, 0, captures_lenient); +} + +TEST_F(URITemplateRouterViewTest, strict_path_reconstruction_preserves_input) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/foo//bar", "op_954", 1); + router.add("////", "op_955", 2); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_EQ(restored.path(1), "/foo//bar"); + EXPECT_EQ(restored.path(2), "////"); +} + +TEST_F(URITemplateRouterViewTest, strict_root_template_still_works) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/", "op_956", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_ROUTER_MATCH(restored, "/", 1, 0, captures_match); + EXPECT_EQ(captures_match.size(), 0); + EXPECT_ROUTER_MATCH(restored, "//", 0, 0, captures_double); +} From a1be3b3f56478806f5cfc0f2739b637f6d2124fa Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Sat, 30 May 2026 19:26:00 -0400 Subject: [PATCH 3/4] More Signed-off-by: Juan Cruz Viotti --- src/core/uritemplate/uritemplate_router.cc | 13 ++++ test/uritemplate/uritemplate_router_test.cc | 71 ++++++++++--------- .../uritemplate_router_view_test.cc | 51 +++++++++---- 3 files changed, 90 insertions(+), 45 deletions(-) diff --git a/src/core/uritemplate/uritemplate_router.cc b/src/core/uritemplate/uritemplate_router.cc index a8f60cde88..884d56273e 100644 --- a/src/core/uritemplate/uritemplate_router.cc +++ b/src/core/uritemplate/uritemplate_router.cc @@ -124,6 +124,19 @@ URITemplateRouter::URITemplateRouter(const std::string_view base_path, const std::string_view base_url) : base_path_{base_path}, base_url_{base_url} { assert(this->base_path_.empty() || this->base_path_.front() == '/'); + const auto base_path_last = this->base_path_.find_last_not_of('/'); + if (base_path_last == std::string::npos) { + this->base_path_.clear(); + } else { + this->base_path_.erase(base_path_last + 1); + } + + const auto base_url_last = this->base_url_.find_last_not_of('/'); + if (base_url_last == std::string::npos) { + this->base_url_.clear(); + } else { + this->base_url_.erase(base_url_last + 1); + } } auto URITemplateRouter::base_path() const noexcept -> std::string_view { diff --git a/test/uritemplate/uritemplate_router_test.cc b/test/uritemplate/uritemplate_router_test.cc index c874cc8b7c..fcbf94c065 100644 --- a/test/uritemplate/uritemplate_router_test.cc +++ b/test/uritemplate/uritemplate_router_test.cc @@ -1118,6 +1118,30 @@ TEST(URITemplateRouter, base_path_with_empty_template) { EXPECT_EQ(captures.size(), 0); } +TEST(URITemplateRouter, base_path_slash_only_is_no_base_path) { + sourcemeta::core::URITemplateRouter router{"/"}; + EXPECT_TRUE(router.base_path().empty()); + router.add("/foo", "op_134", 1); + EXPECT_ROUTER_MATCH(router, "/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, base_path_trailing_slash_normalized) { + sourcemeta::core::URITemplateRouter router{"/prefix/"}; + EXPECT_EQ(router.base_path(), "/prefix"); + router.add("/foo", "op_135", 1); + EXPECT_ROUTER_MATCH(router, "/prefix/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST(URITemplateRouter, base_path_multiple_trailing_slashes_normalized) { + sourcemeta::core::URITemplateRouter router{"/prefix///"}; + EXPECT_EQ(router.base_path(), "/prefix"); + router.add("/foo", "op_136", 1); + EXPECT_ROUTER_MATCH(router, "/prefix/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + TEST(URITemplateRouter, base_path_expansion) { sourcemeta::core::URITemplateRouter router{"/api"}; EXPECT_EQ(router.base_path(), "/api"); @@ -1181,6 +1205,21 @@ TEST(URITemplateRouter, base_url_set_without_base_path) { EXPECT_EQ(router.base_url(), "https://api.example.com"); } +TEST(URITemplateRouter, base_url_trailing_slash_normalized) { + sourcemeta::core::URITemplateRouter router{"", "https://api.example.com/"}; + EXPECT_EQ(router.base_url(), "https://api.example.com"); +} + +TEST(URITemplateRouter, base_url_multiple_trailing_slashes_normalized) { + sourcemeta::core::URITemplateRouter router{"", "https://api.example.com///"}; + EXPECT_EQ(router.base_url(), "https://api.example.com"); +} + +TEST(URITemplateRouter, base_url_only_slash_collapses_to_empty) { + sourcemeta::core::URITemplateRouter router{"", "/"}; + EXPECT_TRUE(router.base_url().empty()); +} + TEST(URITemplateRouter, base_url_not_used_for_matching) { sourcemeta::core::URITemplateRouter router{"/v1", "https://api.example.com"}; router.add("/foo", "op_base_url_match", 1); @@ -2739,38 +2778,6 @@ TEST(URITemplateRouter, strict_empty_template_still_special) { EXPECT_EQ(captures.size(), 0); } -TEST(URITemplateRouter, strict_base_path_preserves_trailing_slash) { - sourcemeta::core::URITemplateRouter router{"/api/"}; - EXPECT_EQ(router.base_path(), "/api/"); -} - -TEST(URITemplateRouter, - strict_base_path_with_trailing_slash_requires_empty_segment_in_request) { - sourcemeta::core::URITemplateRouter router{"/api/"}; - router.add("/foo", "op_808", 1); - EXPECT_ROUTER_MATCH(router, "/api//foo", 1, 0, captures_strict); - EXPECT_EQ(captures_strict.size(), 0); - EXPECT_ROUTER_MATCH(router, "/api/foo", 0, 0, captures_lenient); -} - -TEST(URITemplateRouter, - strict_base_path_without_trailing_slash_matches_canonical_request) { - sourcemeta::core::URITemplateRouter router{"/api"}; - router.add("/foo", "op_809", 1); - EXPECT_ROUTER_MATCH(router, "/api/foo", 1, 0, captures); - EXPECT_EQ(captures.size(), 0); -} - -TEST(URITemplateRouter, strict_base_url_preserves_trailing_slash) { - sourcemeta::core::URITemplateRouter router{"", "https://api.example.com/"}; - EXPECT_EQ(router.base_url(), "https://api.example.com/"); -} - -TEST(URITemplateRouter, strict_base_url_preserves_multiple_trailing_slashes) { - sourcemeta::core::URITemplateRouter router{"", "https://api.example.com///"}; - EXPECT_EQ(router.base_url(), "https://api.example.com///"); -} - TEST(URITemplateRouter, strict_internal_empty_then_literal) { sourcemeta::core::URITemplateRouter router; router.add("/a", "op_810", 1); diff --git a/test/uritemplate/uritemplate_router_view_test.cc b/test/uritemplate/uritemplate_router_view_test.cc index 95c95a3cba..3d49912f5f 100644 --- a/test/uritemplate/uritemplate_router_view_test.cc +++ b/test/uritemplate/uritemplate_router_view_test.cc @@ -2124,6 +2124,33 @@ TEST_F(URITemplateRouterViewTest, base_path_expansion) { EXPECT_ROUTER_CAPTURE(captures, 0, "path", "a/b/c"); } +TEST_F(URITemplateRouterViewTest, base_path_trailing_slash_normalized) { + { + sourcemeta::core::URITemplateRouter router{"/prefix/"}; + router.add("/foo", "op_152", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_EQ(restored.base_path(), "/prefix"); + EXPECT_ROUTER_MATCH(restored, "/prefix/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + +TEST_F(URITemplateRouterViewTest, + base_path_multiple_trailing_slashes_normalized) { + { + sourcemeta::core::URITemplateRouter router{"/prefix///"}; + router.add("/foo", "op_153", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_EQ(restored.base_path(), "/prefix"); + EXPECT_ROUTER_MATCH(restored, "/prefix/foo", 1, 0, captures); + EXPECT_EQ(captures.size(), 0); +} + TEST_F(URITemplateRouterViewTest, base_url_empty_by_default) { { sourcemeta::core::URITemplateRouter router; @@ -2179,6 +2206,17 @@ TEST_F(URITemplateRouterViewTest, base_url_round_trip_without_base_path) { EXPECT_EQ(captures.size(), 0); } +TEST_F(URITemplateRouterViewTest, base_url_trailing_slash_normalized) { + { + sourcemeta::core::URITemplateRouter router{"", "https://api.example.com/"}; + router.add("/foo", "op_base_url_trailing", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path); + } + + const sourcemeta::core::URITemplateRouterView restored{this->path}; + EXPECT_EQ(restored.base_url(), "https://api.example.com"); +} + TEST_F(URITemplateRouterViewTest, base_url_arguments_still_resolve) { { sourcemeta::core::URITemplateRouter router{"/v1", @@ -3624,19 +3662,6 @@ TEST_F(URITemplateRouterViewTest, strict_variable_does_not_bind_empty_segment) { EXPECT_ROUTER_MATCH(restored, "/users/", 0, 0, captures); } -TEST_F(URITemplateRouterViewTest, strict_base_path_preserves_trailing_slash) { - { - sourcemeta::core::URITemplateRouter router{"/api/"}; - router.add("/foo", "op_953", 1); - sourcemeta::core::URITemplateRouterView::save(router, this->path); - } - const sourcemeta::core::URITemplateRouterView restored{this->path}; - EXPECT_EQ(restored.base_path(), "/api/"); - EXPECT_ROUTER_MATCH(restored, "/api//foo", 1, 0, captures_strict); - EXPECT_EQ(captures_strict.size(), 0); - EXPECT_ROUTER_MATCH(restored, "/api/foo", 0, 0, captures_lenient); -} - TEST_F(URITemplateRouterViewTest, strict_path_reconstruction_preserves_input) { { sourcemeta::core::URITemplateRouter router; From fe58667e9b3574519c76b06af759c0236978dd06 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Sat, 30 May 2026 20:05:23 -0400 Subject: [PATCH 4/4] Fix Signed-off-by: Juan Cruz Viotti --- src/core/uritemplate/uritemplate_router.cc | 3 ++- test/uritemplate/uritemplate_router_test.cc | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/uritemplate/uritemplate_router.cc b/src/core/uritemplate/uritemplate_router.cc index 884d56273e..a1aea73679 100644 --- a/src/core/uritemplate/uritemplate_router.cc +++ b/src/core/uritemplate/uritemplate_router.cc @@ -438,7 +438,8 @@ auto URITemplateRouter::add(const std::string_view uri_template, ++position; const bool followed_by_path_operator = - position + 1 < end && *position == '{' && *(position + 1) == '/'; + position < end && *position == '{' && position + 1 < end && + *(position + 1) == '/'; if (position < end && *position != '/' && !followed_by_path_operator) { throw URITemplateRouterInvalidSegmentError{ diff --git a/test/uritemplate/uritemplate_router_test.cc b/test/uritemplate/uritemplate_router_test.cc index fcbf94c065..ea2e2586a6 100644 --- a/test/uritemplate/uritemplate_router_test.cc +++ b/test/uritemplate/uritemplate_router_test.cc @@ -2685,13 +2685,13 @@ TEST(URITemplateRouter, TEST(URITemplateRouter, trailing_slash_both_forms_each_has_distinct_arguments) { sourcemeta::core::URITemplateRouter router; const std::array bare_args{ - sourcemeta::core::URITemplateRouter::Argument{ + {sourcemeta::core::URITemplateRouter::Argument{ "kind", sourcemeta::core::URITemplateRouter::ArgumentValue{ - std::string_view{"bare"}}}}; + std::string_view{"bare"}}}}}; const std::array - slashed_args{sourcemeta::core::URITemplateRouter::Argument{ + slashed_args{{sourcemeta::core::URITemplateRouter::Argument{ "kind", sourcemeta::core::URITemplateRouter::ArgumentValue{ - std::string_view{"slashed"}}}}; + std::string_view{"slashed"}}}}}; router.add("/foo", "op_727", 1, 0, bare_args); router.add("/foo/", "op_728", 2, 0, slashed_args); std::string bare_kind;