Skip to content

Commit 368bf85

Browse files
etrclaude
andcommitted
webserver: decompose post_iterator under the CCN 10 bar (21 -> 7)
webserver_impl::post_iterator is the MHD post-iterator trampoline: called repeatedly per request as MHD feeds form fields and file chunks. The original 82-line body wove together the non-file form-arg path and the file-upload path (including memory and/or disk targets, random vs sanitized filename, stream open/close on key/filename transitions, write + size update). CCN 21. Decomposed into per-responsibility helpers on detail::webserver_impl: handle_post_form_arg static; form-arg path (set vs grow on off). setup_new_upload_file_info per-instance; first-chunk filename choice + content_type/transfer_encoding seed. Returns false if sanitize_upload_filename fails so the caller maps it to MHD_NO. manage_upload_stream static; close-on-transition, open-if-needed. process_file_upload per-instance orchestrator for the disk branch. Returns MHD_YES / MHD_NO. post_iterator is now: dispatch (no filename -> handle_post_form_arg); try-block with memory-target arg-flat update; disk-target dispatch to process_file_upload; catch generateFilenameException -> MHD_NO. CCN 7. Every new helper sits at CCN <= 8. Static helpers are static because they don't read parent state (handle_post_form_arg, manage_upload_stream); the disk-orchestrator and setup_new_upload_file_info are instance methods because they read parent->{file_upload_target, generate_random_filename_on_upload, file_upload_dir}. Behaviour preserved verbatim: the four-way OR that detects (filename, key) transitions; the per-chunk grow vs replace based on @p off; the unlink before opening to avoid appending to a leftover file; the short-circuit returns of MHD_NO on sanitize failure and write failure. Verified locally: full `make check`. scripts/check-complexity.sh CCN_MAX ratcheted 22 -> 19 (the new worst offenders are ip_representation::operator< and populate_all_cert_fields, both CCN 18). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5b86daf commit 368bf85

3 files changed

Lines changed: 110 additions & 64 deletions

File tree

scripts/check-complexity.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
set -euo pipefail
2828

2929
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
30-
CCN_MAX="${CCN_MAX:-22}"
30+
CCN_MAX="${CCN_MAX:-19}"
3131

3232
# Prefer the standalone `lizard` entrypoint if it's on PATH; fall back to
3333
# `python3 -m lizard` which is what `pip install --user lizard` produces

src/httpserver/detail/webserver_impl.hpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,32 @@ class webserver_impl {
513513
method_set methods,
514514
std::shared_ptr<::httpserver::http_resource> shim);
515515

516+
// Helpers carved out of post_iterator. The MHD post-iterator
517+
// trampoline is a static MHD callback; the orchestrator below
518+
// (process_file_upload) is an instance method because it reads the
519+
// const config bag on parent (file_upload_target,
520+
// generate_random_filename_on_upload, file_upload_dir).
521+
static MHD_Result handle_post_form_arg(modded_request* mr,
522+
const char* key,
523+
const char* data,
524+
size_t size,
525+
uint64_t off);
526+
bool setup_new_upload_file_info(::httpserver::http::file_info& file,
527+
const char* filename,
528+
const char* content_type,
529+
const char* transfer_encoding) const;
530+
static void manage_upload_stream(modded_request* mr,
531+
const char* filename,
532+
const char* key,
533+
::httpserver::http::file_info& file);
534+
MHD_Result process_file_upload(modded_request* mr,
535+
const char* key,
536+
const char* filename,
537+
const char* content_type,
538+
const char* transfer_encoding,
539+
const char* data,
540+
size_t size) const;
541+
516542
MHD_Result complete_request(MHD_Connection* connection, modded_request* mr,
517543
const char* version, const char* method);
518544
struct MHD_Response* get_raw_response_with_fallback(modded_request* mr);

src/webserver.cpp

