TASK-072: arena-allocated unescape on the warm path

Route the GET-argument unescape output through the per-connection
arena so the warm-path zero-global-allocation guarantee holds
unconditionally -- previously the v1 code path materialised the
unescape result in a std::string temporary, paying one global-heap
allocation per argument when the value exceeded std::string's SSO
threshold (TASK-018 acknowledged deferral).

build_request_args now allocates the destination pmr::string
directly in the arena domain and runs the standard %HH / '+' decode
in place via a TU-local http_unescape_raw helper. When a user
unescaper is registered the ABI-locked `void(std::string&)` callback
is fed a thread_local std::string scratch buffer whose capacity
amortises across requests on the same worker thread, so the
steady-state per-request global-heap cost is zero on the user path
too.

Tests pinned in test/unit/http_request_unescape_arena_test.cpp:
the headline pair use a global operator-new counter (RAII window
guard) to assert that build_request_args makes zero global-heap
allocations on the warm cycle for both the default and the
user-registered unescaper paths. Correctness + lifetime tests
cover %2F decode, invalid-hex passthrough, empty input,
view-outlives-subsequent-inserts, and the user-callback grow
path.

bench_warm_path gains two variants: (5) %2F-containing value
through the arena-routed unescape and (6) no-escape baseline. The
two medians land within noise (99 ns vs 102 ns on the debug build),
satisfying the "matches the no-unescape baseline within noise"
acceptance criterion.

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

Author: etr

specs/architecture/04-components/http-request.md

@@ -19,6 +19,7 @@
 
 **Key design notes:**
 - The arena allocator is plumbed through `webserver_impl` → connection state → `http_request` constructor. The user does not see it; it is an internal optimization.
+  - GET-argument population (`http_request_impl::build_request_args`) writes the unescaped value directly into the per-connection arena (TASK-072). The standard `%HH` / `+`→space transformation runs in-place on the arena-backed `pmr::string`; when a user-registered unescaper is set, a per-thread reusable `std::string` scratch buffer adapts the ABI-locked `void(std::string&)` callback signature without a per-request global-heap allocation. Pinned by `test/unit/http_request_unescape_arena_test.cpp` (zero global `operator new` calls during the warm cycle) and exercised by `bench_warm_path` variants (5) and (6).
 - Containers returned by `get_*()` reference impl-owned storage; the request must outlive any view derived from it. Documented as a lifetime contract.
 - `gnutls_session_t` (raw GnuTLS handle) is not exposed publicly. Users wanting custom TLS introspection use the high-level `get_client_cert_*` accessors. The handle remains accessible via friend access from internal code.
 

specs/tasks/M7-v2-cleanup/TASK-072.md

@@ -8,10 +8,10 @@
 `src/detail/http_request_impl.cpp:202-206` is an acknowledged TASK-018 deferral: the warm-path "0-alloc" criterion explicitly does not apply when a user unescaper is registered, because the unescape call still allocates a `std::string`. Route the unescape output into the per-connection arena (TASK-016) so the criterion holds unconditionally.
 
 **Action Items:**
-- [ ] Extend the per-connection arena (from TASK-016) with an `unescape_into(string_view in, void* sink) -> string_view` helper, or have the arena expose a scratch-buffer API the unescape can write into.
-- [ ] Refactor the unescape call site at `http_request_impl.cpp:202-206` to allocate into the arena. The returned `string_view` is valid for the lifetime of the request, matching the TASK-018 lifetime documentation.
-- [ ] Update the inline TASK-018 deferral comment to reflect that arena unescape is now wired.
-- [ ] Extend `test/bench_warm_path.cpp` (TASK-058) with an unescape variant: warm GET with `%2F` in the path. Pin "no allocations under heap profiler" as in TASK-058.
+- [x] Extend the per-connection arena (from TASK-016) with an `unescape_into(string_view in, void* sink) -> string_view` helper, or have the arena expose a scratch-buffer API the unescape can write into.
+- [x] Refactor the unescape call site at `http_request_impl.cpp:202-206` to allocate into the arena. The returned `string_view` is valid for the lifetime of the request, matching the TASK-018 lifetime documentation.
+- [x] Update the inline TASK-018 deferral comment to reflect that arena unescape is now wired.
+- [x] Extend `test/bench_warm_path.cpp` (TASK-058) with an unescape variant: warm GET with `%2F` in the path. Pin "no allocations under heap profiler" as in TASK-058.
 
 **Dependencies:**
 - Blocked by: TASK-016 (arena, Done), TASK-018 (string_view getters, Done), TASK-058 (warm-path bench infra, Done)
@@ -27,4 +27,4 @@
 **Related Requirements:** PRD-REQ-REQ-001 (const-correctness + zero-alloc)
 **Related Decisions:** DR-003b (arena)
 
