refactor(dr-014): extract upload_pipeline behavior service

Move multipart / file-upload handling off webserver_impl into
detail::upload_pipeline: handle_post_form_arg, setup_new_upload_file_info,
manage_upload_stream, process_file_upload, and the file branch of
post_iterator (now iterate_file). Holds only const webserver_config&
(file_upload_target / file_upload_dir / generate_random_filename_on_upload).

post_iterator stays a static webserver_impl trampoline (its address is
registered with MHD_create_post_processor) and forwards here: the no-file
form-arg branch calls the static upload_pipeline::handle_post_form_arg so
it works without an owning webserver (post_iterator_null_key_test feeds a
null-ws modded_request), the file branch routes through impl_->upload_.

upload_pipeline is granted friend access to http_request (set_arg /
grow_last_arg / set_arg_flat) and http::file_info (the setters +
grow_file_size), matching webserver_impl's prior access.

113/113 tests pass; cpplint/complexity/file-size gates green.

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

Author: etr

src/Makefile.am

@@ -25,11 +25,11 @@ lib_LTLIBRARIES = libhttpserver.la
 # builds. The WS-off branch in websocket_handler.cpp provides stub
 # definitions (every member throws feature_unavailable except is_valid()
 # which returns false).
-libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp webserver_add_hook.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp http_response_factories.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp peer_address.cpp resource_hook_table.cpp cookie.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/ip_access_control.cpp detail/ws_registry.cpp detail/hook_bus.cpp detail/route_table.cpp detail/daemon_lifecycle.cpp detail/dispatch_util.cpp detail/error_pages.cpp detail/hook_dispatcher.cpp detail/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.cpp detail/response_materializer.cpp detail/webserver_lifecycle.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_routes_upsert.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_response_queue.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_hook_firing.cpp detail/hook_phase_dispatch.cpp
+libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp webserver_add_hook.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp http_response_factories.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp peer_address.cpp resource_hook_table.cpp cookie.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/ip_access_control.cpp detail/ws_registry.cpp detail/hook_bus.cpp detail/route_table.cpp detail/daemon_lifecycle.cpp detail/dispatch_util.cpp detail/error_pages.cpp detail/hook_dispatcher.cpp detail/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.cpp detail/response_materializer.cpp detail/upload_pipeline.cpp detail/webserver_lifecycle.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_routes_upsert.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_response_queue.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_hook_firing.cpp detail/hook_phase_dispatch.cpp
 # noinst_HEADERS: shipped in the tarball but NEVER installed under $prefix/include.
 # Detail headers (httpserver/detail/*.hpp) live here so they cannot leak to
 # downstream consumers — the public surface comes in through <httpserver.hpp>.
-noinst_HEADERS = httpserver/string_utilities.hpp httpserver/detail/modded_request.hpp httpserver/detail/http_endpoint.hpp httpserver/detail/body.hpp httpserver/detail/webserver_impl.hpp httpserver/detail/webserver_impl_dispatch.hpp httpserver/detail/connection_state.hpp httpserver/detail/ip_access_control.hpp httpserver/detail/ws_registry.hpp httpserver/detail/hook_bus.hpp httpserver/detail/route_table.hpp httpserver/detail/daemon_lifecycle.hpp httpserver/detail/dispatch_util.hpp httpserver/detail/error_pages.hpp httpserver/detail/hook_dispatcher.hpp httpserver/detail/response_materializer.hpp httpserver/detail/secure_zero.hpp httpserver/detail/http_request_impl.hpp httpserver/detail/resource_hook_table.hpp httpserver/detail/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/segment_trie.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp httpserver/detail/unescape_helpers.hpp gettext.h
+noinst_HEADERS = httpserver/string_utilities.hpp httpserver/detail/modded_request.hpp httpserver/detail/http_endpoint.hpp httpserver/detail/body.hpp httpserver/detail/webserver_impl.hpp httpserver/detail/webserver_impl_dispatch.hpp httpserver/detail/connection_state.hpp httpserver/detail/ip_access_control.hpp httpserver/detail/ws_registry.hpp httpserver/detail/hook_bus.hpp httpserver/detail/route_table.hpp httpserver/detail/daemon_lifecycle.hpp httpserver/detail/dispatch_util.hpp httpserver/detail/error_pages.hpp httpserver/detail/hook_dispatcher.hpp httpserver/detail/response_materializer.hpp httpserver/detail/upload_pipeline.hpp httpserver/detail/secure_zero.hpp httpserver/detail/http_request_impl.hpp httpserver/detail/resource_hook_table.hpp httpserver/detail/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/segment_trie.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp httpserver/detail/unescape_helpers.hpp gettext.h
 nobase_include_HEADERS = httpserver.hpp httpserver/body_kind.hpp httpserver/cookie.hpp httpserver/constants.hpp httpserver/create_webserver.hpp httpserver/create_webserver_setters.hpp httpserver/create_test_request.hpp httpserver/webserver.hpp httpserver/webserver_routes.hpp httpserver/webserver_runtime.hpp httpserver/webserver_websocket.hpp httpserver/webserver_hooks.hpp httpserver/websocket_handler.hpp httpserver/http_utils.hpp httpserver/http_utils_helpers.hpp httpserver/ip_representation.hpp httpserver/file_info.hpp httpserver/http_request.hpp httpserver/http_response.hpp httpserver/http_resource.hpp httpserver/feature_unavailable.hpp httpserver/iovec_entry.hpp httpserver/http_arg_value.hpp httpserver/http_method.hpp httpserver/hook_phase.hpp httpserver/hook_action.hpp httpserver/hook_handle.hpp httpserver/hook_context.hpp
 
 AM_CXXFLAGS += -fPIC -Wall