Lines changed: 83 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,85 +1521,105 @@ size_t webserver_impl::unescaper_func(void * cls, struct MHD_Connection *c, char
15211521

15221522
namespace detail {
15231523

1524+
MHD_Result webserver_impl::handle_post_form_arg(detail::modded_request* mr,
1525+
const char* key, const char* data, size_t size, uint64_t off) {
1526+
// No file: set the arg key/value and return. A non-zero @p off
1527+
// means MHD is feeding us a continuation chunk of a previously-
1528+
// started value, so append rather than replace.
1529+
if (off > 0) {
1530+
mr->dhr->grow_last_arg(key, std::string(data, size));
1531+
} else {
1532+
mr->dhr->set_arg(key, std::string(data, size));
1533+
}
1534+
return MHD_YES;
1535+
}
1536+
1537+
bool webserver_impl::setup_new_upload_file_info(http::file_info& file,
1538+
const char* filename, const char* content_type,
1539+
const char* transfer_encoding) const {
1540+
// First chunk for this (key, filename) pair: choose the on-disk
1541+
// destination path (random if generate_random_filename_on_upload,
1542+
// otherwise sanitize the client-supplied filename) and prime the
1543+
// file_info with content_type / transfer_encoding when MHD gave
1544+
// them to us.
1545+
if (parent->generate_random_filename_on_upload) {
1546+
file.set_file_system_file_name(
1547+
http_utils::generate_random_upload_filename(parent->file_upload_dir));
1548+
} else {
1549+
std::string safe_name = http_utils::sanitize_upload_filename(filename);
1550+
if (safe_name.empty()) return false;
1551+
file.set_file_system_file_name(parent->file_upload_dir + "/" + safe_name);
1552+
}
1553+
// Avoid appending to a leftover file from a previous request.
1554+
unlink(file.get_file_system_file_name().c_str());
1555+
if (content_type != nullptr) file.set_content_type(content_type);
1556+
if (transfer_encoding != nullptr) file.set_transfer_encoding(transfer_encoding);
1557+
return true;
1558+
}
1559+
1560+
void webserver_impl::manage_upload_stream(detail::modded_request* mr,
1561+
const char* filename, const char* key, http::file_info& file) {
1562+
// If MHD switches us to a different (filename, key) pair, close the
1563+
// previous output stream. The four-way OR covers fresh state (both
1564+
// tracking strings empty) and either coordinate changing.
1565+
if (mr->upload_filename.empty()
1566+
|| mr->upload_key.empty()
1567+
|| strcmp(filename, mr->upload_filename.c_str()) != 0
1568+
|| strcmp(key, mr->upload_key.c_str()) != 0) {
1569+
if (mr->upload_ostrm != nullptr) mr->upload_ostrm->close();
1570+
}
1571+
// Open a stream when we don't already have one (first chunk, or
1572+
// just-closed above).
1573+
if (mr->upload_ostrm == nullptr || !mr->upload_ostrm->is_open()) {
1574+
mr->upload_key = key;
1575+
mr->upload_filename = filename;
1576+
mr->upload_ostrm = std::make_unique<std::ofstream>();
1577+
mr->upload_ostrm->open(file.get_file_system_file_name(),
1578+
std::ios::binary | std::ios::app);
1579+
}
1580+
}
1581+
1582+
MHD_Result webserver_impl::process_file_upload(detail::modded_request* mr,
1583+
const char* key, const char* filename, const char* content_type,
1584+
const char* transfer_encoding, const char* data, size_t size) const {
1585+
http::file_info& file = mr->dhr->get_or_create_file_info(key, filename);
1586+
if (file.get_file_system_file_name().empty()) {
1587+
if (!setup_new_upload_file_info(file, filename, content_type, transfer_encoding)) {
1588+
return MHD_NO;
1589+
}
1590+
}
1591+
manage_upload_stream(mr, filename, key, file);
1592+
if (size > 0) {
1593+
mr->upload_ostrm->write(data, size);
1594+
if (!mr->upload_ostrm->good()) return MHD_NO;
1595+
}
1596+
file.grow_file_size(size);
1597+
return MHD_YES;
1598+
}
1599+
15241600
MHD_Result webserver_impl::post_iterator(void *cls, enum MHD_ValueKind kind,
15251601
const char *key, const char *filename, const char *content_type,
15261602
const char *transfer_encoding, const char *data, uint64_t off, size_t size) {
15271603
// Parameter needed to respect MHD interface, but not needed here.
15281604
std::ignore = kind;
1529-
15301605
auto* mr = static_cast<detail::modded_request*>(cls);
15311606

15321607
if (!filename) {
1533-
// There is no actual file, just set the arg key/value and return.
1534-
if (off > 0) {
1535-
mr->dhr->grow_last_arg(key, std::string(data, size));
1536-
return MHD_YES;
1537-
}
1538-
1539-
mr->dhr->set_arg(key, std::string(data, size));
1540-
return MHD_YES;
1608+
return handle_post_form_arg(mr, key, data, size, off);
15411609
}
15421610

15431611
try {
15441612
if (mr->ws->file_upload_target != FILE_UPLOAD_DISK_ONLY) {
1545-
mr->dhr->set_arg_flat(key, std::string(mr->dhr->get_arg(key)) + std::string(data, size));
1613+
mr->dhr->set_arg_flat(key,
1614+
std::string(mr->dhr->get_arg(key)) + std::string(data, size));
15461615
}
1547-
15481616
if (*filename != '\0' && mr->ws->file_upload_target != FILE_UPLOAD_MEMORY_ONLY) {
1549-
// either get the existing file info struct or create a new one in the file map
1550-
http::file_info& file = mr->dhr->get_or_create_file_info(key, filename);
1551-
// if the file_system_file_name is not filled yet, this is a new entry and the name has to be set
1552-
// (either random or copy of the original filename)
1553-
if (file.get_file_system_file_name().empty()) {
1554-
if (mr->ws->generate_random_filename_on_upload) {
1555-
file.set_file_system_file_name(http_utils::generate_random_upload_filename(mr->ws->file_upload_dir));
1556-
} else {
1557-
std::string safe_name = http_utils::sanitize_upload_filename(filename);
1558-
if (safe_name.empty()) {
1559-
return MHD_NO;
1560-
}
1561-
file.set_file_system_file_name(mr->ws->file_upload_dir + "/" + safe_name);
1562-
}
1563-
// to not append to an already existing file, delete an already existing file
1564-
unlink(file.get_file_system_file_name().c_str());
1565-
if (content_type != nullptr) {
1566-
file.set_content_type(content_type);
1567-
}
1568-
if (transfer_encoding != nullptr) {
1569-
file.set_transfer_encoding(transfer_encoding);
1570-
}
1571-
}
1572-
1573-
// if multiple files are uploaded, a different filename or a different key indicates
1574-
// the start of a new file, so close the previous one
1575-
if (mr->upload_filename.empty() ||
1576-
mr->upload_key.empty() ||
1577-
0 != strcmp(filename, mr->upload_filename.c_str()) ||
1578-
0 != strcmp(key, mr->upload_key.c_str())) {
1579-
if (mr->upload_ostrm != nullptr) {
1580-
mr->upload_ostrm->close();
1581-
}
1582-
}
1583-
1584-
if (mr->upload_ostrm == nullptr || !mr->upload_ostrm->is_open()) {
1585-
mr->upload_key = key;
1586-
mr->upload_filename = filename;
1587-
mr->upload_ostrm = std::make_unique<std::ofstream>();
1588-
mr->upload_ostrm->open(file.get_file_system_file_name(), std::ios::binary | std::ios::app);
1589-
}
1590-
1591-
if (size > 0) {
1592-
mr->upload_ostrm->write(data, size);
1593-
if (!mr->upload_ostrm->good()) {
1594-
return MHD_NO;
1595-
}
1596-
}
1597-
1598-
// update the file size in the map
1599-
file.grow_file_size(size);
1617+
MHD_Result r = mr->ws->impl_->process_file_upload(
1618+
mr, key, filename, content_type, transfer_encoding, data, size);
1619+
if (r != MHD_YES) return r;
16001620
}
16011621
return MHD_YES;
1602-
} catch(const http::generateFilenameException& e) {
1622+
} catch (const http::generateFilenameException&) {
16031623
return MHD_NO;
16041624
}
16051625
}

0 commit comments

Comments
 (0)