Skip to content

Commit fd6c984

Browse files
etrclaude
andcommitted
TASK-015: wire test-request fallback through http_request_impl
create_test_request now allocates the impl_ and stores headers/footers/ cookies/args/querystring/auth/requestor/tls flags on it, instead of on http_request directly. The MHD-touching accessors in http_request_impl (get_connection_value, get_headerlike_values, populate_args, fetch_user_pass, has_tls_session) and in http_request (get_querystring, get_digested_user, get_requestor) now branch on connection_ == nullptr to read from the local maps when running through the test-request path. Also adds a string_body::get_content() helper used by the new create_test_request unit suite, friend-grants create_test_request access to http_request::impl_, and ships the create_test_request unit binary from the autotools build. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e9175c1 commit fd6c984

8 files changed

Lines changed: 157 additions & 33 deletions

File tree

src/Makefile.am

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@
1919
AM_CPPFLAGS = -I../ -I$(srcdir)/httpserver/ -DHTTPSERVER_COMPILATION
2020
METASOURCES = AUTO
2121
lib_LTLIBRARIES = libhttpserver.la
22-
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp file_info.cpp http_request.cpp http_response.cpp http_resource.cpp create_webserver.cpp detail/http_endpoint.cpp detail/body.cpp
22+
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp file_info.cpp http_request.cpp http_response.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp detail/http_endpoint.cpp detail/body.cpp
2323
# noinst_HEADERS: shipped in the tarball but NEVER installed under $prefix/include.
2424
# Detail headers (httpserver/detail/*.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/detail/modded_request.hpp httpserver/detail/http_endpoint.hpp httpserver/detail/body.hpp httpserver/detail/webserver_impl.hpp httpserver/detail/http_request_impl.hpp gettext.h
27-
nobase_include_HEADERS = httpserver.hpp httpserver/body_kind.hpp httpserver/constants.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/feature_unavailable.hpp httpserver/iovec_entry.hpp httpserver/http_arg_value.hpp httpserver/http_method.hpp
27+
nobase_include_HEADERS = httpserver.hpp httpserver/body_kind.hpp httpserver/constants.hpp httpserver/create_webserver.hpp httpserver/create_test_request.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/feature_unavailable.hpp httpserver/iovec_entry.hpp httpserver/http_arg_value.hpp httpserver/http_method.hpp
2828

2929
if HAVE_WEBSOCKET
3030
libhttpserver_la_SOURCES += websocket_handler.cpp

src/create_test_request.cpp

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
*/
2121

2222
#include "httpserver/create_test_request.hpp"
23+
#include "httpserver/detail/http_request_impl.hpp"
2324

25+
#include <memory>
2426
#include <string>
2527
#include <utility>
2628