-**Status:** Backlog
+**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

@@ -36,7 +36,7 @@ TASK-093).
 | TASK-069 | Remove transitional two-arg `http_request_impl` constructor | MED | S | Done |
 | TASK-070 | Migrate `hook_table_` to `std::atomic<std::shared_ptr<T>>` | MED | M | Backlog |
 | TASK-071 | Wire `install_not_found_alias_` stub and remove dead `lambda_handler` arm | MED | S | Done |
-| TASK-072 | Arena-allocated unescape on the warm path | MED | M | Backlog |
+| TASK-072 | Arena-allocated unescape on the warm path | MED | M | Done |
 | TASK-073 | Revisit libmicrohttpd v0.99 unescape workaround | LOW | S | Backlog |
 | TASK-074 | Gate DEBUG raw-body printing behind explicit opt-in | LOW (sec) | S | Backlog |
 | TASK-075 | Propagate `wait_for_server_ready` to sibling hooks integ tests | HIGH | M | Backlog |

src/detail/http_request_impl.cpp

@@ -155,6 +155,90 @@ const http::header_view_map& http_request_impl::ensure_headerlike_cache(MHD_Valu
     return *cache;
 }
 
+namespace {
+
+// TASK-072: hex_digit_value matches src/http_utils.cpp's static helper.
+// Duplicated here (rather than promoted to a header) because both
+// copies are tiny and trivially constexpr-foldable; centralising it
+// would require either a header-only inline helper or an exposed
+// non-public symbol, neither of which justifies the surface area.
+constexpr int hex_digit_value(char c) noexcept {
+    if (c >= '0' && c <= '9') return c - '0';
+    if (c >= 'a' && c <= 'f') return c - 'a' + 10;
+    if (c >= 'A' && c <= 'F') return c - 'A' + 10;
+    return -1;
+}
+
+// TASK-072: raw-buffer unescape, mirrors http_unescape(std::string*)
+// byte-identically. Operates in-place on [data, data+size); returns
+// the new size after the transformation. The output is guaranteed to
+// be <= input length (the standard %HH / '+'->' ' replacement only
+// ever shrinks).
+inline std::size_t http_unescape_raw(char* data, std::size_t size) noexcept {
+    if (size == 0 || data[0] == '\0') return 0;
+    std::size_t rpos = 0;
+    std::size_t wpos = 0;
+    while (rpos < size && data[rpos] != '\0') {
+        switch (data[rpos]) {
+            case '+':
+                data[wpos++] = ' ';
+                ++rpos;
+                break;
+            case '%':
+                if (size > rpos + 2) {
+                    int hi = hex_digit_value(data[rpos + 1]);
+                    int lo = hex_digit_value(data[rpos + 2]);
+                    if (hi >= 0 && lo >= 0) {
+                        data[wpos++] =
+                            static_cast<char>((hi << 4) | lo);
+                        rpos += 3;
+                        break;
+                    }
+                }
+            // intentional fall through!
+            default:
+                data[wpos++] = data[rpos++];
+        }
+    }
+    return wpos;
+}
+
+// TASK-072: arena-routed unescape. The caller passes an arena-backed
+// pmr::string already holding the raw bytes; we run the unescape
+// transformation directly on the pmr::string's storage so no
+// global-heap allocation occurs on the warm path.
+//
+// When a user unescaper is registered, the public callback signature
+// is `void(std::string&)` (ABI-locked), so we route through a
+// thread_local std::string whose capacity amortises across all
+// requests on the same worker thread. The first request on a thread
+// grows it once; subsequent requests reuse the capacity (zero
+// per-request global-heap allocations on the warm path). The result
+// bytes are then assigned back into the arena-backed sink.
+inline void unescape_in_arena(std::pmr::string& sink,
+                              unescaper_ptr user_fn) {
+    if (sink.empty()) return;
+    if (user_fn == nullptr) {
+        // Default %HH / '+' decode: run in-place on the arena
+        // buffer, then truncate to the new size.
+        const std::size_t new_size = http_unescape_raw(
+            sink.data(), sink.size());
+        sink.resize(new_size);
+        return;
+    }
+    // User-callback path: route through a per-thread reusable
+    // std::string scratch buffer so the warm-path cost is zero
+    // global-heap allocations. The thread_local lives for the
+    // worker thread's lifetime; capacity grows once per peak input
+    // size on the thread.
+    thread_local std::string user_scratch;
+    user_scratch.assign(sink.data(), sink.size());
+    user_fn(user_scratch);
+    sink.assign(user_scratch.data(), user_scratch.size());
+}
+
+}  // namespace
+
 MHD_Result http_request_impl::build_request_args(void* cls, MHD_ValueKind kind,
                                                  const char* key, const char* arg_value) {
     // Parameters needed to respect MHD interface, but not used in the implementation.
@@ -190,28 +274,26 @@ MHD_Result http_request_impl::build_request_args(void* cls, MHD_ValueKind kind,
     }
     aa->accumulated_bytes += this_pair_bytes;
 
-    // Unescape into a temporary std::string (the C-style unescaper is
-    // string-typed). This causes one global-heap allocation (the temp) even
-    // on the warm arena path, followed by a second copy from the temp into
-    // the arena-backed pmr::string via emplace_back below. The warm-path
-    // zero-upstream-allocs guarantee therefore does NOT hold for requests
-    // with GET arguments when an unescaper is active.
+    // TASK-072: route the unescape output into the per-connection
+    // arena. The destination pmr::string lives in the arena domain, so
+    // its bytes consume only arena memory (no global-heap allocation).
     //
-    // Tracked as TASK-018: route the unescape output directly into a
-    // pmr::string using the arena allocator, eliminating both the temp and
-    // the second copy. Until then, the acceptance criterion '0 bytes
-    // global-heap allocation from impl construction on warm connection'
-    // applies to construction and arena-backed containers only -- not to
-    // argument population when an unescaper is registered.
-    // (performance-reviewer-iter1-1: acknowledged deferral)
-    std::string value(val_sv);
-    http::base_unescaper(&value, aa->unescaper);
-
-    // Reuse the iterator from the hoisted find above: insert if missing,
-    // then append the (possibly unescaped) value. The key is stored as
-    // pmr::string in the map's allocator domain; the value vector is
-    // allocator-constructed in place via the same allocator (scoped
-    // propagation gives nested pmr::strings the right allocator too).
+    // Two paths:
+    //   - null user-unescaper: do the standard %HH / '+'->' '
+    //     transformation in-place on the arena-backed pmr::string via
+    //     http_unescape_raw (raw-buffer overload of the standard
+    //     unescape, defined in the anonymous namespace below).
+    //   - user-registered unescaper: the public callback signature
+    //     `void(std::string&)` is ABI-locked, so we pass it a
+    //     thread_local std::string scratch buffer; its capacity
+    //     amortises across requests on the same worker thread, leaving
+    //     the warm-path per-request cost at zero global-heap
+    //     allocations. The result is then copied into the arena-backed
+    //     pmr::string sink.
+    //
+    // The returned pmr::string's data is owned by the arena and lives
+    // until connection_state::reset_arena() runs (request completion),
+    // matching the TASK-018 string_view lifetime contract.
     auto pmr_alloc = args.get_allocator();
     auto it = existing_it;
     if (it == args.end()) {
@@ -221,11 +303,14 @@ MHD_Result http_request_impl::build_request_args(void* cls, MHD_ValueKind kind,
             std::move(empty));
         it = inserted.first;
     }
-    // emplace_back into a pmr::vector<pmr::string>: use (ptr, size); the
-    // outer vector's allocator-propagating construct wires the inner
-    // pmr::string's allocator automatically. Passing the allocator
-    // ourselves leads to double-injection via uses-allocator construction.
-    it->second.emplace_back(value.data(), value.size());
+
+    // Allocate the destination pmr::string in the arena domain and
+    // copy the raw input bytes into it. The outer vector's allocator-
+    // propagating construct wires the inner pmr::string's allocator,
+    // so we pass (ptr, size) here. The unescape transformation then
+    // runs in-place on the arena-backed buffer.
+    auto& sink = it->second.emplace_back(val_sv.data(), val_sv.size());
+    unescape_in_arena(sink, aa->unescaper);
     return MHD_YES;
 }
 

