Skip to content

Commit d2a2544

Browse files
etretr
committed
Merge TASK-004: Add httpserver::iovec_entry POD with layout-pinning asserts
2 parents a05f79c + 259b4bb commit d2a2544

9 files changed

Lines changed: 519 additions & 21 deletions

‎src/Makefile.am‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp fil
2424
# Detail headers (httpserver/details/*.hpp) live here so they cannot leak to
2525
# downstream consumers — the public surface comes in through <httpserver.hpp>.
2626
noinst_HEADERS = httpserver/string_utilities.hpp httpserver/details/modded_request.hpp httpserver/details/http_endpoint.hpp gettext.h
27-
nobase_include_HEADERS = httpserver.hpp httpserver/create_webserver.hpp httpserver/webserver.hpp httpserver/http_utils.hpp httpserver/file_info.hpp httpserver/http_request.hpp httpserver/http_response.hpp httpserver/http_resource.hpp httpserver/string_response.hpp httpserver/digest_auth_fail_response.hpp httpserver/deferred_response.hpp httpserver/file_response.hpp httpserver/pipe_response.hpp httpserver/empty_response.hpp httpserver/feature_unavailable.hpp httpserver/iovec_response.hpp httpserver/http_arg_value.hpp
27+
nobase_include_HEADERS = httpserver.hpp httpserver/create_webserver.hpp httpserver/webserver.hpp httpserver/http_utils.hpp httpserver/file_info.hpp httpserver/http_request.hpp httpserver/http_response.hpp httpserver/http_resource.hpp httpserver/string_response.hpp httpserver/digest_auth_fail_response.hpp httpserver/deferred_response.hpp httpserver/file_response.hpp httpserver/pipe_response.hpp httpserver/empty_response.hpp httpserver/feature_unavailable.hpp httpserver/iovec_entry.hpp httpserver/iovec_response.hpp httpserver/http_arg_value.hpp
2828

2929
if HAVE_BAUTH
3030
libhttpserver_la_SOURCES += basic_auth_fail_response.cpp

‎src/httpserver.hpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include "httpserver/http_resource.hpp"
4343
#include "httpserver/http_response.hpp"
4444
#include "httpserver/http_utils.hpp"
45+
#include "httpserver/iovec_entry.hpp"
4546
#include "httpserver/iovec_response.hpp"
4647
#include "httpserver/file_info.hpp"
4748
#include "httpserver/pipe_response.hpp"

‎src/httpserver/iovec_entry.hpp‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2019 Sebastiano Merlino
4+
5+
This library is free software; you can redistribute it and/or
6+
modify it under the terms of the GNU Lesser General Public
7+
License as published by the Free Software Foundation; either
8+
version 2.1 of the License, or (at your option) any later version.
9+
10+
This library is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public
16+
License along with this library; if not, write to the Free Software
17+
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
18+
USA
19+
*/
20+
21+
#if !defined (_HTTPSERVER_HPP_INSIDE_) && !defined (HTTPSERVER_COMPILATION)
22+
#error "Only <httpserver.hpp> or <httpserverpp> can be included directly."
23+
#endif
24+
25+
#ifndef SRC_HTTPSERVER_IOVEC_ENTRY_HPP_
26+
#define SRC_HTTPSERVER_IOVEC_ENTRY_HPP_
27+
28+
#include <cstddef>
29+
30+
namespace httpserver {
31+
32+
// Library-defined POD describing a single scatter/gather buffer at the
33+
// public API surface. Replaces `struct iovec` from <sys/uio.h>, keeping
34+
// the public-header surface free of POSIX-only system headers.
35+
//
36+
// Layout is pinned to match POSIX `struct iovec` and libmicrohttpd's
37+
// `MHD_IoVec` so the dispatch path can `reinterpret_cast` a contiguous
38+
// array of iovec_entry into either C type at zero copy. The pinning
39+
// asserts live next to the cast site (currently `iovec_response.cpp`,
40+
// moving to `details/body.hpp` once TASK-009 lands).
41+
//
42+
// `base` is `const void*` because libhttpserver never writes through
43+
// these buffers on the response path.
44+
struct iovec_entry {
45+
const void* base;
46+
std::size_t len;
47+
};
48+
49+
} // namespace httpserver
50+
#endif // SRC_HTTPSERVER_IOVEC_ENTRY_HPP_

