Skip to content

Commit ea3e67c

Browse files
etrclaude
andcommitted
refactor: clarify response-body and connection-context naming
Rename internal names that read as under-specified for a reader encountering them out of context, keeping the qualifier only where it adds meaning (and off where it would stutter or break public API). - detail::modded_request -> detail::connection_context, and its handle variable mr -> conn throughout the dispatch pipeline. "modded" (modified) said nothing; the struct is the per-connection processing context that adapts MHD's *con_cls and accumulates state across answer_to_connection calls. - detail::body hierarchy -> *_response_body (response_body base; string_/file_/iovec_/pipe_/deferred_/empty_/digest_challenge_ response_body). These flow through the pipeline as bare detail::body*, decontextualized from any response. - string_response_body::content_ -> data_ (get_content -> get_data): the response payload no longer borrows the request-side "content" vocabulary. - digest_challenge::body field and the http_response::string / unauthorized factory params -> response_body. Deliberately unchanged: the public body_kind enum (always reached via http_response::kind(), so response context is present) and http_response's own body_/emplace_body/destroy_body members (the enclosing type already says "response"; response_body_ would stutter). Point-in-time records (ChangeLog, DR decision records, task specs, unworked_review_issues) keep their original terminology. Verified: out-of-source build green, make check 113/113, all header/readme/release-notes gates pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015tAodxYJMEY4VxCX4dk62e
1 parent eea7163 commit ea3e67c

63 files changed

