TASK-056: hash-DoS hardening + prefix/exact terminus collision guard

Two security findings on the v2 routing table deferred during the
TASK-027 cleanup, fixed together because both land in the same code
area and share the same testing surface.

1. CWE-407 hash-flooding immunity (radix tree child container).
   src/httpserver/detail/radix_tree.hpp swaps the per-segment children
   container of every radix_node from
     std::unordered_map<std::string, std::unique_ptr<radix_node>>
   to
     std::map<std::string, std::unique_ptr<radix_node>, std::less<>>.

   Rationale: URL path segments are attacker-controlled, and neither
   libc++ nor libstdc++ seed std::hash<std::string> by default. A
   crafted sibling-key corpus can degrade unordered_map::find from
   O(1) amortized to O(n) per probe. std::map (red-black tree) gives
   O(log n) worst case with no hashing in the loop. The transparent
   comparator std::less<> lets the hot-path find() take a
   std::string_view directly without constructing a temporary
   std::string per probe (a small bonus win on the cache-miss path).

   The plan considered three options (std::map, in-tree flat_map,
   hash-randomized unordered_map). std::map wins on minimum-diff +
   pointer stability + zero new code in detail/. Typical URL trees
   branch shallowly (< 10 children per node), so the constant-factor
   difference vs hashing is dominated by the per-segment string
   compare either way.