src/detail/upload_pipeline.cpp

@@ -0,0 +1,156 @@
+/*
+     This file is part of libhttpserver
+     Copyright (C) 2011-2026 Sebastiano Merlino
+
+     This library is free software; you can redistribute it and/or
+     modify it under the terms of the GNU Lesser General Public
+     License as published by the Free Software Foundation; either
+     version 2.1 of the License, or (at your option) any later version.
+
+     This library is distributed in the hope that it will be useful,
+     but WITHOUT ANY WARRANTY; without even the implied warranty of
+     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+     Lesser General Public License for more details.
+
+     You should have received a copy of the GNU Lesser General Public
+     License along with this library; if not, write to the Free Software
+     Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301
+     USA
+*/
+
+// upload_pipeline behavior service (DR-014 §4.11). Logic moved verbatim out
+// of detail/webserver_callbacks.cpp (handle_post_form_arg /
+// setup_new_upload_file_info / manage_upload_stream / process_file_upload and
+// the file branch of post_iterator, here iterate_file). The post_iterator
+// static MHD trampoline stays on webserver_impl and forwards here. Rewiring:
+// parent->config.* becomes config_.*.
+
+#include "httpserver/detail/upload_pipeline.hpp"
+
+#include <microhttpd.h>
+#include <unistd.h>
+
+#include <cstring>
+#include <fstream>
+#include <memory>
+#include <string>
+
+#include "httpserver/create_webserver.hpp"
+#include "httpserver/file_info.hpp"
+#include "httpserver/http_request.hpp"
+#include "httpserver/http_utils.hpp"
+#include "httpserver/detail/modded_request.hpp"
+
+namespace httpserver {
+
+using httpserver::http::http_utils;
+
+namespace detail {
+
+MHD_Result upload_pipeline::handle_post_form_arg(detail::modded_request* mr,
+        const char* key, const char* data, size_t size, uint64_t off) {
+    // MHD may invoke the post iterator with a null key on a continuation
+    // chunk (off > 0): the field name was supplied on the first call and is
+    // not repeated. With no field name there is nothing to store the value
+    // under, so silently accept the chunk (MHD_YES; MHD_NO would abort the
+    // request). Guarding here also stops the raw pointer from reaching
+    // std::string, which throws std::logic_error on null and aborts via
+    // std::terminate (the throw escapes a C callback). See issue #375.
+    if (key == nullptr) {
+        return MHD_YES;
+    }
+    // A non-zero @p off means MHD is feeding a continuation chunk of a
+    // previously-started value, so append rather than replace.
+    if (off > 0) {
+        mr->request->grow_last_arg(key, std::string(data, size));
+    } else {
+        mr->request->set_arg(key, std::string(data, size));
+    }
+    return MHD_YES;
+}
+
+bool upload_pipeline::setup_new_upload_file_info(http::file_info& file,
+        const char* filename, const char* content_type,
+        const char* transfer_encoding) const {
+    // First chunk for this (key, filename) pair: choose the on-disk
+    // destination path (random if generate_random_filename_on_upload,
+    // otherwise sanitize the client-supplied filename) and prime the
+    // file_info with content_type / transfer_encoding when MHD gave them.
+    if (config_.generate_random_filename_on_upload) {
+        file.set_file_system_file_name(
+            http_utils::generate_random_upload_filename(config_.file_upload_dir));
+    } else {
+        std::string safe_name = http_utils::sanitize_upload_filename(filename);
+        if (safe_name.empty()) return false;
+        file.set_file_system_file_name(config_.file_upload_dir + "/" + safe_name);
+    }
+    // Avoid appending to a leftover file from a previous request.
+    unlink(file.get_file_system_file_name().c_str());
+    if (content_type != nullptr) file.set_content_type(content_type);
+    if (transfer_encoding != nullptr) file.set_transfer_encoding(transfer_encoding);
+    return true;
+}
+
+void upload_pipeline::manage_upload_stream(detail::modded_request* mr,
+        const char* filename, const char* key, http::file_info& file) {
+    // If MHD switches us to a different (filename, key) pair, close the
+    // previous output stream. The four-way OR covers fresh state (both
+    // tracking strings empty) and either coordinate changing.
+    if (mr->upload_filename.empty()
+            || mr->upload_key.empty()
+            || strcmp(filename, mr->upload_filename.c_str()) != 0
+            || strcmp(key, mr->upload_key.c_str()) != 0) {
+        if (mr->upload_ostrm != nullptr) mr->upload_ostrm->close();
+    }
+    // Open a stream when we don't already have one (first chunk, or
+    // just-closed above).
+    if (mr->upload_ostrm == nullptr || !mr->upload_ostrm->is_open()) {
+        mr->upload_key = key;
+        mr->upload_filename = filename;
+        mr->upload_ostrm = std::make_unique<std::ofstream>();
+        mr->upload_ostrm->open(file.get_file_system_file_name(),
+                               std::ios::binary | std::ios::app);
+    }
+}
+
+MHD_Result upload_pipeline::process_file_upload(detail::modded_request* mr,
+        const char* key, const char* filename, const char* content_type,
+        const char* transfer_encoding, const char* data, size_t size) const {
+    http::file_info& file = mr->request->get_or_create_file_info(key, filename);
+    if (file.get_file_system_file_name().empty()) {
+        if (!setup_new_upload_file_info(file, filename, content_type,
+                                        transfer_encoding)) {
+            return MHD_NO;
+        }
+    }
+    manage_upload_stream(mr, filename, key, file);
+    if (size > 0) {
+        mr->upload_ostrm->write(data, size);
+        if (!mr->upload_ostrm->good()) return MHD_NO;
+    }
+    file.grow_file_size(size);
+    return MHD_YES;
+}
+
+MHD_Result upload_pipeline::iterate_file(detail::modded_request* mr,
+        const char* key, const char* filename, const char* content_type,
+        const char* transfer_encoding, const char* data, size_t size) {
+    try {
+        if (config_.file_upload_target != FILE_UPLOAD_DISK_ONLY) {
+            mr->request->set_arg_flat(key,
+                std::string(mr->request->get_arg(key)) + std::string(data, size));
+        }
+        if (*filename != '\0'
+                && config_.file_upload_target != FILE_UPLOAD_MEMORY_ONLY) {
+            MHD_Result r = process_file_upload(
+                mr, key, filename, content_type, transfer_encoding, data, size);
+            if (r != MHD_YES) return r;
+        }
+        return MHD_YES;
+    } catch (const http::generateFilenameException&) {
+        return MHD_NO;
+    }
+}
+
+}  // namespace detail
+}  // namespace httpserver

