Skip to content

Commit a11e5ee

Browse files
etrclaude
andcommitted
TASK-036 review: document auth_handler_ptr deferral (findings 4 & 7)
- Add a TODO comment on auth_handler_ptr in create_webserver.hpp documenting the deferred migration to std::optional<http_response> (findings 4 & 7: code-quality-reviewer + code-simplifier). Changing the public typedef mid-milestone would cascade into centralized_authentication.cpp and authentication.cpp; scoped out. - Rename local variable auth_resp -> auth_rejection_response in the auth_handler alias hook (webserver_aliases.cpp) to clarify that null means "allow" and non-null means "reject with this response" — the minimum improvement from finding 7 recommendation (b). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5d49d42 commit a11e5ee

2 files changed

Lines changed: 20 additions & 5 deletions

File tree

src/detail/webserver_aliases.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,14 +194,20 @@ void webserver::install_default_alias_hooks_() {
194194
if (impl_ptr->should_skip_auth(path)) {
195195
return hook_action::pass();
196196
}
197-
// Call the user-supplied auth_handler.
198-
std::shared_ptr<http_response> auth_resp =
197+
// Call the user-supplied auth_handler. Returns nullptr
198+
// to mean "allow" (pass-through); non-null means
199+
// "reject with this response". The shared_ptr<> is the
200+
// v1 nullable-optional pattern; see the TODO on
201+
// auth_handler_ptr in create_webserver.hpp for the
202+
// deferred migration plan.
203+
std::shared_ptr<http_response> auth_rejection_response =
199204
ws_ptr->auth_handler(*ctx.request);
200-
if (auth_resp == nullptr) {
205+
if (auth_rejection_response == nullptr) {
201206
return hook_action::pass();
202207
}
203-
// Auth failed: short-circuit with the auth response.
204-
return hook_action::respond_with(std::move(*auth_resp));
208+
// Auth failed: short-circuit with the rejection response.
209+
return hook_action::respond_with(
210+
std::move(*auth_rejection_response));
205211
})))
206212
.detach();
207213
}

src/httpserver/create_webserver.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ typedef std::function<std::pair<std::string, std::string>(const std::string& ser
7878
namespace http { class file_info; }
7979

8080
typedef std::function<bool(const std::string&, const std::string&, const http::file_info&)> file_cleanup_callback_ptr;
81+
82+
// TODO(follow-up): migrate auth_handler_ptr to
83+
// std::function<std::optional<http_response>(const http_request&)>
84+
// to complete the DR-004 value-return rollout to the auth hook (removing
85+
// the per-authenticated-request heap allocation for the shared_ptr control
86+
// block). This requires updating the call site in webserver_finalize.cpp,
87+
// the example in examples/centralized_authentication.cpp, and the
88+
// integration test in test/integ/authentication.cpp. Deferred from TASK-036
89+
// to avoid a cascading public-API break mid-milestone.
8190
typedef std::function<std::shared_ptr<http_response>(const http_request&)> auth_handler_ptr;
8291

8392
/**

0 commit comments

Comments
 (0)