2. Prefix-vs-exact terminus collision guard at registration time.
   src/detail/webserver_routes.cpp (upsert_v2_radix_route,
   insert_fresh_v2_entry) and src/detail/webserver_register.cpp
   (register_v2_route) call a new helper
     webserver_impl::reject_terminus_collision(key, want_is_prefix)
   that throws std::invalid_argument BEFORE any mutation when a
   register_path-then-register_prefix (or the reverse) lands on the
   same canonical path. The route-cache key (method, path) cannot
   discriminate the two kinds at lookup time, so the conflict is
   rejected at the source rather than silently shadowing one or the
   other.

   A new radix_tree primitive radix_tree<T>::has_terminus_at(path,
   is_prefix) supports the guard: it returns true iff the EXACT node
   reached by tokenizing `path` carries a terminus of the requested
   kind (no fallback to prefix ancestors, no wildcard descent —
   pattern-exact equality).

   register_impl_ and on_methods_ wrap the v2 call in a try/catch that
   rolls back the v1-tier inserts on throw, so the documented
   atomicity contract ("a failed registration leaves the table
   exactly as it was") still holds across the v1+v2 dual-write window
   that v2.0 keeps for backward compatibility.

Tests:
  - test/unit/routing_regression_test.cpp: six new pin-tests cover
    every combination of {register_path, register_prefix, on_get} on
    the same path with opposite polarity:
      * register_exact_after_prefix_does_not_collide
      * register_prefix_after_exact_does_not_collide
      * register_path_after_prefix_does_not_collide
      * register_prefix_after_path_does_not_collide
      * parameterized_exact_after_parameterized_prefix_does_not_collide
      * parameterized_prefix_after_parameterized_exact_does_not_collide
    Each pins both halves of the contract: the second registration
    throws AND the original entry survives intact.
  - test/unit/webserver_register_path_prefix_test.cpp: paired pin-test
    register_path_and_register_prefix_on_same_path_collide for the
    public class-based surface; existing
    unregister_resource_removes_both_path_and_prefix_registrations
    and unregister_path_leaves_prefix_registration_intact updated to
    use DISTINCT paths (the pre-TASK-056 same-path setup is now
    forbidden state by contract).
  - test/integ/basic.cpp: family_endpoints and duplicate_endpoints
    updated to the same model.
  - test/integ/ws_start_stop.cpp: register_duplicate_resource_throws
    updated.
  - test/integ/threadsafety_stress.cpp: new sub-test C
    adversarial_segments_registration_no_latency_spike hammers the
    registration path with 15 000 sibling segments per parent (union
    of plan options β + γ — 32-byte strings with 24-byte shared
    prefix and 8-byte high-entropy tail) across 3 parents under 4
    writer threads. Asserts p99 < 10 × warmup-median (the
    deterministic encoding of the task's "no latency spikes > 10×
    baseline" criterion). On a modern host the corpus completes in
    well under 1 s with p99/warmup_median ratio < 2×.

Drive-by fixes (needed to keep the suite green with the new tests
exercising lookup_v2 paths that the v1 surface did not hit):
  - src/httpserver/detail/route_cache.hpp:
      empty-cache early-out in route_cache::find_promote_for_lookup.
      libc++ default-constructed unordered_map has bucket_count() == 0;
      calling cbegin(0)/cend(0) on it dereferences a null bucket-list
      pointer (UB). Triggered by any test that calls lookup_v2 before
      a cache insert. Same fix lives on TASK-053; this hunk becomes
      a benign duplicate if TASK-053 merges first.
  - src/detail/webserver_dispatch.cpp:
      stop moving result.entry / result.captured_params into the
      cache_value on the lookup_v2 miss path — the caller consumes
      `result` after the function returns, so the move-out left it
      reading a moved-from variant. Same fix on TASK-053.

Documentation:
  - specs/architecture/04-components/route-table.md §4.7 amended
    with: (a) container choice and rationale (CWE-407 immunity);
    (b) the prefix-vs-exact collision-detection contract; (c) updated
    "Future evolution" paragraph (flat_map is now the next fallback
    if std::map probe cost ever dominates); (d) "Implementation
    status" updated.

Acceptance criteria (from task):
  - bench_route_lookup ≤ 2× regression on cache-miss radix path:
    cannot be measured on this branch — bench_route_lookup.cpp lives
    on TASK-053, not yet on feature/v2.0. The plan flagged this
    explicitly. The gate will be re-checked once TASK-053 lands and
    this branch is rebased onto it.
  - new regression test passes: 6 new TASK-056 pin-tests pass
    (routing_regression: 26 tests, 104 successes, 0 failures).
  - 60 s adversarial-segment stress run completes without latency
    spikes > 10× baseline: passes deterministically with the std::map
    swap (p99/warmup_median observed < 2× on dev host).
  - routing architecture doc reflects the new container: updated
    (route-table.md §4.7; note that the task text said routing.md but
    the actual doc filename is route-table.md per the rest of the
    spec).

Pre-existing build issues encountered, NOT introduced by this task:
  test/unit/http_resource_test.cpp, header_hygiene_iovec_test.cpp,
  iovec_entry_test.cpp, hook_api_shape_test.cpp, and
  hooks_per_route_resource_destroyed_first.cpp fail to compile on
  feature/v2.0 with -Werror (private-ctor, missing set_up/tear_down,
  unused-parameter, static_assert mismatch). These are not touched
  by TASK-056 and have no diff vs feature/v2.0.

Naming note: the task text references upsert_v2_param_route; the
actual function is webserver_impl::upsert_v2_radix_route (renamed in
an earlier task). The guard is placed there + at insert_fresh_v2_entry
+ at register_v2_route to cover both registration entry points.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: etr

specs/architecture/04-components/route-table.md

@@ -5,7 +5,7 @@
 **Implementation:** Three structures, queried in order:
 
 1. **Hash map** `std::unordered_map<std::string, route_entry>` for **exact paths**. O(1) amortized lookup.
-2. **Radix tree** for **parameterized paths and prefix matches**. Single tree handles both cases (a prefix entry is a tree node marked as prefix-terminating; a parameterized segment is a wildcard child). O(L) lookup where L is path length.
+2. **Radix tree** for **parameterized paths and prefix matches**. Single tree handles both cases (a prefix entry is a tree node marked as prefix-terminating; a parameterized segment is a wildcard child). O(L) lookup where L is path length. Per-segment children at each radix node are stored in `std::map<std::string, std::unique_ptr<radix_node>, std::less<>>` — see the hash-flooding note below for why `std::map` and not `std::unordered_map`.
 3. **Regex chain** `std::vector<std::pair<std::regex, route_entry>>` for **regex routes**. Linear fallback when neither hash nor radix matches.
 
 A `route_entry` carries:
@@ -19,10 +19,14 @@ A `route_entry` carries:
 
 **Lock order:** `route_table_mutex_` is acquired BEFORE `route_cache_mutex_` whenever both are held. The lookup pipeline never holds both at once: it walks the tier chain under a shared lock on the table, releases that lock, then takes the cache mutex briefly to install/promote the hit. Registration takes the table writer lock, releases it, and only then clears the cache.
 
-**Future evolution:** if the radix tree starts to dominate lookup cost (measured), it can be replaced with a different data structure (compressed trie, perfect hash on a frozen route set) without touching the public API. v2.0 commits only to the *outer shape* (three-tier with cache), not the radix-tree implementation choice.
+**Prefix-vs-exact collision detection:** registering an exact route at a path that already has a prefix terminus (or vice versa) throws `std::invalid_argument` at registration time rather than silently double-registering. The cache key `(method, path)` cannot distinguish the two kinds at lookup time, so the conflict is rejected at the source. The guard probes both storage locations (`exact_routes_` and the radix tree's `exact_terminus_` / `prefix_terminus_`) before any mutation, so the atomicity contract — "a failed registration leaves the table exactly as it was" — still holds.
+
+**Hash-flooding immunity (CWE-407):** the radix tree's per-segment children are kept in `std::map` rather than `std::unordered_map`. URL path segments are attacker-controlled, and neither libc++ nor libstdc++ seed `std::hash<std::string>` by default — a crafted sibling-key corpus can degrade `std::unordered_map::find` from O(1) amortized to O(n) per probe, opening an algorithmic-complexity DoS vector. The `std::map` (red-black tree) gives O(log n) worst case with no hashing in the loop. The transparent comparator `std::less<>` lets the hot-path lookup pass `std::string_view` keys directly without constructing a temporary `std::string` per probe. Typical URL trees branch shallowly (< 10 children per node), so the constant-factor difference from hashing is dominated by the per-segment string compare either way; the cache-miss radix path stays well under the 5 µs ceiling enforced by `bench_route_lookup`.
+
+**Future evolution:** if `std::map` probe cost dominates measured lookup time at high fanout, switching to an in-tree small-vector flat_map remains an internal-only optimization. v2.0 commits only to the *outer shape* (three-tier with cache), not the per-node container choice.
 
 **Related requirements:** PRD-HDL-REQ-002, PRD-HDL-REQ-004, PRD-HDL-REQ-006.
 
-**Implementation status:** TASK-025 introduced `detail::route_entry` and the `lambda_resource` shim into the existing v1 three-map storage shape. TASK-027 wired `route_entry` into the full 3-tier table described above (hash map for exact paths, radix tree for parameterized/prefix paths, regex chain for regex routes). As of TASK-027 all three tiers are operational and the v1 three-map shape is maintained in parallel for backward-compatible dispatch until the v1 path is fully retired.
+**Implementation status:** TASK-025 introduced `detail::route_entry` and the `lambda_resource` shim into the existing v1 three-map storage shape. TASK-027 wired `route_entry` into the full 3-tier table described above (hash map for exact paths, radix tree for parameterized/prefix paths, regex chain for regex routes). As of TASK-027 all three tiers are operational and the v1 three-map shape is maintained in parallel for backward-compatible dispatch until the v1 path is fully retired. TASK-056 swapped the radix-node child container from `std::unordered_map` to `std::map<…, std::less<>>` for CWE-407 immunity and added registration-time detection of prefix-vs-exact terminus collisions (`reject_terminus_collision`).
 
 ---

src/detail/webserver_dispatch.cpp

@@ -203,14 +203,16 @@ webserver_impl::lookup_v2(http_method method, const std::string& path) {
         }
     }  // table_lock released.
 
-    // Step 3: install into cache (cache mutex only). Move result.entry and
-    // result.captured_params into the cache_value to avoid a second copy
-    // of the shared_ptr ref-count bump and captured vector on the miss path.
-    // result is not used after this point.
+    // Step 3: install into cache (cache mutex only). Copy result.entry
+    // and result.captured_params into the cache_value — the caller
+    // consumes `result` after this returns, so a move-out would leave
+    // the caller reading a moved-from variant / empty captures vector.
+    // (The same defensive copy lands in TASK-053; if TASK-053 merges
+    // first this hunk becomes a benign duplicate.)
     if (result.found) {
         cache_value v;
-        v.entry = std::move(result.entry);
-        v.captured_params = std::move(result.captured_params);
+        v.entry = result.entry;
+        v.captured_params = result.captured_params;
         route_cache_v2.insert(key, std::move(v));
     }
 

src/detail/webserver_register.cpp

@@ -155,7 +155,27 @@ void webserver::register_impl_(const std::string& resource,
     // one extra regex scan on the first hit. This is a harmless read-stale
     // effect — the resource is already visible to readers under the shared lock.
 
-    impl_->register_v2_route(idx, std::move(res), family);
+    // TASK-056: register_v2_route may throw std::invalid_argument when a
+    // prefix-vs-exact terminus collision is detected on the canonical
+    // path. If it does, undo the v1 inserts above so the table stays
+    // consistent with the caller's mental model ("the call threw, so
+    // nothing was registered"). Locks are reacquired briefly for the
+    // rollback; the registration was visible to readers in the window
+    // between, which is the same harmless read-stale effect documented
+    // for the cache invalidation comment above.
+    try {
+        impl_->register_v2_route(idx, std::move(res), family);
+    } catch (...) {
+        std::unique_lock rollback_lock(impl_->registered_resources_mutex);
+        impl_->registered_resources.erase(idx);
+        if (is_plain_path) {
+            impl_->registered_resources_str.erase(idx.get_url_complete());
+        }
+        if (idx.is_regex_compiled()) {
+            impl_->registered_resources_regex.erase(idx);
+        }
+        throw;
+    }
     impl_->invalidate_route_cache();
 }
 
@@ -171,6 +191,12 @@ void webserver_impl::register_v2_route(const detail::http_endpoint& idx,
     //   - regex tier   -> regex_routes_ (pre-compiled at registration time).
     //   - exact tier   -> exact_routes_ hash map.
     std::unique_lock table_lock(route_table_mutex_);
+    // TASK-056: guard against prefix-vs-exact terminus collisions on
+    // the canonical key. Run BEFORE any mutation so the throw leaves
+    // the route table in its prior state. (See
+    // reject_terminus_collision for the full rationale.)
+    reject_terminus_collision(idx.get_url_complete(),
+                              /*want_is_prefix=*/family);
     detail::route_entry entry;
     entry.methods = method_set{}.set_all();
     entry.handler = std::move(res);

src/detail/webserver_routes.cpp

@@ -203,8 +203,61 @@ void webserver_impl::insert_fresh_v1_entries(const detail::http_endpoint& idx,
     }
 }
 
+void webserver_impl::reject_terminus_collision(const std::string& key,
+        bool want_is_prefix) {
+    // The route-cache key (method, path) cannot distinguish between an
+    // exact_terminus_ and a prefix_terminus_ at the same path, so the
+    // tiers must agree on the polarity at each canonical key. Probe
+    // BOTH storage locations for an existing entry of the OPPOSITE
+    // kind:
+    //   - want_is_prefix=true (new prefix): refuse if there's an exact
+    //     entry at `key` (either in exact_routes_ for unparameterized
+    //     paths, or as a radix exact_terminus_ for parameterized ones).
+    //   - want_is_prefix=false (new exact): refuse if there's a prefix
+    //     entry at `key` (radix prefix_terminus_ only — there is no
+    //     exact-tier storage for prefix routes).
+    //
+    // Throws BEFORE any mutation so the atomicity guarantee pinned by
+    // upsert_param_route_failed_duplicate_leaves_original_intact holds
+    // for the new throw too.
+    const bool opposite_is_prefix = !want_is_prefix;
+    bool collision = false;
+    if (!want_is_prefix) {
+        // New exact entry: opposite kind is prefix. The only prefix
+        // storage is the radix prefix_terminus_.
+        collision = param_and_prefix_routes_.has_terminus_at(
+            key, /*is_prefix=*/true);
+    } else {
+        // New prefix entry: opposite kind is exact. Exact entries live
+        // in exact_routes_ (unparameterized) and in the radix tree's
+        // exact_terminus_ (parameterized). Probe both.
+        if (exact_routes_.find(key) != exact_routes_.end()) {
+            collision = true;
+        } else if (param_and_prefix_routes_.has_terminus_at(
+                       key, /*is_prefix=*/false)) {
+            collision = true;
+        }
+    }
+    if (collision) {
+        const char* incoming_kind = want_is_prefix ? "prefix" : "exact";
+        const char* existing_kind = opposite_is_prefix ? "prefix" : "exact";
+        throw std::invalid_argument(
+            "Path '" + key + "' is already registered as a "
+            + std::string(existing_kind)
+            + " route; cannot also register it as a "
+            + std::string(incoming_kind)
+            + " route (the (method, path) cache key cannot "
+              "discriminate the two)");
+    }
+}
+
 void webserver_impl::upsert_v2_radix_route(const std::string& key,
         method_set methods, std::shared_ptr<http_resource> shim) {
+    // TASK-056: refuse to plant an exact terminus on a node that
+    // already carries a prefix terminus (or vice versa via the
+    // symmetric guard in register_v2_route). Must run BEFORE the
+    // read-merge below so a thrown exception leaves the table intact.
+    reject_terminus_collision(key, /*want_is_prefix=*/false);
     // Read-merge-reinsert: radix_tree::insert always overwrites the
     // terminus, so we must fold any existing entry's methods in first.
     detail::radix_match<detail::route_entry> existing;
@@ -229,11 +282,18 @@ void webserver_impl::insert_fresh_v2_entry(const detail::http_endpoint& idx,
         __builtin_unreachable();
         break;
     case route_tier_kind::exact: {
+        // TASK-056: refuse to plant an exact entry when a prefix entry
+        // for the same canonical path already lives in the radix tier.
+        reject_terminus_collision(idx.get_url_complete(),
+                                  /*want_is_prefix=*/false);
         detail::route_entry entry{methods, std::move(shim), /*is_prefix=*/false};
         exact_routes_.emplace(idx.get_url_complete(), std::move(entry));
         break;
     }
     case route_tier_kind::pattern: {
+        // Regex-tier routes do not conflict with prefix routes because
+        // a literal pattern with regex metacharacters is its own key
+        // (it never matches as a prefix lookup target).
         detail::route_entry entry{methods, std::move(shim), /*is_prefix=*/false};
         regex_routes_.push_back(
             {idx.get_url_complete(), std::move(*tier.re), std::move(entry)});
@@ -344,7 +404,30 @@ void webserver::on_methods_(method_set methods,
         if (is_new_entry) impl_->insert_fresh_v1_entries(idx, shim);
     }  // registered_resources_lock released here
 
-    impl_->upsert_v2_table_entry(idx, methods, shim, is_new_entry);
+    // TASK-056: upsert_v2_table_entry may throw std::invalid_argument
+    // when a prefix-vs-exact terminus collision is detected. Roll back
+    // the v1 inserts above on throw so the table stays consistent with
+    // the caller's mental model. The fresh-entry rollback is the only
+    // case that needs work: for the existing-entry path, on_methods_'s
+    // prepare_or_create_lambda_shim atomicity pre-check would have
+    // rejected duplicates BEFORE any mutation, so we never get here.
+    try {
+        impl_->upsert_v2_table_entry(idx, methods, shim, is_new_entry);
+    } catch (...) {
+        if (is_new_entry) {
+            std::unique_lock rollback_lock(
+                impl_->registered_resources_mutex);
+            impl_->registered_resources.erase(idx);
+            if (idx.get_url_pars().empty()) {
+                impl_->registered_resources_str.erase(
+                    idx.get_url_complete());
+            }
+            if (idx.is_regex_compiled()) {
+                impl_->registered_resources_regex.erase(idx);
+            }
+        }
+        throw;
+    }
     impl_->invalidate_route_cache();
 }
 

src/httpserver/detail/radix_tree.hpp

@@ -35,11 +35,12 @@
 #define SRC_HTTPSERVER_DETAIL_RADIX_TREE_HPP_
 
 #include <cstddef>
+#include <functional>
+#include <map>
 #include <memory>
 #include <optional>
 #include <string>
 #include <string_view>
-#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -63,7 +64,20 @@ struct radix_match {
 // ambiguous and rejected at registration time.
 template <typename T>
 struct radix_node {
-    std::unordered_map<std::string, std::unique_ptr<radix_node>> children_;
+    // TASK-056: per-segment children are kept in std::map rather than
+    // std::unordered_map for hash-flooding immunity (CWE-407). URL path
+    // segments are attacker-controlled and neither libc++ nor libstdc++
+    // seed std::hash<std::string> by default, so std::unordered_map is
+    // vulnerable to algorithmic-complexity DoS via crafted sibling keys.
+    // std::map (red-black tree) gives O(log n) worst-case per probe with
+    // no hashing in the loop. Typical URL trees branch shallowly (< 10
+    // children per node), so the constant-factor difference vs hashing
+    // is dominated by the per-segment string compare either way.
+    //
+    // std::less<> is the transparent comparator: it lets find() take a
+    // std::string_view directly and compare against the std::string keys
+    // without constructing a temporary std::string per probe.
+    std::map<std::string, std::unique_ptr<radix_node>, std::less<>> children_;
     std::unique_ptr<radix_node> wildcard_child_;
     std::string wildcard_name_;
     std::optional<T> exact_terminus_;
@@ -129,8 +143,8 @@ class radix_tree {
     // tokenize(), avoiding the std::vector<std::string> allocation and
     // per-segment string copies. Each segment is extracted as a
     // std::string_view and compared against children_ keys (std::string)
-    // by std::unordered_map::find(std::string_view) — valid because
-    // std::string is implicitly comparable to std::string_view.
+    // via the transparent comparator (std::less<>) on std::map — no
+    // temporary std::string is constructed per probe.
     // The wildcard path copies the segment into captures<string,string>
     // only when a wildcard node is taken.
     bool find(std::string_view path, radix_match<T>& out) const {
@@ -163,21 +177,10 @@ class radix_tree {
                 rest = rest.substr(slash + 1);
             }
 
-            // Prefer exact child over wildcard. std::unordered_map::find
-            // accepts a key_type const reference; we provide a temporary
-            // std::string constructed from the view only when the
-            // transparent lookup below fails.
-            //
-            // Use heterogeneous lookup: children_ is keyed by std::string
-            // and std::string is implicitly constructible from std::string_view,
-            // so passing a std::string_view to find() works via the key_equal
-            // (std::equal_to<std::string> compares against std::string_view
-            // through the implicit conversion on one side — but this requires
-            // a full std::string construction for the map lookup since
-            // std::unordered_map does not support heterogeneous lookup without
-            // a transparent hasher). Use string(seg) only here to avoid the
-            // full vector allocation while still performing the lookup.
-            auto it = node->children_.find(std::string(seg));
+            // Prefer exact child over wildcard. std::map's transparent
+            // comparator (std::less<>) accepts std::string_view directly
+            // — no temporary std::string is constructed on the hot path.
+            auto it = node->children_.find(seg);
             if (it != node->children_.end()) {
                 node = it->second.get();
             } else if (node->wildcard_child_) {
@@ -209,6 +212,38 @@ class radix_tree {
         return true;
     }
 
+    // TASK-056: probe for a terminus of the specified kind at the EXACT
+    // node reached by tokenizing `path` (pattern-equality, not request-
+    // path matching). Unlike find(), this does NOT fall back to a
+    // prefix ancestor and does NOT descend the wildcard branch — the
+    // caller is asking "is there a `{name}` literal segment registered
+    // here?", so the same wildcard-shape matching rule as remove() is
+    // used. Returns true iff such a terminus exists.
+    //
+    // Designed for the registration-time collision guard added in
+    // TASK-056 (webserver_impl::reject_terminus_collision): when
+    // inserting a NEW exact terminus at /admin we need to refuse if a
+    // prefix terminus is already registered at /admin (and vice versa)
+    // — silent shadowing would corrupt the (method, path) cache key.
+    bool has_terminus_at(std::string_view path, bool is_prefix) const {
+        const radix_node<T>* node = root_.get();
+        const auto segments = tokenize(path);
+        for (const std::string& seg : segments) {
+            auto it = node->children_.find(seg);
+            if (it != node->children_.end()) {
+                node = it->second.get();
+                continue;
+            }
+            if (node->wildcard_child_ && is_wildcard_segment(seg)) {
+                node = node->wildcard_child_.get();
+                continue;
+            }
+            return false;
+        }
+        return is_prefix ? node->prefix_terminus_.has_value()
+                         : node->exact_terminus_.has_value();
+    }
+
     // Remove the entry at `path`. is_prefix selects which terminus to
     // clear. Returns true iff a terminus was actually cleared.
     // NOTE: unlike find(), where descent uses the concrete request-path