Skip to content

Commit cd15cfb

Browse files
etrclaude
andcommitted
TASK-018: http_request single-key getters return string_view, all const
Per-key getters (get_header / get_cookie / get_footer / get_arg / get_arg_flat) and the always-present getters (get_path / get_method / get_version / get_content / get_querystring) were already const-callable and (post-TASK-015/016/017) empty-on-miss without insertion. This task finalizes the contract: - Mark the five always-present getters `noexcept`. The four trivial string-returning getters (path/method/version/content) are reduced noexcept because string_view's conversion from std::string is itself noexcept since C++17. `get_querystring()` becomes noexcept by moving its lazy MHD enumeration to the http_request ctor on the live-MHD path; the public reader is now a trivial member access. - Add Doxygen `@note` blocks reiterating the per-request lifetime contract on each of the five. - Lock the entire surface (10 getters) at compile time via a new pure-static_assert TU `test/unit/http_request_const_getters_test.cpp`, asserting `is_invocable_v<..., const http_request&, ...>`, exact return-type identity (string_view for nine; http_arg_value for get_arg, deliberately preserved for multi-value semantics), and `noexcept` on the five always-present getters. - Add behavioral tests in create_test_request_test.cpp: a missing-key-no-insert assertion that hammers each per-key getter with five misses and verifies the underlying container sizes don't grow, plus a hit-returns-correct-value assertion. - Wire the new TU into test/Makefile.am. Note: `get_arg` keeps its `http_arg_value` return type. The architecture spec §4.2 reads "string_view get_arg" but test/integ/basic.cpp depends on `.get_all_values()` (multi-value semantics that get_arg_flat doesn't cover). Narrowing get_arg to string_view would either alias get_arg_flat or require renaming the multi-value method -- both wider than TASK-018's stated scope. The interpretive deviation is documented in the test's static_assert comments. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ea10882 commit cd15cfb

5 files changed

Lines changed: 252 additions & 28 deletions

File tree

src/http_request.cpp

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,20 @@ http_request::http_request(struct MHD_Connection* underlying_connection, unescap
656656
impl_.reset(p);
657657
impl_.get_deleter().fn = &detail::destroy_impl_arena;
658658
}
659+
660+
// TASK-018: assemble the querystring eagerly on the live-MHD path so
661+
// the public reader can be `noexcept`. On the test-request path
662+
// (connection_ == nullptr) the create_test_request builder is the
663+
// sole writer of impl_->querystring; leave it untouched here.
664+
// Allocations during assembly land on the per-connection arena (or
665+
// the heap fallback) and may throw -- that's permitted during
666+
// construction.
667+
if (impl_->connection_ != nullptr) {
668+
MHD_get_connection_values(
669+
impl_->connection_, MHD_GET_ARGUMENT_KIND,
670+
&detail::http_request_impl::build_request_querystring,
671+
reinterpret_cast<void*>(&impl_->querystring));
672+
}
659673
}
660674

661675
http_request::~http_request() {
@@ -845,20 +859,10 @@ const std::map<std::string, std::map<std::string, http::file_info>>& http_reques
845859
return impl_->files_;
846860
}
847861