src/detail/webserver_callbacks.cpp

@@ -65,6 +65,7 @@
 #include "httpserver/detail/http_endpoint.hpp"
 #include "httpserver/detail/lambda_resource.hpp"
 #include "httpserver/detail/modded_request.hpp"
+#include "httpserver/detail/upload_pipeline.hpp"
 #include "httpserver/http_request.hpp"
 #include "httpserver/http_resource.hpp"
 #include "httpserver/http_response.hpp"
@@ -362,94 +363,14 @@ size_t webserver_impl::unescaper_func(void * cls, struct MHD_Connection *c, char
     return std::char_traits<char>::length(s);
 }
 
-MHD_Result webserver_impl::handle_post_form_arg(detail::modded_request* mr,
-        const char* key, const char* data, size_t size, uint64_t off) {
-    // MHD may invoke the post iterator with a null key on a continuation
-    // chunk (off > 0): the field name was supplied on the first call and
-    // is not repeated. With no field name there is nothing to store the
-    // value under, so silently accept the chunk (MHD_YES tells MHD to
-    // continue; MHD_NO would abort the whole request). Guarding here also
-    // stops the raw pointer from reaching std::string, which throws
-    // std::logic_error on null and aborts the process via std::terminate
-    // because the throw escapes a C callback. See issue #375 (same class
-    // of bug as the null-uri fix in uri_log, issue #371).
-    if (key == nullptr) {
-        return MHD_YES;
-    }
-    // No file: set the arg key/value and return. A non-zero @p off
-    // means MHD is feeding us a continuation chunk of a previously-
-    // started value, so append rather than replace.
-    if (off > 0) {
-        mr->request->grow_last_arg(key, std::string(data, size));
-    } else {
-        mr->request->set_arg(key, std::string(data, size));
-    }
-    return MHD_YES;
-}
-
-bool webserver_impl::setup_new_upload_file_info(http::file_info& file,
-        const char* filename, const char* content_type,
-        const char* transfer_encoding) const {
-    // First chunk for this (key, filename) pair: choose the on-disk
-    // destination path (random if generate_random_filename_on_upload,
-    // otherwise sanitize the client-supplied filename) and prime the
-    // file_info with content_type / transfer_encoding when MHD gave
-    // them to us.
-    if (parent->config.generate_random_filename_on_upload) {
-        file.set_file_system_file_name(
-            http_utils::generate_random_upload_filename(parent->config.file_upload_dir));
-    } else {
-        std::string safe_name = http_utils::sanitize_upload_filename(filename);
-        if (safe_name.empty()) return false;
-        file.set_file_system_file_name(parent->config.file_upload_dir + "/" + safe_name);
-    }
-    // Avoid appending to a leftover file from a previous request.
-    unlink(file.get_file_system_file_name().c_str());
-    if (content_type != nullptr) file.set_content_type(content_type);
-    if (transfer_encoding != nullptr) file.set_transfer_encoding(transfer_encoding);
-    return true;
-}
-
-void webserver_impl::manage_upload_stream(detail::modded_request* mr,
-        const char* filename, const char* key, http::file_info& file) {
-    // If MHD switches us to a different (filename, key) pair, close the
-    // previous output stream. The four-way OR covers fresh state (both
-    // tracking strings empty) and either coordinate changing.
-    if (mr->upload_filename.empty()
-            || mr->upload_key.empty()
-            || strcmp(filename, mr->upload_filename.c_str()) != 0
-            || strcmp(key, mr->upload_key.c_str()) != 0) {
-        if (mr->upload_ostrm != nullptr) mr->upload_ostrm->close();
-    }
-    // Open a stream when we don't already have one (first chunk, or
-    // just-closed above).
-    if (mr->upload_ostrm == nullptr || !mr->upload_ostrm->is_open()) {
-        mr->upload_key = key;
-        mr->upload_filename = filename;
-        mr->upload_ostrm = std::make_unique<std::ofstream>();
-        mr->upload_ostrm->open(file.get_file_system_file_name(),
-                               std::ios::binary | std::ios::app);
-    }
-}
-
-MHD_Result webserver_impl::process_file_upload(detail::modded_request* mr,
-        const char* key, const char* filename, const char* content_type,
-        const char* transfer_encoding, const char* data, size_t size) const {
-    http::file_info& file = mr->request->get_or_create_file_info(key, filename);
-    if (file.get_file_system_file_name().empty()) {
-        if (!setup_new_upload_file_info(file, filename, content_type, transfer_encoding)) {
-            return MHD_NO;
-        }
-    }
-    manage_upload_stream(mr, filename, key, file);
-    if (size > 0) {
-        mr->upload_ostrm->write(data, size);
-        if (!mr->upload_ostrm->good()) return MHD_NO;
-    }
-    file.grow_file_size(size);
-    return MHD_YES;
-}
-
+// MHD post-iterator trampoline. Registered with MHD_create_post_processor
+// (webserver_body_pipeline.cpp); its address is taken, so it stays a static
+// webserver_impl member. The upload logic moved to the upload_pipeline
+// behavior service (DR-014 §4.11). The no-file form-arg branch uses the
+// static upload_pipeline::handle_post_form_arg so it is reachable without an
+// owning webserver (post_iterator_null_key_test feeds a null-ws
+// modded_request); the file branch routes through the owning webserver's
+// upload_ instance.
 MHD_Result webserver_impl::post_iterator(void *cls, enum MHD_ValueKind kind,
         const char *key, const char *filename, const char *content_type,
         const char *transfer_encoding, const char *data, uint64_t off, size_t size) {
@@ -458,23 +379,10 @@ MHD_Result webserver_impl::post_iterator(void *cls, enum MHD_ValueKind kind,
     auto* mr = static_cast<detail::modded_request*>(cls);
 
     if (!filename) {
-        return handle_post_form_arg(mr, key, data, size, off);
-    }
-
-    try {
-        if (mr->ws->config.file_upload_target != FILE_UPLOAD_DISK_ONLY) {
-            mr->request->set_arg_flat(key,
-                std::string(mr->request->get_arg(key)) + std::string(data, size));
-        }
-        if (*filename != '\0' && mr->ws->config.file_upload_target != FILE_UPLOAD_MEMORY_ONLY) {
-            MHD_Result r = mr->ws->impl_->process_file_upload(
-                mr, key, filename, content_type, transfer_encoding, data, size);
-            if (r != MHD_YES) return r;
-        }
-        return MHD_YES;
-    } catch (const http::generateFilenameException&) {
-        return MHD_NO;
+        return upload_pipeline::handle_post_form_arg(mr, key, data, size, off);
     }
+    return mr->ws->impl_->upload_.iterate_file(
+        mr, key, filename, content_type, transfer_encoding, data, size);
 }
 
 }  // namespace detail