Skip to content

Latest commit

 

History

History
90 lines (75 loc) · 4.71 KB

File metadata and controls

90 lines (75 loc) · 4.71 KB

DR-009: Handler error-propagation contract

Status: Accepted; revised 2026-05-29 (Revision 1) Date: 2026-04-30 Context: With DR-4 (return-by-value), null-return is impossible. Two cases remain: handler throws, library-internal exception during dispatch.

Options considered:

  1. Any uncaught exception → 500 via internal_error_handler (formalize v1).
  2. Library-defined http_error : std::exception translates to a status — ergonomic; new public API; "two ways to do one thing."
  3. Single http_error{status, body} class only, no hierarchy — small API but same fundamental issue as 2.

Decision: Option 1.

Rationale: With return-by-value cheap, return http_response::empty().with_status(404) is one line — barely longer than throw not_found{}. PRD doesn't ask for throw-as-status. Adding it now creates two ways to express one thing and forces a position on the "exceptions for control flow" debate. Reverse migration (add now, deprecate later) is harder than the forward path (add later if requested).

Consequences:

  • 6-point error-propagation contract documented in §5.2.
  • feature_unavailable (a std::runtime_error) is just another std::exception from the dispatch view; no special status mapping.
  • internal_error_handler is the single user-overridable error escape hatch.

Revision 1 (2026-05-29) — Sanitize the default 500 body

Context: The original decision had internal_error_page surface e.what() in the default-handler 500 body for debugging ergonomics. This was flagged by two security reviewers on TASK-031 (task-031 #3, task-031 #4) and one Pass-1 sweep on TASK-036 (task-036 #44) as a CWE-209 information-disclosure surface: handler exception text routinely embeds file paths, SQL fragments, internal identifiers, and attacker-influenced input that must not cross a process boundary to an untrusted client.

Revised decision: The default body of the no-handler-configured 500 path is the fixed string "Internal Server Error". The originating exception message is still:

  1. Forwarded verbatim to a user-configured internal_error_handler (if one is wired) — that handler can render any body it wants.
  2. Forwarded verbatim to the user-configured log_error callback (the server-side log).

Only the default HTTP response body is sanitized; everything else is unchanged. A new opt-in builder flag, create_webserver::expose_exception_messages(bool), restores the pre-revision behaviour of including the message in the default body. The flag is documented as development-only (see the @warning block on the setter).

Contract points 1–6 in §5.2 stand; only the default-body clause in point 1 is amended.

Consequences (Revision 1):

  • The regression tests dr009_runtime_error_message_surfaces_in_default_body and dr009_non_std_exception_yields_unknown_exception_in_default_body are renamed/split into a "fixed body" pair (the new contract) plus a "verbose body" pair (the opt-in round-trip). See test/integ/basic.cpp.
  • The legacy integration tests in basic_suite that pre-date this revision and assert on the message-in-body (exception_forces_500, untyped_error_forces_500, file_serving_resource_missing, file_serving_resource_dir, long_error_message, dr009_feature_unavailable_lands_as_generic_500) opt the suite-shared ws into the verbose mode via .expose_exception_messages(true) so their original assertion intent — "the dispatcher's message-forwarding path works" — is preserved without diluting coverage.
  • No request-id concept is introduced as part of this revision. The original action item mentioned "with the request id (if any) appended"; the codebase has no request-id plumbing today and adding one is out of scope for a security-fix revision. The fixed-body alone satisfies CWE-209. A request-id concept can be added later without re-opening this decision.
  • log_dispatch_error continues to log e.what() verbatim regardless of the flag. The error log is the canonical destination for the verbatim exception text. Handlers that may throw exceptions containing sensitive data (DB URLs, credentials, attacker-influenced input) SHOULD catch and sanitize the message before re-throwing if those values must not appear in the server log either; see the Doxygen note on webserver_impl::log_dispatch_error.
  • ABI: this revision is shipping on feature/v2.0 before v2.0 cuts; SOVERSION is already 2 and there is no released ABI to preserve. The added const bool expose_exception_messages on webserver and the bool _expose_exception_messages on create_webserver recompile cleanly for any consumer TU.

Related findings: task-031 #3, task-031 #4, task-036 #44, task-034 #24.