848-
std::string_view http_request::get_querystring() const {
849-
if (!impl_->querystring.empty()) {
850-
return impl_->querystring;
851-
}
852-
853-
// Test-request path: connection_ is null, querystring already set (or empty).
854-
if (impl_->connection_ == nullptr) {
855-
return impl_->querystring;
856-
}
857-
858-
MHD_get_connection_values(impl_->connection_, MHD_GET_ARGUMENT_KIND,
859-
&detail::http_request_impl::build_request_querystring,
860-
reinterpret_cast<void*>(&impl_->querystring));
861-
862+
std::string_view http_request::get_querystring() const noexcept {
863+
// TASK-018: querystring is assembled eagerly during construction (live
864+
// MHD path) or set directly by create_test_request (test path), so the
865+
// reader is a trivial member access -- genuinely noexcept.
862866
return impl_->querystring;
863867
}
864868

src/httpserver/http_request.hpp

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,14 @@ class http_request {
165165
#endif // HAVE_BAUTH
166166

167167
/**
168-
* Method used to get the path requested
169-
* @return string representing the path requested.
170-
**/
171-
std::string_view get_path() const {
168+
* Method used to get the path requested.
169+
* @return string_view spelling the request path.
170+
* @note The returned view aliases storage owned by this http_request and
171+
* is only valid for the lifetime of the request object (typically
172+
* the duration of the handler invocation). Copy into std::string
173+
* to extend the lifetime.
174+
**/
175+
std::string_view get_path() const noexcept {
172176
return path;
173177
}
174178

@@ -188,9 +192,13 @@ class http_request {
188192

189193
/**
190194
* Method used to get the METHOD used to make the request.
191-
* @return string representing the method.
195+
* @return string_view spelling the request method (GET / POST / ...).
196+
* @note The returned view aliases storage owned by this http_request and
197+
* is only valid for the lifetime of the request object (typically
198+
* the duration of the handler invocation). Copy into std::string
199+
* to extend the lifetime.
192200
**/
193-
std::string_view get_method() const {
201+
std::string_view get_method() const noexcept {
194202
return method;
195203
}
196204

@@ -289,9 +297,13 @@ class http_request {
289297

290298
/**
291299
* Method used to get the content of the request.
292-
* @return the content in string representation
300+
* @return string_view over the request body.
301+
* @note The returned view aliases storage owned by this http_request and
302+
* is only valid for the lifetime of the request object (typically
303+
* the duration of the handler invocation). Copy into std::string
304+
* to extend the lifetime.
293305
**/
294-
std::string_view get_content() const {
306+
std::string_view get_content() const noexcept {
295307
return content;
296308
}
297309

@@ -304,16 +316,26 @@ class http_request {
304316
}
305317
/**
306318
* Method used to get the content of the query string.
307-
* @return the query string in string representation.
308-
* @note The returned view is only valid within the handler's call frame.
319+
* @return string_view over the assembled query string (e.g. "?a=1&b=2"),
320+
* empty when no query was supplied.
321+
* @note The returned view aliases storage owned by this http_request and
322+
* is only valid for the lifetime of the request object (typically
323+
* the duration of the handler invocation). Copy into std::string
324+
* to extend the lifetime.
325+
* @note (TASK-018) The querystring is assembled eagerly at construction
326+
* on the live MHD path, so the public reader is `noexcept`.
309327
**/
310-
std::string_view get_querystring() const;
328+
std::string_view get_querystring() const noexcept;
311329

312330
/**
313331
* Method used to get the version of the request.
314-
* @return the version in string representation
332+
* @return string_view spelling the HTTP version (e.g. "HTTP/1.1").
333+
* @note The returned view aliases storage owned by this http_request and
334+
* is only valid for the lifetime of the request object (typically
335+
* the duration of the handler invocation). Copy into std::string
336+
* to extend the lifetime.
315337
**/
316-
std::string_view get_version() const {
338+
std::string_view get_version() const noexcept {
317339
return version;
318340
}
319341

test/Makefile.am

Lines changed: 11 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 create_test_request http_request_arena
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 http_request_arena http_request_const_getters
3030

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

@@ -126,6 +126,16 @@ create_test_request_LDADD = $(LDADD) -lmicrohttpd
126126
http_request_arena_SOURCES = unit/http_request_arena_test.cpp
127127
http_request_arena_LDADD = $(LDADD) -lmicrohttpd
128128

129+
# http_request_const_getters: TASK-018 sentinel. Compile-time assertions
130+
# that the per-key getters (get_header / get_cookie / get_footer /
131+
# get_arg / get_arg_flat) and the always-present getters (get_path /
132+
# get_method / get_version / get_content / get_querystring) are callable
133+
# on `const http_request&` and (for the always-present five) noexcept.
134+
# Locks the public-API contract via static_assert so accidental drift is
135+
# caught at compile time. Pure compile test -- empty LDADD.
136+
http_request_const_getters_SOURCES = unit/http_request_const_getters_test.cpp
137+
http_request_const_getters_LDADD =
138+
129139
noinst_HEADERS = littletest.hpp
130140
AM_CXXFLAGS += -Wall -fPIC -Wno-overloaded-virtual
131141

test/unit/create_test_request_test.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,62 @@ LT_BEGIN_AUTO_TEST(create_test_request_suite, getters_return_const_ref_stable)
329329
LT_CHECK_EQ(cref.get_path_pieces().size(), static_cast<size_t>(3));
330330
LT_END_AUTO_TEST(getters_return_const_ref_stable)
331331

332+
// TASK-018: per-key getters must be empty-on-miss and must NOT insert
333+
// the missing key into the underlying maps. We assert this externally by
334+
// observing the public container-getter sizes before and after a series
335+
// of misses. The container caches are built on the first call (so we
336+
// snapshot the baseline AFTER the first call), then we hammer the per-key
337+
// getters with missing keys and confirm the container sizes haven't grown.
338+
LT_BEGIN_AUTO_TEST(create_test_request_suite, missing_key_does_not_insert)
339+
auto req = create_test_request()
340+
.header("Present", "yes")
341+
.footer("AlsoPresent", "yes")
342+
.cookie("CookiePresent", "yes")
343+
.arg("argKey", "v")
344+
.build();
345+
const httpserver::http_request& r = req;
346+
347+
// Build the container caches once so the size snapshot is stable.
348+
const auto headers_before = r.get_headers().size();
349+
const auto footers_before = r.get_footers().size();
350+
const auto cookies_before = r.get_cookies().size();
351+
const auto args_before = r.get_args().size();
352+
353+
// Five misses on each kind. Each must return empty and must NOT
354+
// insert into the underlying maps.
355+
for (int i = 0; i < 5; ++i) {
356+
LT_CHECK(r.get_header("Missing-Header").empty());
357+
LT_CHECK(r.get_footer("Missing-Footer").empty());
358+
LT_CHECK(r.get_cookie("Missing-Cookie").empty());
359+
LT_CHECK(r.get_arg_flat("Missing-Arg").empty());
360+
LT_CHECK(r.get_arg("Missing-Arg").values.empty());
361+
}
362+
363+
// The container caches expose the underlying map sizes. If any of
364+
// the per-key misses had inserted, these would have grown.
365+
LT_CHECK_EQ(r.get_headers().size(), headers_before);
366+
LT_CHECK_EQ(r.get_footers().size(), footers_before);
367+
LT_CHECK_EQ(r.get_cookies().size(), cookies_before);
368+
LT_CHECK_EQ(r.get_args().size(), args_before);
369+
LT_END_AUTO_TEST(missing_key_does_not_insert)
370+
371+
// TASK-018: per-key getters return string_view aliasing the request's
372+
// owned storage and surface the correct value on a hit.
373+
LT_BEGIN_AUTO_TEST(create_test_request_suite, getters_return_string_view_correct_value)
374+
auto req = create_test_request()
375+
.header("X-Foo", "foo-value")
376+
.footer("X-Bar", "bar-value")
377+
.cookie("session", "sess-value")
378+
.arg("q", "q-value")
379+
.build();
380+
const httpserver::http_request& r = req;
381+
382+
LT_CHECK_EQ(std::string(r.get_header("X-Foo")), std::string("foo-value"));
383+
LT_CHECK_EQ(std::string(r.get_footer("X-Bar")), std::string("bar-value"));
384+
LT_CHECK_EQ(std::string(r.get_cookie("session")), std::string("sess-value"));
385+
LT_CHECK_EQ(std::string(r.get_arg_flat("q")), std::string("q-value"));
386+
LT_END_AUTO_TEST(getters_return_string_view_correct_value)
387+
332388
LT_BEGIN_AUTO_TEST_ENV()
333389
AUTORUN_TESTS()
334390
LT_END_AUTO_TEST_ENV()
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2026 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+
11+
// TASK-018: compile-time guarantees of http_request's per-key and
12+
// always-present getters.
13+
//
14+
// We assert the structural invariants TASK-018 owns:
15+
// 1. Every per-key getter (get_header / get_cookie / get_footer /
16+
// get_arg / get_arg_flat) is callable on `const http_request&` with
17+
// a `std::string_view` key.
18+
// 2. Every always-present getter (get_path / get_method / get_version
19+
// / get_content / get_querystring) is callable on `const
20+
// http_request&` and returns `std::string_view`.
21+
// 3. The five always-present getters above are `noexcept`. The
22+
// string_view-of-std::string conversion is itself noexcept since
23+
// C++17, so every getter that just hands back a member's string can
24+
// lock that contract in.
25+
// 4. Return-type lockdowns: every per-key getter returns
26+
// `std::string_view`, except `get_arg` which deliberately returns
27+
// `httpserver::http_arg_value` to preserve multi-value semantics
28+
// (see TASK-018 plan section 4 for the rationale).
29+
//
30+
// The intent of these static_asserts is to catch silent regressions:
31+
// e.g. if a future refactor narrows `get_arg`'s signature without
32+
// adjusting the multi-value tests, or if someone strips `noexcept` off
33+
// `get_path`, the build breaks at compile time rather than at the next
34+
// integration-test failure.
35+
36+
// HTTPSERVER_COMPILATION is supplied by test/Makefile.am AM_CPPFLAGS.
37+
#include "httpserver/http_request.hpp"
38+
#include "httpserver/http_arg_value.hpp"
39+
40+
#include <string_view>
41+
#include <type_traits>
42+
#include <utility>
43+
44+
namespace {
45+
46+
using h = httpserver::http_request;
47+
using cref = const h&;
48+
49+
// (1) Const-callable invocability for every per-key getter.
50+
static_assert(std::is_invocable_v<decltype(&h::get_header), cref, std::string_view>,
51+
"get_header must be invocable on const http_request& with string_view");
52+
static_assert(std::is_invocable_v<decltype(&h::get_cookie), cref, std::string_view>,
53+
"get_cookie must be invocable on const http_request& with string_view");
54+
static_assert(std::is_invocable_v<decltype(&h::get_footer), cref, std::string_view>,
55+
"get_footer must be invocable on const http_request& with string_view");
56+
static_assert(std::is_invocable_v<decltype(&h::get_arg), cref, std::string_view>,
57+
"get_arg must be invocable on const http_request& with string_view");
58+
static_assert(std::is_invocable_v<decltype(&h::get_arg_flat), cref, std::string_view>,
59+
"get_arg_flat must be invocable on const http_request& with string_view");
60+
61+
// (2) Const-callable invocability for every always-present getter.
62+
static_assert(std::is_invocable_v<decltype(&h::get_path), cref>,
63+
"get_path must be invocable on const http_request&");
64+
static_assert(std::is_invocable_v<decltype(&h::get_method), cref>,
65+
"get_method must be invocable on const http_request&");
66+
static_assert(std::is_invocable_v<decltype(&h::get_version), cref>,
67+
"get_version must be invocable on const http_request&");
68+
static_assert(std::is_invocable_v<decltype(&h::get_content), cref>,
69+
"get_content must be invocable on const http_request&");
70+
static_assert(std::is_invocable_v<decltype(&h::get_querystring), cref>,
71+
"get_querystring must be invocable on const http_request&");
72+
73+
// (3) `noexcept` lockdowns for the five always-present getters.
74+
static_assert(noexcept(std::declval<cref>().get_path()),
75+
"get_path must be noexcept");
76+
static_assert(noexcept(std::declval<cref>().get_method()),
77+
"get_method must be noexcept");
78+
static_assert(noexcept(std::declval<cref>().get_version()),
79+
"get_version must be noexcept");
80+
static_assert(noexcept(std::declval<cref>().get_content()),
81+
"get_content must be noexcept");
82+
static_assert(noexcept(std::declval<cref>().get_querystring()),
83+
"get_querystring must be noexcept");
84+
85+
// (4) Return-type lockdowns: per-key getters narrow to string_view,
86+
// except get_arg which preserves multi-value semantics via http_arg_value.
87+
static_assert(std::is_same_v<
88+
decltype(std::declval<cref>().get_header(std::string_view{})),
89+
std::string_view>,
90+
"get_header must return std::string_view");
91+
static_assert(std::is_same_v<
92+
decltype(std::declval<cref>().get_cookie(std::string_view{})),
93+
std::string_view>,
94+
"get_cookie must return std::string_view");
95+
static_assert(std::is_same_v<
96+
decltype(std::declval<cref>().get_footer(std::string_view{})),
97+
std::string_view>,
98+
"get_footer must return std::string_view");
99+
static_assert(std::is_same_v<
100+
decltype(std::declval<cref>().get_arg_flat(std::string_view{})),
101+
std::string_view>,
102+
"get_arg_flat must return std::string_view");
103+
static_assert(std::is_same_v<
104+
decltype(std::declval<cref>().get_arg(std::string_view{})),
105+
httpserver::http_arg_value>,
106+
"get_arg must return http_arg_value (multi-value semantics)");
107+
108+
// Always-present getters all return string_view.
109+
static_assert(std::is_same_v<
110+
decltype(std::declval<cref>().get_path()),
111+
std::string_view>,
112+
"get_path must return std::string_view");
113+
static_assert(std::is_same_v<
114+
decltype(std::declval<cref>().get_method()),
115+
std::string_view>,
116+
"get_method must return std::string_view");
117+
static_assert(std::is_same_v<
118+
decltype(std::declval<cref>().get_version()),
119+
std::string_view>,
120+
"get_version must return std::string_view");
121+
static_assert(std::is_same_v<
122+
decltype(std::declval<cref>().get_content()),
123+
std::string_view>,
124+
"get_content must return std::string_view");
125+
static_assert(std::is_same_v<
126+
decltype(std::declval<cref>().get_querystring()),
127+
std::string_view>,
128+
"get_querystring must return std::string_view");
129+
130+
} // namespace
131+
132+
int main() { return 0; }

0 commit comments

Comments
 (0)