‎src/httpserver/iovec_response.hpp‎

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <vector>
3131
#include "httpserver/http_utils.hpp"
3232
#include "httpserver/http_response.hpp"
33+
#include "httpserver/iovec_entry.hpp"
3334

3435
struct MHD_Response;
3536

@@ -39,25 +40,66 @@ class iovec_response : public http_response {
3940
public:
4041
iovec_response() = default;
4142

43+
// Owning constructor: the response takes ownership of the string buffers.
44+
// The iovec_entry array is built eagerly at construction so get_raw_response()
45+
// allocates nothing on the hot dispatch path.
4246
explicit iovec_response(
43-
std::vector<std::string> buffers,
47+
std::vector<std::string> owned_buffers,
4448
int response_code = http::http_utils::http_ok,
45-
const std::string& content_type = http::http_utils::text_plain):
46-
http_response(response_code, content_type),
47-
buffers(std::move(buffers)) { }
49+
const std::string& content_type = http::http_utils::text_plain);
50+
51+
/**
52+
* Non-owning constructor: the caller supplies pre-built iovec_entry pairs.
53+
* This is TASK-004's genuine zero-copy path: no heap allocation or data
54+
* copy is performed.
55+
*
56+
* @attention The caller is responsible for keeping the pointed-to buffers
57+
* alive at least until MHD_destroy_response() returns for the response
58+
* produced by get_raw_response(). libmicrohttpd holds a reference to the
59+
* buffer pointers until MHD_destroy_response() is called in the dispatch
60+
* path (webserver.cpp). Freeing any backing buffer before that point
61+
* causes a use-after-free inside libmicrohttpd (CWE-416). In practice
62+
* this means the buffers must outlive the iovec_response object AND the
63+
* MHD response lifecycle, which ends at MHD_destroy_response().
64+
*
65+
* @note This API surface is transitional (see PRD-RSP-REQ-006 /
66+
* TASK-010); it will be removed or replaced in a future v2.0 revision.
67+
*/
68+
explicit iovec_response(
69+
std::vector<iovec_entry> caller_entries,
70+
int response_code = http::http_utils::http_ok,
71+
const std::string& content_type = http::http_utils::text_plain);
72+
73+
// Copy construction and copy assignment are deleted: the owning constructor
74+
// stores void* pointers (entries_) into owned_buffers_ string storage.
75+
// A defaulted copy would shallow-copy entries_ while deep-copying
76+
// owned_buffers_ to new addresses, making entries_ dangle as soon as the
77+
// source is destroyed (CWE-416). Deletion forces callers onto move
78+
// semantics, which are safe because std::vector move transfers the heap
79+
// block and keeps string addresses stable.
80+
iovec_response(const iovec_response&) = delete;
81+
iovec_response& operator=(const iovec_response&) = delete;
4882

49-
iovec_response(const iovec_response& other) = default;
5083
iovec_response(iovec_response&& other) noexcept = default;
51-
52-
iovec_response& operator=(const iovec_response& b) = default;
53-
iovec_response& operator=(iovec_response&& b) = default;
84+
iovec_response& operator=(iovec_response&& b) noexcept = default;
5485

5586
~iovec_response() = default;
5687

88+
// Returns a new MHD_Response* or nullptr on error (e.g. buffer count
89+
// exceeds MHD's unsigned-int limit). The caller does not own the returned
90+
// pointer; MHD manages its lifetime. May return nullptr; all callers on
91+
// the dispatch path must check before use.
5792
MHD_Response* get_raw_response();
5893