Lines changed: 755 additions & 755 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Makefile.am

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh s
6262
scripts/check-deprecated-cookie-overload.sh \
6363
scripts/test_check_littletest_skip_exit_code.sh \
6464
scripts/verify-installed-examples.sh \
65-
test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_detail_modded.cpp \
65+
test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_detail_connection_context.cpp \
6666
test/headers/consumer_umbrella.cpp \
6767
test/headers/consumer_post_umbrella.cpp \
6868
test/headers/consumer_umbrella_no_backend.cpp
@@ -96,7 +96,7 @@ CHECK_HEADERS_CXX = $(CXX) -std=c++20 -I$(top_builddir) -I$(top_srcdir)/src -I$(
9696
# a public httpserver/* header is included without the umbrella having defined
9797
# _HTTPSERVER_HPP_INSIDE_.
9898
CHECK_HEADERS_PUBLIC_GATE_MSG = Only <httpserver.hpp> or <httpserverpp> can be included directly.
99-
# Detail-header gate (consumer_detail, consumer_detail_modded): post-TASK-014
99+
# Detail-header gate (consumer_detail, consumer_detail_connection_context): post-TASK-014
100100
# the detail-header gate is HTTPSERVER_COMPILATION-only and emits a message of
101101
# the form "httpserver/detail/<file>.hpp is internal; only include it when
102102
# compiling libhttpserver (HTTPSERVER_COMPILATION must be defined)."
@@ -138,22 +138,22 @@ check-headers:
138138
fi
139139
@rm -f check-headers-A2.log
140140
@echo " PASS: A.2 gate fired as expected"
141-
@echo "=== check-headers A.2b: direct detail-header (modded_request) include must fail ==="
141+
@echo "=== check-headers A.2b: direct detail-header (connection_context) include must fail ==="
142142
@# Complements A.2 (http_endpoint.hpp); both detail headers must carry the gate.
143-
@if $(CHECK_HEADERS_CXX) -c $(top_srcdir)/test/headers/consumer_detail_modded.cpp -o /dev/null 2>check-headers-A2b.log; then \
144-
echo "FAIL: consumer_detail_modded.cpp compiled but should have errored"; \
143+
@if $(CHECK_HEADERS_CXX) -c $(top_srcdir)/test/headers/consumer_detail_connection_context.cpp -o /dev/null 2>check-headers-A2b.log; then \
144+
echo "FAIL: consumer_detail_connection_context.cpp compiled but should have errored"; \
145145
cat check-headers-A2b.log; \
146146
rm -f check-headers-A2b.log; \
147147
exit 1; \
148148
fi
149149
@if ! grep -q "$(CHECK_HEADERS_DETAIL_GATE_MSG)" check-headers-A2b.log; then \
150-
echo "FAIL: consumer_detail_modded.cpp failed but not for the gate reason"; \
150+
echo "FAIL: consumer_detail_connection_context.cpp failed but not for the gate reason"; \
151151
cat check-headers-A2b.log; \
152152
rm -f check-headers-A2b.log; \
153153
exit 1; \
154154
fi
155155
@rm -f check-headers-A2b.log
156-
@echo " PASS: A.2b gate fired as expected (modded_request.hpp)"
156+
@echo " PASS: A.2b gate fired as expected (connection_context.hpp)"
157157
@echo "=== check-headers A.3: umbrella include must succeed ==="
158158
@if ! $(CHECK_HEADERS_CXX) -c $(top_srcdir)/test/headers/consumer_umbrella.cpp -o consumer_umbrella.check.o 2>check-headers-A3.log; then \
159159
echo "FAIL: consumer_umbrella.cpp did not compile"; \

specs/architecture/02-architectural-drivers.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
| Attribute | Requirement | Architecture response |
1313
|---|---|---|
14-
| Public-header decoupling | No `<microhttpd.h>` / `<gnutls/gnutls.h>` / `<pthread.h>` / `<sys/socket.h>` / `<sys/uio.h>` in installed headers | PIMPL on `webserver` and `http_request`; forward-declared `detail::body` for `http_response`; high-level accessors (cert DN, fingerprint) replacing raw GnuTLS handles; library-defined `httpserver::iovec_entry` POD replacing `struct iovec` in the public `http_response::iovec(...)` factory |
14+
| Public-header decoupling | No `<microhttpd.h>` / `<gnutls/gnutls.h>` / `<pthread.h>` / `<sys/socket.h>` / `<sys/uio.h>` in installed headers | PIMPL on `webserver` and `http_request`; forward-declared `detail::response_body` for `http_response`; high-level accessors (cert DN, fingerprint) replacing raw GnuTLS handles; library-defined `httpserver::iovec_entry` POD replacing `struct iovec` in the public `http_response::iovec(...)` factory |
1515
| Build-flag stability | Public API surface invariant under `HAVE_BAUTH` / `HAVE_DAUTH` / `HAVE_GNUTLS` / `HAVE_WEBSOCKET` | Unconditional declarations; runtime sentinels or `feature_unavailable` throws when backends disabled; `webserver::features()` reports availability |
1616
| Const correctness | Pure accessors `const`; lazy caches OK via `mutable`; daemon-driving methods exempt | Request-side caches in `mutable` storage (or unique_ptr); `is_running` / `get_fdset` / `get_timeout` documented as exempt operations |
1717
| Hot-path performance | Per-request getters do not allocate or copy containers | Container-returning getters change to `const&` / `string_view`; per-request impl arena-allocated from a per-connection `std::pmr::monotonic_buffer_resource`; method-state held as a `uint32_t` bitmask, not a `std::map` |

specs/architecture/03-system-overview.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
│ ├── per-connection arena (std::pmr::monotonic_buffer_resource) │
2323
│ └── http_request::impl (allocated from connection's arena) │
2424
│ │
25-
│ detail::body (polymorphic; subclasses string/file/iovec/pipe/ │
25+
│ detail::response_body (polymorphic; subclasses string/file/iovec/pipe/ │
2626
│ deferred/empty live in detail/body.hpp) │
2727
└──────────┬───────────────────────────────────────────────────────────┘
2828
@@ -41,7 +41,7 @@
4141
| `http_response` | Response value: status, headers, footers, cookies, body | Non-PIMPL value type; polymorphic body in 64-byte SBO buffer with heap fallback |
4242
| `http_resource` | Class-form handler (state shared across HTTP methods of one resource) | Public abstract base; allow-mask held as `method_set` (`uint32_t` bitmask) |
4343
| `websocket_handler` | Per-endpoint WebSocket protocol handler | Public abstract base; registered via `unique_ptr` / `shared_ptr` overloads |
44-
| `detail::body` | Polymorphic body kinds (string / file / iovec / pipe / deferred / empty) | Internal hierarchy in `src/httpserver/detail/body.hpp` |
44+
| `detail::response_body` | Polymorphic body kinds (string / file / iovec / pipe / deferred / empty) | Internal hierarchy in `src/httpserver/detail/body.hpp` |
4545
| Route table | Path → (method_set, handler) lookup | `unordered_map` (exact) + radix tree (parameterized + prefix) + regex chain (fallback) |
4646

4747
---

specs/architecture/04-components/body-hierarchy.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
### 4.8 `detail::body` hierarchy
1+
### 4.8 `detail::response_body` hierarchy
22

33
**Responsibility:** Polymorphic body representation backing `http_response`'s SBO buffer. Each subclass carries the data needed for one body kind and knows how to stream itself into an MHD response.
44

@@ -21,24 +21,24 @@ protected:
2121
body& operator=(body&&) = delete;
2222
};
2323

24-
class string_body : public body { /* std::string content_; */ };
25-
class file_body : public body { /* std::string path_; std::size_t size_; int fd_; bool materialized_; */ };
26-
class iovec_body : public body { /* std::vector<iovec_entry> entries_; std::size_t total_size_; */ };
27-
class pipe_body : public body { /* int fd_; bool materialized_; */ };
28-
class deferred_body: public body { /* std::function<ssize_t(uint64_t, char*, std::size_t)> producer_; */ };
29-
class empty_body : public body { /* int flags_; */ };
24+
class string_response_body : public body { /* std::string content_; */ };
25+
class file_response_body : public body { /* std::string path_; std::size_t size_; int fd_; bool materialized_; */ };
26+
class iovec_response_body : public body { /* std::vector<iovec_entry> entries_; std::size_t total_size_; */ };
27+
class pipe_response_body : public body { /* int fd_; bool materialized_; */ };
28+
class deferred_response_body: public body { /* std::function<ssize_t(uint64_t, char*, std::size_t)> producer_; */ };
29+
class empty_response_body : public body { /* int flags_; */ };
3030
}
3131
```
3232
3333
**SBO storage:** factories use placement-new into the response's `body_storage_` buffer when the subclass fits (always true for v2.0's set). New body kinds added in v2.x check at compile time (`static_assert`) whether they fit; if they don't, the factory falls back to `new`-allocating and storing the heap pointer.
3434
3535
**Materialization timing:** `materialize()` is called from `webserver`'s dispatch, not from the handler. The body holds whatever data it needs (strings, paths, callables) until that point.
3636
37-
**file_body open contract (implementation-close-out note):** Unlike the general guidance above, `file_body` opens the file descriptor and calls `fstat` at **construction** rather than in `materialize()`. This deliberate eager-open design achieves two goals: (1) `size()` is accurate immediately without calling `materialize()` first, and (2) `materialize()` can use `st_size` from the already-obtained `fstat` result, avoiding a TOCTOU race and an extra `lseek` syscall. The lazy-open guidance applies to the other body kinds (pipe, deferred); it does not apply to `file_body`.
37+
**file_response_body open contract (implementation-close-out note):** Unlike the general guidance above, `file_response_body` opens the file descriptor and calls `fstat` at **construction** rather than in `materialize()`. This deliberate eager-open design achieves two goals: (1) `size()` is accurate immediately without calling `materialize()` first, and (2) `materialize()` can use `st_size` from the already-obtained `fstat` result, avoiding a TOCTOU race and an extra `lseek` syscall. The lazy-open guidance applies to the other body kinds (pipe, deferred); it does not apply to `file_response_body`.
3838
39-
**pipe_body ownership contract:** `pipe_body` takes ownership of a read-side file descriptor at construction. If `materialize()` succeeds, libmicrohttpd owns the fd (it is closed when `MHD_destroy_response` is called). If `materialize()` is never called, `~pipe_body()` closes the fd to prevent a leak. The `materialized_` field suppresses the destructor's close once MHD has taken ownership.
39+
**pipe_response_body ownership contract:** `pipe_response_body` takes ownership of a read-side file descriptor at construction. If `materialize()` succeeds, libmicrohttpd owns the fd (it is closed when `MHD_destroy_response` is called). If `materialize()` is never called, `~pipe_response_body()` closes the fd to prevent a leak. The `materialized_` field suppresses the destructor's close once MHD has taken ownership.
4040
41-
**iovec_body allocation note:** `iovec_body` intentionally incurs one heap allocation for its `std::vector<iovec_entry>` backing store. The SBO slot holds only the vector control block (~24 bytes); the iovec entry array lives on the heap. This is the only SBO-resident body kind that performs a secondary heap allocation, and it is accepted as an inherent cost of `std::vector` (not a DR-005 violation — DR-005 governs the body-pointer allocation, not internal allocations within a fitting body).
41+
**iovec_response_body allocation note:** `iovec_response_body` intentionally incurs one heap allocation for its `std::vector<iovec_entry>` backing store. The SBO slot holds only the vector control block (~24 bytes); the iovec entry array lives on the heap. This is the only SBO-resident body kind that performs a secondary heap allocation, and it is accepted as an inherent cost of `std::vector` (not a DR-005 violation — DR-005 governs the body-pointer allocation, not internal allocations within a fitting body).
4242
4343
**Related requirements:** PRD-RSP-REQ-006, PRD-HDR-REQ-005.
4444

specs/architecture/04-components/create-webserver.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ The builder remains non-PIMPL (it's a pure value carrier; PIMPL would buy nothin
1010
callback is `std::function<std::optional<http_response>(const http_request&)>`.
1111
Returning `std::nullopt` allows the request through; returning an
1212
engaged optional short-circuits the before_handler chain with that
13-
response (the dispatcher moves it into `mr->response`). The earlier v2
13+
response (the dispatcher moves it into `conn->response`). The earlier v2
1414
work-in-progress shape returned `std::shared_ptr<http_response>`; that
1515
shape remains available for one transitional build via the
1616
`httpserver::compat::auth_handler_v1_ptr` typedef alias and a

specs/architecture/04-components/dispatch-pipeline.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
**Responsibility:** The request-processing behavior of the server — everything between a libmicrohttpd callback firing and a response being queued — factored out of the `webserver_impl` god-object (DR-014) into seven per-server **behavior services**. Each is an internal `httpserver::detail` type gated on `HTTPSERVER_COMPILATION`; none appears on the public surface or ABI. (The connection-lifecycle callbacks — `connection_notify` / `policy_callback` / `request_completed` — were *not* extracted into an eighth service; they stay in the MHD-adapter layer, see below and DR-014.)
44

5-
**What a behavior service is.** Distinct from a *state collaborator* (route_table/hook_bus/… — owns a mutex + data): a behavior service owns *logic*, not state. It is constructed once per `webserver_impl`, holds only `const&` references to its dependencies, owns no mutable state, takes no locks, and operates on the per-request `detail::modded_request` passed in by reference. Because it is stateless and lock-free it is inherently shareable across MHD worker threads; the decomposition adds no synchronization.
5+
**What a behavior service is.** Distinct from a *state collaborator* (route_table/hook_bus/… — owns a mutex + data): a behavior service owns *logic*, not state. It is constructed once per `webserver_impl`, holds only `const&` references to its dependencies, owns no mutable state, takes no locks, and operates on the per-request `detail::connection_context` passed in by reference. Because it is stateless and lock-free it is inherently shareable across MHD worker threads; the decomposition adds no synchronization.
66

77
**The seven services.**
88

@@ -33,11 +33,11 @@ websocket_upgrader ─▶ ws_registry
3333

3434
Acyclic, so the composition root wires it with plain member references — no mediator. Services store references at construction but never invoke a dependency during their own constructor, so binding a reference to a sibling member is well-defined irrespective of member-declaration order (`-Wreorder -Werror` guards accidental reorders).
3535

36-
**The MHD adapter layer.** libmicrohttpd calls in through fixed-signature C trampolines carrying a `void* cls` closure: `answer_to_connection`, `request_completed`, `connection_notify`, `policy_callback`, `post_iterator`, `uri_log`, `error_log`, `unescaper_func`, `upgrade_handler`, plus the GnuTLS `psk_cred_handler_func` / `sni_cert_callback_func`. These stay `static`/free functions — they unpack `cls` (a `webserver_impl*`, `webserver*`, `modded_request*`, or `ws_upgrade_data*`) and forward into a service. They are the C-ABI boundary, not behavior. `connection_notify`, `policy_callback`, and `request_completed` are the fullest members of this layer: they keep their per-connection arena (`connection_state` new/delete via `socket_context`), accept-policy, and request-teardown glue inline, delegating only the extracted free functions (`classify_decision`, `make_peer_address`) and the `ip_access_control` / `hook_dispatcher` collaborators — which is why they were not carved into a separate service (DR-014).
36+
**The MHD adapter layer.** libmicrohttpd calls in through fixed-signature C trampolines carrying a `void* cls` closure: `answer_to_connection`, `request_completed`, `connection_notify`, `policy_callback`, `post_iterator`, `uri_log`, `error_log`, `unescaper_func`, `upgrade_handler`, plus the GnuTLS `psk_cred_handler_func` / `sni_cert_callback_func`. These stay `static`/free functions — they unpack `cls` (a `webserver_impl*`, `webserver*`, `connection_context*`, or `ws_upgrade_data*`) and forward into a service. They are the C-ABI boundary, not behavior. `connection_notify`, `policy_callback`, and `request_completed` are the fullest members of this layer: they keep their per-connection arena (`connection_state` new/delete via `socket_context`), accept-policy, and request-teardown glue inline, delegating only the extracted free functions (`classify_decision`, `make_peer_address`) and the `ip_access_control` / `hook_dispatcher` collaborators — which is why they were not carved into a separate service (DR-014).
3737

3838
**Free functions (not services).** Pure, instance-stateless helpers live as free functions in `httpserver::detail`, not one-method classes: `log_dispatch_error(const webserver_config&, std::string_view)` (called by every error path — a free function keeps it dependency-edge-free), `serialize_allow_methods` / `format_allow_header`, `resolve_method_callback`, `materialize_response`, `decorate_mhd_response`, `handle_post_form_arg`, `manage_upload_stream`.
3939

40-
**Per-request context.** `detail::modded_request` (per connection, arena/PMR-allocated per DR-003b) is the object the pipeline threads through the MHD callback sequence: `uri_log` allocates it; `answer_to_connection` (first invocation) stamps `start_time` / `standardized_url` / `method_enum` and builds the `http_request`; the body steps accumulate upload data; `finalize_answer` stages `response`; `request_completed` fires the terminal hook and deletes it. The services read and write its fields but do not own it.
40+
**Per-request context.** `detail::connection_context` (per connection, arena/PMR-allocated per DR-003b) is the object the pipeline threads through the MHD callback sequence: `uri_log` allocates it; `answer_to_connection` (first invocation) stamps `start_time` / `standardized_url` / `method_enum` and builds the `http_request`; the body steps accumulate upload data; `finalize_answer` stages `response`; `request_completed` fires the terminal hook and deletes it. The services read and write its fields but do not own it.
4141

4242
**Key design notes:**
4343
- Config is read-only to every service (`const webserver_config&`); no service holds the `webserver*` back-pointer. The back-pointer stays on the composition root for the members that genuinely need the owning `webserver*`.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
**Responsibility:** Stateful handler base for cases where state is shared across HTTP methods of one resource (counter, cache, DB handle, auth context).
44

5-
**Implementation:** Public abstract base. Subclasses override one of `render_get / render_post / render_put / render_delete / render_patch / render_options / render_head` (renamed from v1's `render_GET` etc., to comply with PRD-NAM-REQ-001 snake_case). Each `render_*` override returns `http_response` **by value** (DR-004 / PRD-RSP-REQ-007); the webserver moves the returned value into the per-connection `modded_request::response` anchor. The default `render(...)` falls back when the method-specific override is not provided; it returns a default-constructed `http_response` whose `status_code_ == -1` sentinel routes through `internal_error_page`.
5+
**Implementation:** Public abstract base. Subclasses override one of `render_get / render_post / render_put / render_delete / render_patch / render_options / render_head` (renamed from v1's `render_GET` etc., to comply with PRD-NAM-REQ-001 snake_case). Each `render_*` override returns `http_response` **by value** (DR-004 / PRD-RSP-REQ-007); the webserver moves the returned value into the per-connection `connection_context::response` anchor. The default `render(...)` falls back when the method-specific override is not provided; it returns a default-constructed `http_response` whose `status_code_ == -1` sentinel routes through `internal_error_page`.
66

77
The allow-mask (formerly `std::map<std::string, bool> method_state`) becomes `method_set methods_allowed_;` — a `uint32_t` bitmask wrapper (DR-6). `is_allowed(http_method)` and `get_allowed_methods()` are `const` and return without allocation.
88

0 commit comments

Comments
 (0)