TASK-013: review-pass fixes (DRY forbidden-char constant, dispatch null-safety, digest-test contract)

- Reuse anonymous-namespace kForbiddenFieldChars in unauthorized() instead
  of the duplicate local kForbidden (code-quality-iter1-8, DRY).
- Move the operator<< friend declaration out of the now-misleading
  protected: section in http_response.hpp; http_response is final, so the
  section was never reachable by subclasses.
- Collapse the duplicate catch(const std::exception&)/catch(...) block in
  finalize_answer to a single catch(...). Both branches did the same work.
- Add LT_CHECK_EQ(http_code, 401) to the canonical digest_auth integration
  test and TODO(v2-digest) markers to the four sibling digest_auth_*
  variants documenting why they are currently behaviourally redundant
  under the v2 static-challenge path (test-quality-iter1-10/28).
- Migrate examples/service.cpp render_* methods from
  shared_ptr(new http_response(...)) to make_shared<http_response>(...)
  (code-quality-iter1-6, idiomatic v2 form).
- Add iovec_factory_empty_span and iovec_factory_single_entry edge-case
  tests, with an expanded comment block on iovec_factory_deep_copies_span
  spelling out the span deep-copy / caller-buffer lifetime contract
  (test-quality-iter1-29).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: etr

examples/service.cpp