test/Makefile.am

@@ -26,7 +26,7 @@ LDADD += -lcurl
 
 AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/ -DHTTPSERVER_COMPILATION
 METASOURCES = AUTO
-check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication digest_challenge_format deferred http_resource http_response create_webserver create_webserver_explicit new_response_types daemon_info uri_log feature_unavailable header_hygiene_iovec header_hygiene iovec_entry http_method constants body http_response_sbo http_response_factories http_response_digest_factory http_response_move_sanitizer webserver_pimpl http_request_pimpl create_test_request http_request_arena http_request_const_getters http_request_tls_accessors http_request_operator_stream webserver_register_smartptr webserver_register_path_prefix webserver_on_methods webserver_route route_table lookup_pipeline route_table_concurrency routing_regression route_lookup_canonicalize auth_skip_normalize http_resource_allow_cache v2_dispatch_contract threadsafety_stress webserver_features webserver_ws_unavailable webserver_register_ws_smartptr webserver_dauth_unavailable consumer_fixture header_hygiene_hooks hook_api_shape hooks_no_firing hooks_accept_ctx_shape hooks_connection_lifecycle hooks_accept_decision_banned hooks_accept_decision_throwing hooks_body_chunk_ctx_shape hooks_request_received_short_circuit hooks_body_chunk_observes_progress hooks_body_chunk_short_circuit_no_leak hooks_before_handler_ctx_shape hooks_route_resolved_miss_and_hit hooks_before_handler_short_circuit hooks_alias_count hooks_alias_functional hooks_handler_exception_chain hooks_handler_exception_user_handler_throws_continues_chain hooks_handler_exception_fallback_to_hardcoded_500 hooks_handler_exception_slot hooks_response_sent_ctx_shape hooks_request_completed_ctx_shape hooks_after_handler_replaces_response hooks_after_handler_mutates_response_in_place hooks_response_sent_carries_status_bytes_timing hooks_request_completed_fires_on_early_failure hooks_log_access_alias_slot hooks_per_route_invalid_phase_throws hooks_per_route_order hooks_per_route_early_413_per_endpoint hooks_per_route_resource_destroyed_first hooks_per_route_concurrent_registration hooks_not_found_alias auth_handler_optional_signature no_v1_compat_shim cookie_header_sentinel cookie_render cookie_deprecation_sentinel http_response_cookie_wire http_request_cookies_parsed peer_address_to_string secure_zero_dce connection_state_sentinel connection_state_body_residue
+check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication digest_challenge_format deferred http_resource http_response create_webserver create_webserver_explicit new_response_types daemon_info uri_log feature_unavailable header_hygiene_iovec header_hygiene iovec_entry http_method constants body http_response_sbo http_response_factories http_response_digest_factory http_response_move_sanitizer webserver_pimpl http_request_pimpl create_test_request http_request_arena http_request_unescape_arena http_request_const_getters http_request_tls_accessors http_request_operator_stream webserver_register_smartptr webserver_register_path_prefix webserver_on_methods webserver_route route_table lookup_pipeline route_table_concurrency routing_regression route_lookup_canonicalize auth_skip_normalize http_resource_allow_cache v2_dispatch_contract threadsafety_stress webserver_features webserver_ws_unavailable webserver_register_ws_smartptr webserver_dauth_unavailable consumer_fixture header_hygiene_hooks hook_api_shape hooks_no_firing hooks_accept_ctx_shape hooks_connection_lifecycle hooks_accept_decision_banned hooks_accept_decision_throwing hooks_body_chunk_ctx_shape hooks_request_received_short_circuit hooks_body_chunk_observes_progress hooks_body_chunk_short_circuit_no_leak hooks_before_handler_ctx_shape hooks_route_resolved_miss_and_hit hooks_before_handler_short_circuit hooks_alias_count hooks_alias_functional hooks_handler_exception_chain hooks_handler_exception_user_handler_throws_continues_chain hooks_handler_exception_fallback_to_hardcoded_500 hooks_handler_exception_slot hooks_response_sent_ctx_shape hooks_request_completed_ctx_shape hooks_after_handler_replaces_response hooks_after_handler_mutates_response_in_place hooks_response_sent_carries_status_bytes_timing hooks_request_completed_fires_on_early_failure hooks_log_access_alias_slot hooks_per_route_invalid_phase_throws hooks_per_route_order hooks_per_route_early_413_per_endpoint hooks_per_route_resource_destroyed_first hooks_per_route_concurrent_registration hooks_not_found_alias auth_handler_optional_signature no_v1_compat_shim cookie_header_sentinel cookie_render cookie_deprecation_sentinel http_response_cookie_wire http_request_cookies_parsed peer_address_to_string secure_zero_dce connection_state_sentinel connection_state_body_residue
 
 MOSTLYCLEANFILES = *.gcda *.gcno *.gcov
 
@@ -207,6 +207,16 @@ http_request_operator_stream_LDADD = $(LDADD) -lmicrohttpd
 http_request_arena_SOURCES = unit/http_request_arena_test.cpp
 http_request_arena_LDADD = $(LDADD) -lmicrohttpd
 
+# http_request_unescape_arena: TASK-072 unit test. Pins the warm-path
+# zero-upstream-alloc invariant for build_request_args with a "%2F"-
+# containing GET argument, both for the default unescaper and for a
+# user-registered unescaper. Also covers correctness (percent-encoded
+# decode, invalid-hex passthrough, empty input, lifetime contract,
+# user-callback grow). Needs -lmicrohttpd because it touches MHD types
+# through the impl.
+http_request_unescape_arena_SOURCES = unit/http_request_unescape_arena_test.cpp
+http_request_unescape_arena_LDADD = $(LDADD) -lmicrohttpd
+
 # http_request_const_getters: TASK-018 sentinel. Compile-time assertions
 # that the per-key getters (get_header / get_cookie / get_footer /
 # get_arg / get_arg_flat) and the always-present getters (get_path /