Merge TASK-012: http_response fluent with_* setters

Author: etr

specs/tasks/M2-response/TASK-012.md

@@ -8,11 +8,11 @@
 Make `with_header` / `with_footer` / `with_cookie` / `with_status` return `http_response&` so factory chains work.
 
 **Action Items:**
-- [ ] `http_response& with_header(std::string key, std::string value) &;`
-- [ ] `http_response&& with_header(std::string key, std::string value) &&;` (rvalue overload to keep `http_response::string("hi").with_header(...)` zero-copy).
-- [ ] Same pattern for `with_footer`, `with_cookie`, `with_status(int code)`.
-- [ ] Cookie API takes a structured cookie type (name, value, attrs) or string-as-Set-Cookie; pick one and document.
-- [ ] Update v1 callers: `r.with_header(...)` chains now compile; previous `void`-returning calls still work (statement form is fine) but enable the fluent style.
+- [x] `http_response& with_header(std::string key, std::string value) &;`
+- [x] `http_response&& with_header(std::string key, std::string value) &&;` (rvalue overload to keep `http_response::string("hi").with_header(...)` zero-copy).
+- [x] Same pattern for `with_footer`, `with_cookie`, `with_status(int code)`.
+- [x] Cookie API takes a structured cookie type (name, value, attrs) or string-as-Set-Cookie; pick one and document. (Decision: keep v1 string-pair `(name, value)`; structured cookie type deferred to a follow-up task. Documented on `with_cookie` Doxygen.)
+- [x] Update v1 callers: `r.with_header(...)` chains now compile; previous `void`-returning calls still work (statement form is fine) but enable the fluent style.
 
 **Dependencies:**
 - Blocked by: TASK-009