@@ -29,40 +31,44 @@ namespace httpserver {
2931
http_request create_test_request::build() {
3032
http_request req;
3133

34+
// Allocate an impl for this test request (connection_ stays null,
35+
// indicating the test-request path to all MHD-touching accessors).
36+
req.impl_ = std::make_unique<detail::http_request_impl>();
37+
3238
req.set_method(_method);
3339
req.set_path(_path);
3440
req.set_version(_version);
3541
req.set_content(_content);
3642

37-
req.headers_local = std::move(_headers);
38-
req.footers_local = std::move(_footers);
39-
req.cookies_local = std::move(_cookies);
43+
req.impl_->headers_local = std::move(_headers);
44+
req.impl_->footers_local = std::move(_footers);
45+
req.impl_->cookies_local = std::move(_cookies);
4046

4147
for (auto& [key, values] : _args) {
4248
for (auto& value : values) {
43-
req.cache->unescaped_args[key].push_back(std::move(value));
49+
req.impl_->unescaped_args[key].push_back(std::move(value));
4450
}
4551
}
46-
req.cache->args_populated = true;
52+
req.impl_->args_populated = true;
4753

4854
if (!_querystring.empty()) {
49-
req.cache->querystring = std::move(_querystring);
55+
req.impl_->querystring = std::move(_querystring);
5056
}
5157

5258
#ifdef HAVE_BAUTH
53-
req.cache->username = std::move(_user);
54-
req.cache->password = std::move(_pass);
59+
req.impl_->username = std::move(_user);
60+
req.impl_->password = std::move(_pass);
5561
#endif // HAVE_BAUTH
5662

5763
#ifdef HAVE_DAUTH
58-
req.cache->digested_user = std::move(_digested_user);
64+
req.impl_->digested_user = std::move(_digested_user);
5965
#endif // HAVE_DAUTH
6066

61-
req.cache->requestor_ip = std::move(_requestor);
62-
req.requestor_port_local = _requestor_port;
67+
req.impl_->requestor_ip = std::move(_requestor);
68+
req.impl_->requestor_port_local = _requestor_port;
6369

6470
#ifdef HAVE_GNUTLS
65-
req.tls_enabled_local = _tls_enabled;
71+
req.impl_->tls_enabled_local = _tls_enabled;
6672
#endif // HAVE_GNUTLS
6773

6874
return req;

src/http_request.cpp

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,23 @@ struct arguments_accumulator {
133133
namespace detail {
134134

135135
std::string_view http_request_impl::get_connection_value(std::string_view key, MHD_ValueKind kind) const {
136+
// Test-request path: connection_ is null, fall back to local storage.
137+
if (connection_ == nullptr) {
138+
const auto* map = [&]() -> const http::header_map* {
139+
switch (kind) {
140+
case MHD_HEADER_KIND: return &headers_local;
141+
case MHD_FOOTER_KIND: return &footers_local;
142+
case MHD_COOKIE_KIND: return &cookies_local;
143+
default: return nullptr;
144+
}
145+
}();
146+
if (map != nullptr) {
147+
auto it = map->find(std::string(key));
148+
if (it != map->end()) return it->second;
149+
}
150+
return http_request::EMPTY;
151+
}
152+
136153
const char* header_c = MHD_lookup_connection_value(connection_, kind, key.data());
137154

138155
if (header_c == nullptr) return http_request::EMPTY;
@@ -153,6 +170,24 @@ MHD_Result http_request_impl::build_request_header(void* cls, MHD_ValueKind kind
153170
http::header_view_map http_request_impl::get_headerlike_values(MHD_ValueKind kind) const {
154171
http::header_view_map headers;
155172

173+
// Test-request path: connection_ is null, build view map from local storage.
174+
if (connection_ == nullptr) {
175+
const auto* map = [&]() -> const http::header_map* {
176+
switch (kind) {
177+
case MHD_HEADER_KIND: return &headers_local;
178+
case MHD_FOOTER_KIND: return &footers_local;
179+
case MHD_COOKIE_KIND: return &cookies_local;
180+
default: return nullptr;
181+
}
182+
}();
183+
if (map != nullptr) {
184+
for (const auto& [k, v] : *map) {
185+
headers[k] = v;
186+
}
187+
}
188+
return headers;
189+
}
190+
156191
MHD_get_connection_values(connection_, kind, &http_request_impl::build_request_header,
157192
reinterpret_cast<void*>(&headers));
158193

@@ -197,6 +232,11 @@ void http_request_impl::populate_args() const {
197232
if (args_populated) {
198233
return;
199234
}
235+
// Test-request path: connection_ is null, args already set directly.
236+
if (connection_ == nullptr) {
237+
args_populated = true;
238+
return;
239+
}
200240
arguments_accumulator aa;
201241
aa.unescaper = unescaper_;
202242
aa.arguments = &unescaped_args;
@@ -252,6 +292,10 @@ void http_request_impl::grow_last_arg(const std::string& key, const std::string&
252292

253293
#ifdef HAVE_BAUTH
254294
void http_request_impl::fetch_user_pass() const {
295+
// Test-request path: connection_ is null, credentials already set.
296+
if (connection_ == nullptr) {
297+
return;
298+
}
255299
struct MHD_BasicAuthInfo* info = MHD_basic_auth_get_username_password3(connection_);
256300

257301
if (info != nullptr) {
@@ -266,6 +310,10 @@ void http_request_impl::fetch_user_pass() const {
266310

267311
#ifdef HAVE_GNUTLS
268312
bool http_request_impl::has_tls_session() const {
313+
// Test-request path: connection_ is null, return the local flag.
314+
if (connection_ == nullptr) {
315+
return tls_enabled_local;
316+
}
269317
const MHD_ConnectionInfo* conninfo = MHD_get_connection_info(connection_, MHD_CONNECTION_INFO_GNUTLS_SESSION);
270318
return (conninfo != nullptr);
271319
}
@@ -514,6 +562,8 @@ http_arg_value http_request::get_arg(std::string_view key) const {
514562
}
515563

516564
std::string_view http_request::get_arg_flat(std::string_view key) const {
565+
impl_->populate_args();
566+
517567
auto const it = impl_->unescaped_args.find(key);
518568

519569
if (it != impl_->unescaped_args.end()) {
@@ -558,6 +608,11 @@ std::string_view http_request::get_querystring() const {
558608
return impl_->querystring;
559609
}
560610

611+
// Test-request path: connection_ is null, querystring already set (or empty).
612+
if (impl_->connection_ == nullptr) {
613+
return impl_->querystring;
614+
}
615+
561616
MHD_get_connection_values(impl_->connection_, MHD_GET_ARGUMENT_KIND,
562617
&detail::http_request_impl::build_request_querystring,
563618
reinterpret_cast<void*>(&impl_->querystring));
@@ -589,6 +644,11 @@ std::string_view http_request::get_digested_user() const {
589644
return impl_->digested_user;
590645
}
591646

647+
// Test-request path: connection_ is null, digested_user already set.
648+
if (impl_->connection_ == nullptr) {
649+
return impl_->digested_user;
650+
}
651+
592652
struct MHD_DigestAuthUsernameInfo* info = MHD_digest_auth_get_username3(impl_->connection_);
593653

594654
impl_->digested_user = EMPTY;
@@ -657,15 +717,33 @@ std::string_view http_request::get_requestor() const {
657717
return impl_->requestor_ip;
658718
}
659719

720+
// Test-request path: connection_ is null, requestor_ip already set.
721+
if (impl_->connection_ == nullptr) {
722+
return impl_->requestor_ip;
723+
}
724+
660725
const MHD_ConnectionInfo* conninfo = MHD_get_connection_info(impl_->connection_, MHD_CONNECTION_INFO_CLIENT_ADDRESS);
661726

727+
if (conninfo == nullptr) {
728+
return EMPTY;
729+
}
730+
662731
impl_->requestor_ip = http::get_ip_str(conninfo->client_addr);
663732
return impl_->requestor_ip;
664733
}
665734

666735
uint16_t http_request::get_requestor_port() const {
736+
// Test-request path: connection_ is null, use local port.
737+
if (impl_->connection_ == nullptr) {
738+
return impl_->requestor_port_local;
739+
}
740+
667741
const MHD_ConnectionInfo* conninfo = MHD_get_connection_info(impl_->connection_, MHD_CONNECTION_INFO_CLIENT_ADDRESS);
668742

743+
if (conninfo == nullptr) {
744+
return 0;
745+
}
746+
669747
return http::get_port(conninfo->client_addr);
670748
}
671749

src/httpserver/detail/body.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,11 @@ class string_body final : public body {
139139
::new (dst) string_body(std::move(*this));
140140
}
141141

142+
/// Returns the body string. Primarily for tests.
143+
[[nodiscard]] const std::string& get_content() const noexcept {
144+
return content_;
145+
}
146+
142147
private:
143148
std::string content_;
144149
};

src/httpserver/detail/http_request_impl.hpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,18 @@ class http_request_impl {
9292
file_cleanup_callback_ptr file_cleanup_callback_ = nullptr;
9393
std::map<std::string, std::map<std::string, http::file_info>> files_;
9494

95+
// --- test-request local storage ---
96+
// When connection_ is null (create_test_request path), get_header /
97+
// get_footer / get_cookie / get_headerlike_values / get_requestor_port /
98+
// has_tls_session fall back to these instead of calling MHD APIs.
99+
http::header_map headers_local;
100+
http::header_map footers_local;
101+
http::header_map cookies_local;
102+
uint16_t requestor_port_local = 0;
103+
#ifdef HAVE_GNUTLS
104+
bool tls_enabled_local = false;
105+
#endif // HAVE_GNUTLS
106+
95107
// --- lazy caches (formerly the http_request_data_cache struct) ---
96108
// All marked mutable: const accessors lazily populate them.
97109
#ifdef HAVE_BAUTH

src/httpserver/http_request.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ class http_request {
472472
friend class webserver;
473473
friend class detail::webserver_impl; // TASK-014: PIMPL dispatch path
474474
friend struct detail::modded_request;
475+
friend class create_test_request; // TASK-015: test builder accesses impl_
475476
};
476477

477478
std::ostream &operator<< (std::ostream &os, const http_request &r);

test/Makefile.am

Lines changed: 8 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 header_hygiene_iovec header_hygiene iovec_entry http_method constants body http_response_sbo http_response_factories webserver_pimpl http_request_pimpl
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 header_hygiene iovec_entry http_method constants body http_response_sbo http_response_factories webserver_pimpl http_request_pimpl create_test_request
3030

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

@@ -109,6 +109,13 @@ webserver_pimpl_LDADD =
109109
http_request_pimpl_SOURCES = unit/http_request_pimpl_test.cpp
110110
http_request_pimpl_LDADD =
111111

112+
# create_test_request: TASK-015 functional tests for the create_test_request
113+
# builder and the http_request public API on test-constructed requests. Wires
114+
# up headers/footers/cookies/args/querystring/requestor/auth via impl_ local
115+
# maps rather than a live MHD_Connection*.
116+
create_test_request_SOURCES = unit/create_test_request_test.cpp
117+
create_test_request_LDADD = $(LDADD) -lmicrohttpd
118+
112119
noinst_HEADERS = littletest.hpp
113120
AM_CXXFLAGS += -Wall -fPIC -Wno-overloaded-virtual
114121

test/unit/create_test_request_test.cpp

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,32 @@
2222
#include <string>
2323

2424
#include "./httpserver.hpp"
25+
#include "httpserver/create_test_request.hpp"
26+
#include "httpserver/detail/body.hpp"
2527
#include "./littletest.hpp"
2628

2729
using httpserver::create_test_request;
2830
using httpserver::http_request;
2931
using httpserver::http_resource;
3032
using httpserver::http_response;
31-
using httpserver::string_response;
32-
using httpserver::file_response;
33+
34+
// Test-only accessor for http_response internals (same pattern as
35+
// http_response_sbo_test.cpp and http_response_factories_test.cpp).
36+
namespace httpserver {
37+
struct http_response_sbo_test_access {
38+
static bool body_inline(http_response& r) noexcept {
39+
return r.body_inline_;
40+
}
41+
static httpserver::detail::body* body_ptr(http_response& r) noexcept {
42+
return r.body_;
43+
}
44+
static body_kind kind(http_response& r) noexcept { return r.kind_; }
45+
};
46+
} // namespace httpserver
47+
48+
namespace {
49+
using SBO = httpserver::http_response_sbo_test_access;
50+
} // namespace
3351

3452
LT_BEGIN_SUITE(create_test_request_suite)
3553
void set_up() {
@@ -195,7 +213,7 @@ class greeting_resource : public http_resource {
195213
std::shared_ptr<http_response> render_GET(const http_request& req) override {
196214
std::string name(req.get_arg_flat("name"));
197215
if (name.empty()) name = "World";
198-
return std::make_shared<string_response>("Hello, " + name);
216+
return std::make_shared<http_response>(http_response::string("Hello, " + name));
199217
}
200218
};
201219

@@ -206,23 +224,16 @@ LT_BEGIN_AUTO_TEST(create_test_request_suite, render_with_test_request)
206224
.arg("name", "Alice")
207225
.build();
208226
auto resp = resource.render_GET(req);
209-
auto* sr = dynamic_cast<string_response*>(resp.get());
210-
LT_ASSERT(sr != nullptr);
211-
LT_CHECK_EQ(std::string(sr->get_content()), std::string("Hello, Alice"));
227+
LT_ASSERT(resp != nullptr);
228+
// Verify the response body kind is string.
229+
LT_CHECK_EQ(static_cast<int>(resp->kind()),
230+
static_cast<int>(httpserver::body_kind::string));
231+
// Verify the response body content reflects the arg.
232+
auto* sb = dynamic_cast<httpserver::detail::string_body*>(SBO::body_ptr(*resp));
233+
LT_ASSERT(sb != nullptr);
234+
LT_CHECK_EQ(sb->get_content(), std::string("Hello, Alice"));
212235
LT_END_AUTO_TEST(render_with_test_request)
213236

214-
// Test string_response get_content
215-
LT_BEGIN_AUTO_TEST(create_test_request_suite, string_response_get_content)
216-
string_response resp("test body", 200);
217-
LT_CHECK_EQ(std::string(resp.get_content()), std::string("test body"));
218-
LT_END_AUTO_TEST(string_response_get_content)
219-
220-
// Test file_response get_filename
221-
LT_BEGIN_AUTO_TEST(create_test_request_suite, file_response_get_filename)
222-
file_response resp("/tmp/test.txt", 200);
223-
LT_CHECK_EQ(std::string(resp.get_filename()), std::string("/tmp/test.txt"));
224-
LT_END_AUTO_TEST(file_response_get_filename)
225-
226237
// Test full chain of all builder methods
227238
LT_BEGIN_AUTO_TEST(create_test_request_suite, full_chain)
228239
auto req = create_test_request()
@@ -237,8 +248,10 @@ LT_BEGIN_AUTO_TEST(create_test_request_suite, full_chain)
237248
.arg("key1", "val1")
238249
.arg("key2", "val2")
239250
.querystring("?key1=val1&key2=val2")
251+
#ifdef HAVE_BAUTH
240252
.user("testuser")
241253
.pass("testpass")
254+
#endif
242255
.requestor("10.0.0.1")
243256
.requestor_port(9090)
244257
.build();
@@ -254,8 +267,10 @@ LT_BEGIN_AUTO_TEST(create_test_request_suite, full_chain)
254267
LT_CHECK_EQ(std::string(req.get_arg_flat("key1")), std::string("val1"));
255268
LT_CHECK_EQ(std::string(req.get_arg_flat("key2")), std::string("val2"));
256269
LT_CHECK_EQ(std::string(req.get_querystring()), std::string("?key1=val1&key2=val2"));
270+
#ifdef HAVE_BAUTH
257271
LT_CHECK_EQ(std::string(req.get_user()), std::string("testuser"));
258272
LT_CHECK_EQ(std::string(req.get_pass()), std::string("testpass"));
273+
#endif
259274
LT_CHECK_EQ(std::string(req.get_requestor()), std::string("10.0.0.1"));
260275
LT_CHECK_EQ(req.get_requestor_port(), static_cast<uint16_t>(9090));
261276
LT_END_AUTO_TEST(full_chain)

0 commit comments

Comments
 (0)