Skip to content

Commit db36c0f

Browse files
committed
TASK-054: migrate auth_handler_ptr to optional<http_response>
Flips the canonical auth handler typedef from std::function<std::shared_ptr<http_response>(const http_request&)> to std::function<std::optional<http_response>(const http_request&)>, completing the DR-009 / PRD-RSP-REQ-007 value-typed response rollout onto the last remaining heap-allocating slot on the public API. One-build escape hatch: - compat::auth_handler_v1_ptr keeps the v1 shared_ptr-returning std::function shape (with a [[deprecated]] attribute). - compat::adapt_legacy_auth(legacy) wraps the v1 callable into the new optional shape (nullptr -> nullopt; non-null -> moved-from optional). - A [[deprecated]] create_webserver::auth_handler(compat::auth_handler_v1_ptr) overload accepts the legacy shape and routes through adapt_legacy_auth. Both deprecation sites cite TASK-054 and point at the new typedef. Dispatch path: the before_handler auth-alias hook in webserver_aliases.cpp now consumes std::optional<http_response> directly. Removes one heap allocation per authenticated request (the shared_ptr control block); small responses still ride the http_response SBO with zero further allocs. Tests: - test/unit/auth_handler_optional_signature_test.cpp NEW: static_assert the new typedef shape; end-to-end pin nullopt-allows + engaged-rejects via curl; hook-count invariant (+1 on before_handler). - test/unit/auth_handler_legacy_shim_test.cpp NEW: pin the v1 typedef alias shape, the deprecated setter overload, hook-count invariant via the shim, and verbatim status/header forwarding through compat::adapt_legacy_auth (proves response state is moved, not lost). TU-scope #pragma GCC diagnostic ignored "-Wdeprecated-declarations". - Migrated: test/unit/hooks_alias_count_test.cpp (auth lambdas -> optional shape) test/unit/webserver_register_path_prefix_test.cpp (reject_auth) test/unit/create_webserver_test.cpp (builder_auth_handler) test/integ/authentication.cpp (centralized_auth_handler) Docs: - README.md: setter blurb + worked example switched to optional shape. - RELEASE_NOTES.md: added "auth handler" entry under "What changed semantically" + rename-pair row in the v1->v2 table. - specs/architecture/04-components/create-webserver.md: documented the new signature, the compat alias, and the one-build deprecation window. - src/httpserver/create_webserver.hpp: Doxygen on the setter rewritten; TODO at lines 92-99 removed. - examples/centralized_authentication.cpp: drops <memory>/make_shared, returns std::optional<http_response> directly. Verification: - All TASK-054 new + migrated tests PASS (auth_handler_optional_signature, auth_handler_legacy_shim, hooks_alias_count, webserver_register_path_prefix, create_webserver, authentication). - 73 PASS / 0 NEW FAIL in the broader test suite. 3 pre-existing baseline segfaults (lookup_pipeline, route_table_concurrency, routing_regression) and 6 pre-existing compile failures (basic, http_resource, header_hygiene_iovec, iovec_entry, hook_api_shape, hooks_per_route_resource_destroyed_first) reproduce on feature/v2.0 HEAD without these changes. - scripts/check-examples.sh, check-readme.sh, check-release-notes.sh: OK. - cpplint clean on new + modified files. Note on the task action-item line: the spec says "Update the auth short-circuit path in webserver_dispatch.cpp" -- the actual call site post-TASK-048 is in webserver_aliases.cpp::install_default_alias_hooks_ (verified by grep -- webserver_dispatch.cpp does not reference auth_handler). The fix is in webserver_aliases.cpp.
1 parent 00d57e2 commit db36c0f

13 files changed

Lines changed: 584 additions & 57 deletions