5994
private:
60-
std::vector<std::string> buffers;
95+
// Owned string buffers (populated by the owning constructor).
96+
std::vector<std::string> owned_buffers_;
97+
98+
// Flattened iovec_entry array ready for the MHD cast. For the owning
99+
// constructor this is populated at construction time (zero allocation on
100+
// dispatch). For the non-owning constructor the caller-supplied entries
101+
// are stored directly.
102+
std::vector<iovec_entry> entries_;
61103
};
62104

63105
} // namespace httpserver

‎src/iovec_response.cpp‎

Lines changed: 97 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,113 @@
1919
*/
2020

2121
#include "httpserver/iovec_response.hpp"
22+
#include "httpserver/iovec_entry.hpp"
23+
24+
#include <cstddef>
25+
#include <limits>
2226
#include <microhttpd.h>
27+
#include <sys/uio.h>
28+
#include <type_traits>
2329
#include <vector>
2430

2531
struct MHD_Response;
2632

2733
namespace httpserver {
2834

35+
// ---------------------------------------------------------------------------
36+
// TASK-004: layout-pinning static_asserts.
37+
//
38+
// httpserver::iovec_entry is the public scatter/gather POD; libmicrohttpd's
39+
// MHD_IoVec is the actual cast target on the dispatch path. POSIX struct
40+
// iovec is asserted in parallel because the spec mandates it and because
41+
// every platform we ship to defines all three with identical layout
42+
// (glibc, musl, macOS, FreeBSD, NetBSD, OpenBSD, illumos).
43+
//
44+
// LIBHTTPSERVER_TODO_TASK004_MEMCPY_FALLBACK: if any of the asserts below
45+
// ever fires on a divergent-layout platform, the fix is to replace the
46+
// reinterpret_cast in the dispatch path with an element-by-element copy
47+
// into a stack/heap MHD_IoVec[]. Until such a platform appears the
48+
// asserts are the gate — a build failure on the divergent platform is
49+
// the desired outcome (loud, immediate, with the assert string naming
50+
// what diverged).
51+
// ---------------------------------------------------------------------------
52+
static_assert(sizeof(::httpserver::iovec_entry) == sizeof(struct iovec),
53+
"iovec_entry size must match POSIX struct iovec — divergent platform; "
54+
"implement memcpy fallback (see TASK-004)");
55+
static_assert(offsetof(::httpserver::iovec_entry, base) ==
56+
offsetof(struct iovec, iov_base),
57+
"iovec_entry::base offset must match struct iovec::iov_base");
58+
static_assert(offsetof(::httpserver::iovec_entry, len) ==
59+
offsetof(struct iovec, iov_len),
60+
"iovec_entry::len offset must match struct iovec::iov_len");
61+
62+
static_assert(sizeof(::httpserver::iovec_entry) == sizeof(MHD_IoVec),
63+
"iovec_entry size must match libmicrohttpd MHD_IoVec — MHD layout drift");
64+
static_assert(offsetof(::httpserver::iovec_entry, base) ==
65+
offsetof(MHD_IoVec, iov_base),
66+
"iovec_entry::base offset must match MHD_IoVec::iov_base");
67+
static_assert(offsetof(::httpserver::iovec_entry, len) ==
68+
offsetof(MHD_IoVec, iov_len),
69+
"iovec_entry::len offset must match MHD_IoVec::iov_len");
70+
71+
// Alignment pinning: ensures the reinterpret_cast array stride is safe on
72+
// architectures that trap on misaligned loads (SPARC, some ARM configs).
73+
// CWE-704: without alignof equality the cast is UB even when size/offset match.
74+
static_assert(alignof(::httpserver::iovec_entry) == alignof(struct iovec),
75+
"iovec_entry alignment must match POSIX struct iovec — divergent platform; "
76+
"implement memcpy fallback (see TASK-004)");
77+
static_assert(alignof(::httpserver::iovec_entry) == alignof(MHD_IoVec),
78+
"iovec_entry alignment must match MHD_IoVec — MHD layout drift");
79+
80+
// Standard-layout guarantee: required so that reinterpret_cast between
81+
// pointer-interconvertible types is well-defined under -fstrict-aliasing.
82+
static_assert(std::is_standard_layout_v<::httpserver::iovec_entry>,
83+
"iovec_entry must be standard layout for reinterpret_cast to MHD_IoVec");
84+
85+
iovec_response::iovec_response(
86+
std::vector<std::string> owned_buffers,
87+
int response_code,
88+
const std::string& content_type)
89+
: http_response(response_code, content_type),
90+
owned_buffers_(std::move(owned_buffers)) {
91+
// Build the iovec_entry array eagerly so get_raw_response() is
92+
// allocation-free on the hot dispatch path.
93+
entries_.reserve(owned_buffers_.size());
94+
for (const auto& b : owned_buffers_) {
95+
entries_.push_back({b.data(), b.size()});
96+
}
97+
}
98+
99+
iovec_response::iovec_response(
100+
std::vector<iovec_entry> caller_entries,
101+
int response_code,
102+
const std::string& content_type)
103+
: http_response(response_code, content_type),
104+
entries_(std::move(caller_entries)) {
105+
// owned_buffers_ is empty — buffer ownership stays with the caller.
106+
}
107+
29108
MHD_Response* iovec_response::get_raw_response() {
30-
// MHD_create_response_from_iovec makes an internal copy of the iov array,
31-
// so the local vector is safe. The buffer data pointed to by iov_base must
32-
// remain valid until the response is destroyed — this is guaranteed because
33-
// the buffers are owned by this iovec_response object.
34-
std::vector<MHD_IoVec> iov(buffers.size());
35-
for (size_t i = 0; i < buffers.size(); ++i) {
36-
iov[i].iov_base = buffers[i].data();
37-
iov[i].iov_len = buffers[i].size();
109+
// Guard against integer narrowing: MHD_create_response_from_iovec takes
110+
// an unsigned int count. A vector with more than UINT_MAX entries would
111+
// silently truncate, causing MHD to read only part of the array while the
112+
// reported body length diverges from the actual allocation (CWE-190,
113+
// CWE-125). Return nullptr (the documented MHD "error" sentinel) instead.
114+
if (entries_.size() >
115+
static_cast<std::size_t>(
116+
std::numeric_limits<unsigned int>::max())) {
117+
return nullptr;
38118
}
119+
120+
// The reinterpret_cast is well-defined because the layout-pinning
121+
// static_asserts above guarantee identical size, field offsets, and
122+
// alignment between iovec_entry and MHD_IoVec (C++ [basic.align],
123+
// CWE-704). entries_ was populated at construction time: no heap
124+
// allocation occurs on this path. The cast bridge will move into
125+
// details/body.hpp when TASK-009 lands.
39126
return MHD_create_response_from_iovec(
40-
iov.data(),
41-
static_cast<unsigned int>(iov.size()),
127+
reinterpret_cast<const MHD_IoVec*>(entries_.data()),
128+
static_cast<unsigned int>(entries_.size()),
42129
nullptr,
43130
nullptr);
44131
}

‎test/Makefile.am‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ LDADD += -lcurl
2626

2727
AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/ -DHTTPSERVER_COMPILATION
2828
METASOURCES = AUTO
29-
check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource http_response create_webserver new_response_types daemon_info uri_log feature_unavailable
29+
check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource http_response create_webserver new_response_types daemon_info uri_log feature_unavailable header_hygiene_iovec iovec_entry iovec_response
3030

3131
MOSTLYCLEANFILES = *.gcda *.gcno *.gcov
3232

@@ -52,6 +52,9 @@ uri_log_SOURCES = unit/uri_log_test.cpp
5252
# LDADD (modern ld enforces --no-copy-dt-needed-entries).
5353
uri_log_LDADD = $(LDADD) -lmicrohttpd
5454
feature_unavailable_SOURCES = unit/feature_unavailable_test.cpp
55+
header_hygiene_iovec_SOURCES = unit/header_hygiene_iovec_test.cpp
56+
iovec_entry_SOURCES = unit/iovec_entry_test.cpp
57+
iovec_response_SOURCES = unit/iovec_response_test.cpp
5558

5659
noinst_HEADERS = littletest.hpp
5760
AM_CXXFLAGS += -Wall -fPIC -Wno-overloaded-virtual

‎test/unit/header_hygiene_iovec_test.cpp‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2019 Sebastiano Merlino
4+
5+
This library is free software; you can redistribute it and/or
6+
modify it under the terms of the GNU Lesser General Public
7+
License as published by the Free Software Foundation; either
8+
version 2.1 of the License, or (at your option) any later version.
9+
10+
This library is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public
16+
License along with this library; if not, write to the Free Software
17+
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
18+
USA
19+
*/
20+
21+
// Header-hygiene sentinel for TASK-004:
22+
//
23+
// AC #4 of TASK-004 ("public header must not include <sys/uio.h>") is
24+
// enforced by including iovec_entry.hpp in isolation, then checking the
25+
// well-known include-guard macros that <sys/uio.h> defines on every
26+
// supported platform:
27+
//
28+
// Linux/glibc: _SYS_UIO_H (set by <sys/uio.h>)
29+
// macOS/BSD: _SYS_UIO_H_ (set by <sys/uio.h>)
30+
// musl: _SYS_UIO_H (same as glibc)
31+
//
32+
// If any of those macros is defined after including iovec_entry.hpp, the
33+
// header has leaked <sys/uio.h> and the build fails with a descriptive
34+
// #error message. The TU compiling at all (and none of those macros being
35+
// defined) is the assertion — no runtime test is needed for this guarantee.
36+
//
37+
// HTTPSERVER_COMPILATION is defined by AM_CPPFLAGS in test/Makefile.am
38+
// so the inclusion guard in iovec_entry.hpp is satisfied.
39+
40+
#include "httpserver/iovec_entry.hpp"
41+
42+
// --- preprocessor-based leak detection ------------------------------------
43+
44+
#ifdef _SYS_UIO_H
45+
# error "<sys/uio.h> was pulled in transitively by httpserver/iovec_entry.hpp (glibc/musl guard _SYS_UIO_H)"
46+
#endif
47+
48+
#ifdef _SYS_UIO_H_
49+
# error "<sys/uio.h> was pulled in transitively by httpserver/iovec_entry.hpp (macOS/BSD guard _SYS_UIO_H_)"
50+
#endif
51+
52+
// --------------------------------------------------------------------------
53+
54+
#include "./littletest.hpp"
55+
56+
LT_BEGIN_SUITE(header_hygiene_iovec_suite)
57+
void set_up() {
58+
}
59+
60+
void tear_down() {
61+
}
62+
LT_END_SUITE(header_hygiene_iovec_suite)
63+
64+
// Verify that iovec_entry is accessible and sizeof/alignof are non-zero
65+
// without any POSIX headers in scope. This confirms that no system types
66+
// leaked in through iovec_entry.hpp and that the type is self-contained.
67+
LT_BEGIN_AUTO_TEST(header_hygiene_iovec_suite, iovec_entry_visible_without_sys_uio)
68+
// If any system header leaked in, alignof/sizeof would still be correct,
69+
// but the #error directives above ensure this test is only reached on a
70+
// clean TU. These checks confirm the type is truly self-contained.
71+
static_assert(sizeof(httpserver::iovec_entry) > 0,
72+
"iovec_entry must have non-zero size without sys/uio.h");
73+
static_assert(alignof(httpserver::iovec_entry) > 0,
74+
"iovec_entry must have non-zero alignment without sys/uio.h");
75+
LT_CHECK_EQ(true, true); // TU compiled clean: no sys/uio.h leak detected
76+
LT_END_AUTO_TEST(iovec_entry_visible_without_sys_uio)
77+
78+
LT_BEGIN_AUTO_TEST_ENV()
79+
AUTORUN_TESTS()
80+
LT_END_AUTO_TEST_ENV()

0 commit comments

Comments
 (0)