@@ -26,4 +26,4 @@ Make `with_header` / `with_footer` / `with_cookie` / `with_status` return `http_
 **Related Requirements:** PRD-RSP-REQ-004
 **Related Decisions:** §4.3
 
-**Status:** Not Started
+**Status:** Done

specs/tasks/_index.md

@@ -94,7 +94,7 @@ Nominally: **13 sequential tasks**, each S–XL. Most other tasks parallelize of
 | TASK-009 | `http_response` value type with SBO buffer | M2 | Done | TASK-008 |
 | TASK-010 | `http_response` factory functions | M2 | Done | TASK-008, TASK-009, TASK-004 |
 | TASK-011 | `http_response` const-correct accessors | M2 | Done | TASK-009 |
-| TASK-012 | `http_response` fluent `with_*` setters | M2 | Not Started | TASK-009 |
+| TASK-012 | `http_response` fluent `with_*` setters | M2 | Done | TASK-009 |
 | TASK-013 | Remove `*_response` subclasses and dispatch virtuals | M2 | Not Started | TASK-009, TASK-010, TASK-011, TASK-012 |
 | TASK-014 | `webserver_impl` skeleton (PIMPL prep) | M3 | Not Started | TASK-002 |
 | TASK-015 | `http_request_impl` skeleton (PIMPL split) | M3 | Not Started | TASK-002, TASK-014 |

specs/unworked_review_issues/2026-05-03_222849_task-012.md

@@ -189,6 +189,119 @@ void http_response::shoutCAST() {
     status_code_ |= http::http_utils::shoutcast_response;
 }
 
+// -----------------------------------------------------------------------
+// Fluent with_* setters (TASK-012, PRD-RSP-REQ-004).
+//
+// Each setter has two ref-qualified overloads that delegate to a private
+// do_set_*() helper containing the validation + mutation logic. The
+// overloads differ only in their return statement: `& overload` returns
+// *this by lvalue reference; `&& overload` returns std::move(*this).
+// Centralising the mutation in a single helper means validation and
+// insert_or_assign are in exactly one place per setter, not duplicated
+// across every overload pair.
+//
+// Validation (security, TASK-012 review-pass):
+//   * with_header / with_footer: reject key or value containing CR,
+//     LF, or NUL — these characters can split an HTTP response and
+//     inject additional headers (CWE-113).
+//   * with_cookie: same CRLF/NUL rejection on name and value.
+//   * with_status: code must be in [100, 599] per RFC 9110 §15.
+//
+// insert_or_assign — rather than `m[k] = v` — is used so the by-value
+// `std::string` parameters can be moved into the map slot directly.
+// -----------------------------------------------------------------------
+
+// Shared forbidden-character set for header/footer/cookie field names
+// and values. The string_view spans all three bytes including the
+// embedded NUL.
+namespace {
+constexpr std::string_view kForbiddenFieldChars("\r\n\0", 3);
+
+void validate_header_field(std::string_view context,
+                           std::string_view key,
+                           std::string_view value) {
+    if (key.find_first_of(kForbiddenFieldChars) != std::string_view::npos) {
+        throw std::invalid_argument(
+            std::string(context) +
+            ": key contains forbidden control character (CR, LF, or NUL)");
+    }
+    if (value.find_first_of(kForbiddenFieldChars) != std::string_view::npos) {
+        throw std::invalid_argument(
+            std::string(context) +
+            ": value contains forbidden control character (CR, LF, or NUL)");
+    }
+}
+}  // namespace
+
+void http_response::do_set_header(std::string key, std::string value) {
+    validate_header_field("with_header", key, value);
+    headers_.insert_or_assign(std::move(key), std::move(value));
+}
+
+void http_response::do_set_footer(std::string key, std::string value) {
+    validate_header_field("with_footer", key, value);
+    footers_.insert_or_assign(std::move(key), std::move(value));
+}
+
+void http_response::do_set_cookie(std::string key, std::string value) {
+    validate_header_field("with_cookie", key, value);
+    cookies_.insert_or_assign(std::move(key), std::move(value));
+}
+
+void http_response::do_set_status(int code) {
+    if (code < 100 || code > 599) {
+        throw std::invalid_argument(
+            "with_status: HTTP status code out of range [100, 599]");
+    }
+    status_code_ = code;
+}
+
+http_response& http_response::with_header(std::string key,
+                                          std::string value) & {
+    do_set_header(std::move(key), std::move(value));
+    return *this;
+}
+
+http_response&& http_response::with_header(std::string key,
+                                           std::string value) && {
+    do_set_header(std::move(key), std::move(value));
+    return std::move(*this);
+}
+
+http_response& http_response::with_footer(std::string key,
+                                          std::string value) & {
+    do_set_footer(std::move(key), std::move(value));
+    return *this;
+}
+
+http_response&& http_response::with_footer(std::string key,
+                                           std::string value) && {
+    do_set_footer(std::move(key), std::move(value));
+    return std::move(*this);
+}
+
+http_response& http_response::with_cookie(std::string key,
+                                          std::string value) & {
+    do_set_cookie(std::move(key), std::move(value));
+    return *this;
+}
+
+http_response&& http_response::with_cookie(std::string key,
+                                           std::string value) && {
+    do_set_cookie(std::move(key), std::move(value));
+    return std::move(*this);
+}
+
+http_response& http_response::with_status(int code) & {
+    do_set_status(code);
+    return *this;
+}
+
+http_response&& http_response::with_status(int code) && {
+    do_set_status(code);
+    return std::move(*this);
+}
+
 // -----------------------------------------------------------------------
 // Const single-key accessors (TASK-011).
 //

src/http_response.cpp

@@ -267,17 +267,60 @@ class http_response {
          return status_code_;
      }
 
-     void with_header(const std::string& key, const std::string& value) {
-         headers_[key] = value;
-     }
+     // ------------------------------------------------------------------
+     // Fluent setters (TASK-012, PRD-RSP-REQ-004).
+     //
+     // Each setter is overloaded on the value-category of *this so that
+     // both lvalue and rvalue (factory) chains keep the response live
+     // and zero-copy:
+     //
+     //   * The `&` overload returns http_response& so that
+     //         r.with_header(k, v).with_status(s);
+     //     compiles and returns *this when `r` is an lvalue.
+     //   * The `&&` overload returns http_response&& so that
+     //         http_response::string("hi").with_header(...).with_status(...)
+     //     keeps the temporary as an rvalue end-to-end; the chain calls
+     //     successive `&&` overloads on the same SBO-inline body without
+     //     any intermediate move-construction or heap relocation.
+     //
+     // String parameters are taken by value: the body internally moves
+     // them into the underlying header/footer/cookie maps via
+     // insert_or_assign, so callers can either copy or move into the
+     // setter without an extra allocation.
+     //
+     // Backward compatibility (constraint): pre-TASK-012 callers wrote
+     //         r.with_header(k, v);
+     // in statement form, discarding the (then `void`) return. Switching
+     // the return type to a non-`[[nodiscard]]` reference is strictly
+     // source-compatible — the reference is silently ignored.
+     //
+     // Cookie API decision (action item #4 of TASK-012): the v2.0 cookie
+     // surface is the v1 (name, value) string-pair shape. `with_cookie`
+     // overwrites any prior entry for `name` (the cookie map is keyed
+     // case-insensitively). The value is rendered verbatim into the
+     // `Set-Cookie` header by decorate_response, so callers who need
+     // attributes (Path, Secure, HttpOnly, SameSite, ...) pre-format the
+     // value, e.g. with_cookie("sid", "abc; Path=/; Secure; HttpOnly").
+     // A structured cookie type with first-class attribute fields is
+     // intentionally deferred to a follow-up task; it can be added as a
+     // non-breaking overload alongside this string-pair API.
+     //
+     // Note on with_status: status replaces the stored code outright,
+     // including any flag bits set by shoutCAST() (which ORs
+     // MHD_ICY_FLAG into status_code_). Callers wanting both write
+     // with_status(...) first and shoutCAST() second.
+     // ------------------------------------------------------------------
+     http_response& with_header(std::string key, std::string value) &;
+     http_response&& with_header(std::string key, std::string value) &&;
 
-     void with_footer(const std::string& key, const std::string& value) {
-         footers_[key] = value;
-     }
+     http_response& with_footer(std::string key, std::string value) &;
+     http_response&& with_footer(std::string key, std::string value) &&;
 
-     void with_cookie(const std::string& key, const std::string& value) {
-         cookies_[key] = value;
-     }
+     http_response& with_cookie(std::string key, std::string value) &;
+     http_response&& with_cookie(std::string key, std::string value) &&;
+
+     http_response& with_status(int code) &;
+     http_response&& with_status(int code) &&;
 
      void shoutCAST();
 
@@ -310,6 +353,16 @@ class http_response {
      void destroy_body() noexcept;
      void adopt_body_from(http_response& o) noexcept;
 
+     // Shared mutation helpers for the fluent setters (TASK-012
+     // review-pass). Each helper validates its inputs, then performs the
+     // map mutation or scalar assignment.  Centralising the logic here
+     // means the & and && overloads only differ in their return
+     // statement; the mutation + validation is in exactly one place.
+     void do_set_header(std::string key, std::string value);
+     void do_set_footer(std::string key, std::string value);
+     void do_set_cookie(std::string key, std::string value);
+     void do_set_status(int code);
+
      // Placement-new a concrete detail::body subclass into the SBO
      // buffer (or, if T does not fit, onto the heap via the matched
      // ::operator new(sizeof(T))/::operator delete pairing the

src/httpserver/http_response.hpp

@@ -440,6 +440,24 @@ LT_BEGIN_AUTO_TEST(http_response_factories_suite,
     LT_CHECK_EQ(other.get_response_code(), 200);
 LT_END_AUTO_TEST(factory_move_preserves_kind_and_headers)
 
+// -----------------------------------------------------------------------
+// TASK-012 zero-copy invariant: chained with_* calls on a factory's
+// rvalue must not perturb the SBO placement. http_response::string(...)
+// places a string_body inline in the SBO buffer; the && overloads of
+// with_header / with_status return http_response&& (i.e. propagate the
+// xvalue without an intermediate copy/move-construct), so the body
+// pointer must remain inline through the chain.
+// -----------------------------------------------------------------------
+LT_BEGIN_AUTO_TEST(http_response_factories_suite,
+                   factory_chain_keeps_body_inline_in_sbo)
+    auto r = http_response::string("hi")
+                 .with_header("X-Foo", "bar")
+                 .with_status(201);
+    LT_CHECK_EQ(SBO::body_inline(r), true);
+    LT_CHECK_EQ(static_cast<int>(SBO::kind(r)),
+                static_cast<int>(body_kind::string));
+LT_END_AUTO_TEST(factory_chain_keeps_body_inline_in_sbo)
+
 LT_BEGIN_AUTO_TEST_ENV()
     AUTORUN_TESTS()
 LT_END_AUTO_TEST_ENV()