README.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -618,9 +618,10 @@ build, `webserver(create_webserver{}.use_ssl(true))` throws
618618
digest auth.
619619
* **`.auth_handler(auth_handler_ptr cb)`** — install a centralized
620620
authentication handler that runs before every resource's `render_*`
621-
call. The callback returns `nullptr` to allow the request to proceed,
622-
or an `http_response` (typically `http_response::unauthorized(realm)`)
623-
to reject it. Single source of truth for auth — see [Centralized
621+
call. The callback returns `std::nullopt` to allow the request to
622+
proceed, or an `http_response` (typically
623+
`http_response::unauthorized(realm)`) to reject it. Single source of
624+
truth for auth — see [Centralized
624625
authentication](#using-centralized-authentication).
625626
* **`.auth_skip_paths(const std::vector<std::string>& paths)`** — paths
626627
to bypass `auth_handler`. Exact match (`"/health"`) and wildcard
@@ -1226,14 +1227,13 @@ resource. Centralized authentication lets you define the policy once and
12261227
have it applied automatically to every request:
12271228

12281229
```cpp
1229-
// auth runs once per request before any render_*; return nullptr to allow
1230+
// auth runs once per request before any render_*; return std::nullopt to allow
12301231
auto auth = [](const httpserver::http_request& req)
1231-
-> std::shared_ptr<httpserver::http_response> {
1232+
-> std::optional<httpserver::http_response> {
12321233
if (req.get_user() != "admin" || req.get_pass() != "secret") {
1233-
return std::make_shared<httpserver::http_response>(
1234-
httpserver::http_response::unauthorized("MyRealm"));
1234+
return httpserver::http_response::unauthorized("MyRealm");
12351235
}
1236-
return nullptr;
1236+
return std::nullopt;
12371237
};
12381238

12391239
httpserver::webserver ws{httpserver::create_webserver(8080)
@@ -1248,9 +1248,10 @@ ws.start(true);
12481248
The `auth_handler` callback runs for every request before the resource's
12491249
render method. It receives the `http_request` and can:
12501250
1251-
* Return `nullptr` to allow the request to proceed normally.
1251+
* Return `std::nullopt` to allow the request to proceed normally.
12521252
* Return an `http_response` (typically `http_response::unauthorized`) to
1253-
reject the request.
1253+
reject the request. The response is moved onto the wire by the
1254+
dispatcher; no heap allocation is required.
12541255
12551256
`auth_skip_paths` accepts a vector of paths that should bypass the
12561257
handler:

RELEASE_NOTES.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ and see the v2 replacement.
159159
| `deferred_response` | `http_response::deferred` |
160160
| `basic_auth_fail_response` | `http_response::unauthorized` |
161161
| `digest_auth_fail_response` | `http_response::unauthorized` |
162+
| `auth_handler_ptr` returning `std::shared_ptr<http_response>` | `auth_handler_ptr` returning `std::optional<http_response>` |
162163

163164
> **Note:** `shoutCAST()` is the sole camelCase survivor in the v2 public API.
164165
> The name maps to the SHOUTcast streaming protocol and is intentionally
@@ -171,6 +172,16 @@ and see the v2 replacement.
171172
v2 returns a value. The framework moves it into the dispatch path. No
172173
heap allocation is required for small responses (small-buffer optimisation
173174
inside `http_response`).
175+
- **Centralized auth handler returns `std::optional<http_response>`.**
176+
The `auth_handler` callback signature is now
177+
`std::function<std::optional<http_response>(const http_request&)>`
178+
(earlier v2 work-in-progress shipped
179+
`std::function<std::shared_ptr<http_response>(const http_request&)>`).
180+
Return `std::nullopt` to allow the request; return an `http_response`
181+
to reject. The v1 `shared_ptr` shape still compiles for one
182+
transitional build via `httpserver::compat::auth_handler_v1_ptr` and a
183+
`[[deprecated]]` setter overload; both emit a deprecation warning.
184+
Removes one heap allocation per authenticated request.
174185
- **`http_request` getters return `const&` / `string_view`.** v1's
175186
`get_header(name)` (and `get_arg`, `get_cookie`, `get_footer`) returned
176187
by value and inserted an empty entry into the request map on miss; v2's

examples/centralized_authentication.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*/
2020

2121
// centralized_authentication.cpp - install a server-wide auth_handler
22-
// that runs before every request. The handler returns nullptr to
22+
// that runs before every request. The handler returns std::nullopt to
2323
// accept, or an http_response to reject. auth_skip_paths lists routes
2424
// that bypass auth entirely.
2525
//
@@ -42,29 +42,30 @@
4242

4343
#include <cstdlib>
4444
#include <iostream>
45-
#include <memory>
45+
#include <optional>
4646
#include <string>
4747

4848
#include <httpserver.hpp>
4949

50-
// Returns nullptr to allow the request, or an http_response to reject it.
51-
std::shared_ptr<httpserver::http_response> auth_handler(
50+
// Returns std::nullopt to allow the request, or an engaged optional
51+
// carrying an http_response to reject it (TASK-054).
52+
std::optional<httpserver::http_response> auth_handler(
5253
const httpserver::http_request& req) {
5354
const char* expected_user = std::getenv("AUTH_USER");
5455
const char* expected_pass = std::getenv("AUTH_PASS");
5556
if (!expected_user || !expected_pass) {
5657
std::cerr << "centralized_authentication: AUTH_USER and AUTH_PASS "
5758
"environment variables must be set.\n";
58-
return std::make_shared<httpserver::http_response>(
59+
return std::optional<httpserver::http_response>(
5960
httpserver::http_response::string("Server configuration error")
6061
.with_status(500));
6162
}
6263
if (req.get_user() != expected_user || req.get_pass() != expected_pass) {
63-
return std::make_shared<httpserver::http_response>(
64+
return std::optional<httpserver::http_response>(
6465
httpserver::http_response::unauthorized(
6566
"Basic", "MyRealm", "Unauthorized"));
6667
}
67-
return nullptr;
68+
return std::nullopt;
6869
}
6970

7071
int main() {

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@
66

77
The builder remains non-PIMPL (it's a pure value carrier; PIMPL would buy nothing).
88

9-
**Related requirements:** PRD-CFG-REQ-001..004.
9+
**`auth_handler_ptr` shape (TASK-054).** The centralised authentication
10+
callback is `std::function<std::optional<http_response>(const http_request&)>`.
11+
Returning `std::nullopt` allows the request through; returning an
12+
engaged optional short-circuits the before_handler chain with that
13+
response (the dispatcher moves it into `mr->response`). The earlier v2
14+
work-in-progress shape returned `std::shared_ptr<http_response>`; that
15+
shape remains available for one transitional build via the
16+
`httpserver::compat::auth_handler_v1_ptr` typedef alias and a
17+
`[[deprecated]]` setter overload (`auth_handler(compat::auth_handler_v1_ptr)`),
18+
both of which wrap the legacy callable via `compat::adapt_legacy_auth`
19+
into the canonical `auth_handler_ptr` shape and emit a deprecation
20+
diagnostic at the call site. The compat alias and overload will be
21+
removed after the next release. Rationale: completes the
22+
PRD-RSP-REQ-007 value-typed response cutover (DR-009) onto the auth
23+
path, removing the per-authenticated-request control-block allocation.
24+
25+
**Related requirements:** PRD-CFG-REQ-001..004, PRD-RSP-REQ-007.
1026

1127
---

src/detail/webserver_aliases.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
#include <cassert>
5959
#include <functional>
6060
#include <memory>
61+
#include <optional>
6162
#include <string>
6263
#include <string_view>
6364
#include <utility>
@@ -211,15 +212,18 @@ void webserver::install_default_alias_hooks_() {
211212
if (impl_ptr->should_skip_auth(path)) {
212213
return hook_action::pass();
213214
}
214-
// Call the user-supplied auth_handler. Returns nullptr
215-
// to mean "allow" (pass-through); non-null means
216-
// "reject with this response". The shared_ptr<> is the
217-
// v1 nullable-optional pattern; see the TODO on
218-
// auth_handler_ptr in create_webserver.hpp for the
219-
// deferred migration plan.
220-
std::shared_ptr<http_response> auth_rejection_response =
215+
// Call the user-supplied auth_handler. TASK-054: the
216+
// return type is std::optional<http_response>. nullopt
217+
// means "allow"; an engaged optional carries the
218+
// rejection response. Compared to the v1
219+
// shared_ptr<http_response> shape this saves the
220+
// per-authenticated-request control-block allocation
221+
// (one heap alloc removed per request that runs through
222+
// this hook), and small responses ride the http_response
223+
// SBO with zero further allocs.
224+
std::optional<http_response> auth_rejection_response =
221225
ws_ptr->auth_handler(*ctx.request);
222-
if (auth_rejection_response == nullptr) {
226+
if (!auth_rejection_response.has_value()) {
223227
return hook_action::pass();
224228
}
225229
// Auth failed: short-circuit with the rejection response.

src/httpserver/create_webserver.hpp

Lines changed: 90 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <memory>
3131
#include <functional>
3232
#include <limits>
33+
#include <optional>
3334
#include <stdexcept>
3435
#include <string>
3536
#include <string_view>
@@ -89,15 +90,71 @@ namespace http { class file_info; }
8990

9091
typedef std::function<bool(const std::string&, const std::string&, const http::file_info&)> file_cleanup_callback_ptr;
9192

92-
// TODO(follow-up): migrate auth_handler_ptr to
93-
// std::function<std::optional<http_response>(const http_request&)>
94-
// to complete the DR-004 value-return rollout to the auth hook (removing
95-
// the per-authenticated-request heap allocation for the shared_ptr control
96-
// block). This requires updating the call site in webserver_finalize.cpp,
97-
// the example in examples/centralized_authentication.cpp, and the
98-
// integration test in test/integ/authentication.cpp. Deferred from TASK-036
99-
// to avoid a cascading public-API break mid-milestone.
100-
typedef std::function<std::shared_ptr<http_response>(const http_request&)> auth_handler_ptr;
93+
/**
94+
* Centralised authentication callback signature.
95+
*
96+
* Returning `std::nullopt` allows the request to proceed to dispatch.
97+
* Returning an engaged `std::optional<http_response>` short-circuits the
98+
* before_handler chain and sends that response on the wire (typically a
99+
* 401 produced by `http_response::unauthorized(realm)`).
100+
*
101+
* Migrated from `std::function<std::shared_ptr<http_response>(...)>` to
102+
* remove the per-authenticated-request control-block allocation; the
103+
* by-value `http_response` carries small bodies via SBO without any heap
104+
* traffic. See TASK-054, DR-009, PRD-RSP-REQ-007.
105+
*/
106+
typedef std::function<std::optional<http_response>(const http_request&)> auth_handler_ptr;
107+
108+
namespace compat {
109+
110+
/**
111+
* One-transitional-build alias preserving the v1 auth callable shape
112+
* (`std::function<std::shared_ptr<http_response>(const http_request&)>`).
113+
*
114+
* A v1 caller can migrate without an in-place call-site change by
115+
* spelling the type as `httpserver::compat::auth_handler_v1_ptr`; the
116+
* deprecated @ref create_webserver::auth_handler(compat::auth_handler_v1_ptr)
117+
* overload wraps the callable via @ref compat::adapt_legacy_auth into the
118+
* new @ref auth_handler_ptr shape. Both this alias and the overload emit
119+
* a deprecation diagnostic; they will be removed in the next release.
120+
*
121+
* @deprecated Migrate to @ref auth_handler_ptr returning
122+
* `std::optional<http_response>`.
123+
*/
124+
using auth_handler_v1_ptr
125+
[[deprecated(
126+
"use auth_handler_ptr returning std::optional<http_response>; "
127+
"see RELEASE_NOTES.md 'auth handler migration' (TASK-054)")]]
128+
= std::function<std::shared_ptr<http_response>(const http_request&)>;
129+
130+
/**
131+
* Adapts a v1 auth callable (`std::shared_ptr<http_response>` return)
132+
* into the v2 @ref auth_handler_ptr shape.
133+
*
134+
* The returned wrapper invokes @p legacy; a null `shared_ptr` maps to
135+
* `std::nullopt` (allow); a non-null `shared_ptr` is dereferenced and
136+
* its pointed-to @ref http_response is MOVED into the engaged optional
137+
* (so response state — status, body, headers — is forwarded verbatim).
138+
*
139+
* @deprecated Provided for one transitional build. Migrate the callable's
140+
* return type to `std::optional<http_response>` and pass it
141+
* directly to @ref create_webserver::auth_handler.
142+
*/
143+
[[deprecated(
144+
"v1 shared_ptr<http_response> auth signature is deprecated; "
145+
"migrate to std::optional<http_response> (TASK-054)")]]
146+
inline auth_handler_ptr adapt_legacy_auth(
147+
std::function<std::shared_ptr<http_response>(
148+
const http_request&)> legacy) {
149+
return [legacy = std::move(legacy)](const http_request& req)
150+
-> std::optional<http_response> {
151+
std::shared_ptr<http_response> ptr = legacy(req);
152+
if (ptr == nullptr) return std::nullopt;
153+
return std::optional<http_response>(std::move(*ptr));
154+
};
155+
}
156+
157+
} // namespace compat
101158

102159
/**
103160
* Fluent builder for @ref webserver instances (PRD-NAM-REQ-004,
@@ -363,9 +420,9 @@ class create_webserver {
363420
/**
364421
* Install the centralised auth handler invoked before every
365422
* dispatched request whose path is not on the @ref auth_skip_paths
366-
* list. Returning a non-null `shared_ptr<http_response>` short-
367-
* circuits dispatch and sends that response on the wire
368-
* (typically a 401 or 403).
423+
* list. Returning an engaged `std::optional<http_response>` short-
424+
* circuits dispatch and sends that response on the wire (typically a
425+
* 401 or 403). Returning `std::nullopt` allows the request to proceed.
369426
*
370427
* @note This is an alias. Calling it (with a non-null callable)
371428
* installs a hook at @ref httpserver::hook_phase::before_handler.
@@ -385,10 +442,30 @@ class create_webserver {
385442
* use a wildcard resource registered at "/" or implement the
386443
* check inside the `not_found_handler`.
387444
*
388-
* @param v auth_handler_ptr callback; pass `nullptr` to clear.
445+
* @param v @ref auth_handler_ptr callback; pass `nullptr` to clear.
389446
* @return reference to this builder for chaining.
390447
*/
391448
create_webserver& auth_handler(auth_handler_ptr v) { _auth_handler = std::move(v); return *this; }
449+
450+
/**
451+
* Deprecated v1 overload: accepts a callable returning
452+
* `std::shared_ptr<http_response>`, wraps it via
453+
* @ref compat::adapt_legacy_auth, and stores it in the canonical
454+
* @ref auth_handler_ptr shape. Provided for one transitional build
455+
* so existing v1 call sites get a deprecation warning rather than
456+
* a hard build break.
457+
*
458+
* @deprecated Migrate callables to return
459+
* `std::optional<http_response>` and use the canonical
460+
* @ref auth_handler(auth_handler_ptr) overload.
461+
*/
462+
[[deprecated(
463+
"v1 shared_ptr<http_response> auth signature is deprecated; "
464+
"return std::optional<http_response> instead (TASK-054)")]]
465+
create_webserver& auth_handler(compat::auth_handler_v1_ptr v) {
466+
_auth_handler = compat::adapt_legacy_auth(std::move(v));
467+
return *this;
468+
}
392469
create_webserver& auth_skip_paths(std::vector<std::string> v) { _auth_skip_paths = std::move(v); return *this; }
393470
create_webserver& sni_callback(sni_callback_t v) { _sni_callback = std::move(v); return *this; }
394471

0 commit comments

Comments
 (0)