@@ -52,11 +52,11 @@ std::shared_ptr<httpserver::http_response> service_resource::render_GET(const ht
     std::cout << "service_resource::render_GET()" << std::endl;
 
     if (verbose) std::cout << req;
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("GET response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("GET response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 
@@ -65,83 +65,83 @@ std::shared_ptr<httpserver::http_response> service_resource::render_PUT(const ht
 
     if (verbose) std::cout << req;
 
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("PUT response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("PUT response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 std::shared_ptr<httpserver::http_response> service_resource::render_POST(const httpserver::http_request &req) {
     std::cout << "service_resource::render_POST()" << std::endl;
 
     if (verbose) std::cout << req;
 
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("POST response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("POST response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 std::shared_ptr<httpserver::http_response> service_resource::render(const httpserver::http_request &req) {
     std::cout << "service_resource::render()" << std::endl;
 
     if (verbose) std::cout << req;
 
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("generic response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("generic response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 std::shared_ptr<httpserver::http_response> service_resource::render_HEAD(const httpserver::http_request &req) {
     std::cout << "service_resource::render_HEAD()" << std::endl;
 
     if (verbose) std::cout << req;
 
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("HEAD response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("HEAD response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 std::shared_ptr<httpserver::http_response> service_resource::render_OPTIONS(const httpserver::http_request &req) {
     std::cout << "service_resource::render_OPTIONS()" << std::endl;
 
     if (verbose) std::cout << req;
 
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("OPTIONS response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("OPTIONS response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 std::shared_ptr<httpserver::http_response> service_resource::render_CONNECT(const httpserver::http_request &req) {
     std::cout << "service_resource::render_CONNECT()" << std::endl;
 
     if (verbose) std::cout << req;
 
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("CONNECT response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("CONNECT response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 std::shared_ptr<httpserver::http_response> service_resource::render_DELETE(const httpserver::http_request &req) {
     std::cout << "service_resource::render_DELETE()" << std::endl;
 
     if (verbose) std::cout << req;
 
-    httpserver::http_response* res = new httpserver::http_response(httpserver::http_response::string("DELETE response"));
+    auto res = std::make_shared<httpserver::http_response>(httpserver::http_response::string("DELETE response"));
 
     if (verbose) std::cout << *res;
 
-    return std::shared_ptr<httpserver::http_response>(res);
+    return res;
 }
 
 void usage() {

src/http_response.cpp

@@ -311,7 +311,7 @@ std::string_view http_response::get_cookie(std::string_view key) const {
 }
 
 namespace {
-static inline http::header_view_map to_view_map(const http::header_map& hdr_map) {
+inline http::header_view_map to_view_map(const http::header_map& hdr_map) {
     http::header_view_map view_map;
     for (const auto& item : hdr_map) {
         view_map[std::string_view(item.first)] = std::string_view(item.second);
@@ -447,13 +447,14 @@ http_response http_response::unauthorized(std::string_view scheme,
     // caller error — callers must never pass untrusted user input as scheme
     // or realm without first validating it. Throw std::invalid_argument so
     // the error is visible and cannot be silently swallowed.
-    static constexpr std::string_view kForbidden("\r\n\0", 3);
-    if (scheme.find_first_of(kForbidden) != std::string_view::npos) {
+    // kForbiddenFieldChars is the same constant used by validate_header_field
+    // above — reused here to avoid a duplicate definition.
+    if (scheme.find_first_of(kForbiddenFieldChars) != std::string_view::npos) {
         throw std::invalid_argument(
             "http_response::unauthorized: scheme contains forbidden control "
             "character (CR, LF, or NUL)");
     }
-    if (realm.find_first_of(kForbidden) != std::string_view::npos) {
+    if (realm.find_first_of(kForbiddenFieldChars) != std::string_view::npos) {
         throw std::invalid_argument(
             "http_response::unauthorized: realm contains forbidden control "
             "character (CR, LF, or NUL)");

src/httpserver/http_response.hpp

@@ -367,7 +367,11 @@ class http_response final {
      template <typename T, typename... Args>
      void emplace_body(body_kind k, Args&&... args);
 
- protected:
+     // Friend declarations belong in private: — friendship is unaffected
+     // by access specifiers, but placing them here (rather than in a
+     // misleading protected: section) signals clearly that these names can
+     // bypass encapsulation; http_response is final so there are no
+     // subclasses to inherit any protected access anyway.
      friend std::ostream &operator<< (std::ostream &os, const http_response &r);
 
      // The TASK-009 SBO unit test exercises the four-case move

src/webserver.cpp

@@ -1389,7 +1389,7 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct detail:
                     mr->dhrs->with_header(http_utils::http_header_allow, header_value);
                 }
             }
-        } catch(const std::exception& e) {
+        } catch(...) {
             // The user-supplied internal_error_resource may itself throw;
             // fall back to the built-in error page in that case (force_our=true)
             // so we never let exceptions escape into libmicrohttpd.
@@ -1398,12 +1398,6 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct detail:
             } catch(...) {
                 mr->dhrs = internal_error_page(mr, true);
             }
-        } catch(...) {
-            try {
-                mr->dhrs = internal_error_page(mr);
-            } catch(...) {
-                mr->dhrs = internal_error_page(mr, true);
-            }
         }
     } else if (mr->dhrs == nullptr) {
         mr->dhrs = not_found_page(mr);

test/integ/authentication.cpp

@@ -240,6 +240,7 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth)
     std::string s;
     CURL *curl = curl_easy_init();
     CURLcode res;
+    long http_code = 0;  // NOLINT(runtime/int)
     curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_DIGEST);
 #if defined(_WINDOWS)
     curl_easy_setopt(curl, CURLOPT_USERPWD, "examplerealm/myuser:mypass");
@@ -256,13 +257,23 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth)
     curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
     res = curl_easy_perform(curl);
     LT_ASSERT_EQ(res, 0);
-    // v2 limitation: digest handshake does not complete — body remains FAIL.
+    curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
+    // v2 contract: the server issues a static 401 Digest challenge; the
+    // handshake cannot complete (no nonce/opaque state machine), so the
+    // body remains FAIL and the status must be 401.
+    LT_CHECK_EQ(http_code, 401);
     LT_CHECK_EQ(s, "FAIL");
     curl_easy_cleanup(curl);
 
     ws.stop();
 LT_END_AUTO_TEST(digest_auth)
 
+// TODO(v2-digest): digest_auth_wrong_pass is indistinguishable from digest_auth
+// under v2 because the handshake never completes regardless of credentials.
+// Wrong-pass vs. correct-pass both reach the same static 401 challenge path.
+// This test becomes meaningful again when full v2 digest support is added
+// (MHD nonce/opaque state machine).  Until then it exercises a different
+// digest_resource instance only.
 LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_wrong_pass)
     webserver ws = create_webserver(PORT)
         .digest_auth_random("myrandom")
@@ -303,6 +314,10 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_wrong_pass)
     ws.stop();
 LT_END_AUTO_TEST(digest_auth_wrong_pass)
 
+// TODO(v2-digest): digest_auth_with_ha1_md5 is indistinguishable from
+// digest_auth under v2 — check_digest_auth_digest() is never reached because
+// get_digested_user() always returns empty string (no nonce roundtrip).
+// This test becomes meaningful when full v2 digest support is added.
 LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5)
     webserver ws = create_webserver(PORT)
         .digest_auth_random("myrandom")
@@ -345,6 +360,9 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5)
     ws.stop();
 LT_END_AUTO_TEST(digest_auth_with_ha1_md5)
 
+// TODO(v2-digest): digest_auth_with_ha1_md5_wrong_pass is indistinguishable
+// from digest_auth_with_ha1_md5 under v2 — wrong-pass vs. correct-pass both
+// yield the same static 401 challenge. Becomes meaningful with full v2 digest.
 LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5_wrong_pass)
     webserver ws = create_webserver(PORT)
         .digest_auth_random("myrandom")
@@ -385,6 +403,9 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5_wrong_pass)
     ws.stop();
 LT_END_AUTO_TEST(digest_auth_with_ha1_md5_wrong_pass)
 
+// TODO(v2-digest): digest_auth_with_ha1_sha256 is indistinguishable from
+// digest_auth under v2 — SHA-256 vs. MD5 path is unreachable (no nonce
+// roundtrip). Becomes meaningful when full v2 digest support is added.
 LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256)
     webserver ws = create_webserver(PORT)
         .digest_auth_random("myrandom")
@@ -427,6 +448,10 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256)
     ws.stop();
 LT_END_AUTO_TEST(digest_auth_with_ha1_sha256)
 
+// TODO(v2-digest): digest_auth_with_ha1_sha256_wrong_pass is
+// indistinguishable from digest_auth_with_ha1_sha256 under v2 — wrong-pass
+// vs. correct-pass both yield the same static 401 challenge. Becomes
+// meaningful when full v2 digest support is added.
 LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256_wrong_pass)
     webserver ws = create_webserver(PORT)
         .digest_auth_random("myrandom")

test/unit/http_response_factories_test.cpp

@@ -185,6 +185,16 @@ LT_BEGIN_AUTO_TEST(http_response_factories_suite,
     // Build a span over a temporary array; let the array go out of
     // scope before we observe r. The factory's deep-copy must keep the
     // body's iovec_entry vector valid.
+    //
+    // Span deep-copy / caller-buffer lifetime contract:
+    //   http_response::iovec() copies the iovec_entry structs (base+len
+    //   pairs) into an internal std::vector.  The *entries* are owned by
+    //   the response, but the *buffers they point to* are NOT copied —
+    //   callers must keep their payload buffers alive until the response
+    //   is dispatched (i.e. until the MHD send callback completes).
+    //   http_response itself is move-only, so copy-prohibition does not
+    //   apply; the invariant to test is that the internal vector survives
+    //   after the caller's span goes out of scope.
     auto r = []() {
         std::array<httpserver::iovec_entry, 1> entries{{ {"x", 1} }};
         return http_response::iovec(entries);
@@ -193,6 +203,26 @@ LT_BEGIN_AUTO_TEST(http_response_factories_suite,
                 static_cast<int>(body_kind::iovec));
 LT_END_AUTO_TEST(iovec_factory_deep_copies_span)
 
+LT_BEGIN_AUTO_TEST(http_response_factories_suite, iovec_factory_empty_span)
+    // An iovec with zero entries must not crash, must have kind iovec,
+    // and the default status (200) must be preserved.
+    std::array<httpserver::iovec_entry, 0> entries{};
+    auto r = http_response::iovec(std::span<const httpserver::iovec_entry>(entries));
+    LT_CHECK_EQ(static_cast<int>(r.kind()),
+                static_cast<int>(body_kind::iovec));
+    LT_CHECK_EQ(r.get_status(), 200);
+LT_END_AUTO_TEST(iovec_factory_empty_span)
+
+LT_BEGIN_AUTO_TEST(http_response_factories_suite, iovec_factory_single_entry)
+    // A single-entry iovec must produce kind iovec and the default status.
+    static const char buf[] = "hello";
+    std::array<httpserver::iovec_entry, 1> entries{{ {buf, 5} }};
+    auto r = http_response::iovec(entries);
+    LT_CHECK_EQ(static_cast<int>(r.kind()),
+                static_cast<int>(body_kind::iovec));
+    LT_CHECK_EQ(r.get_status(), 200);
+LT_END_AUTO_TEST(iovec_factory_single_entry)
+
 // -----------------------------------------------------------------------
 // pipe() — owns the fd, destructor closes it when not materialized.
 // Gated on !_WIN32 because Windows uses _pipe()/CreatePipe() rather