Skip to content

Commit d383a4e

Browse files
etrclaude
andcommitted
TASK-021: http_resource allow-mask via method_set
Replace std::map<std::string, bool> method_state with a 32-bit method_set bitmask. Public API now takes http_method enum values instead of strings; is_allowed and get_allowed_methods are const noexcept; sizeof(http_resource) collapses to vptr + uint32_t + padding. The wire-string-to-enum decode happens once at the dispatch boundary (webserver_impl::answer_to_connection) and the result is cached on modded_request::method_enum so finalize_answer can ask is_allowed without a per-request strcmp loop. Allow-header serialization moves to enum-declaration order; the only in-tree assertion ("HEAD, POST") is preserved by coincidence of head=1, post=2. In-tree string callers (test/integ/basic.cpp, test/integ/ws_start_stop.cpp, examples/allowing_disallowing_methods.cpp, examples/custom_error.cpp) migrated to the typed API. resource_init and the std::map data member are gone; http_resource.hpp drops <map>/<string>/<vector>/<utility>/ <iostream>, leaving only <memory> and httpserver/http_method.hpp. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d2e137f commit d383a4e

12 files changed

Lines changed: 215 additions & 216 deletions

File tree

examples/allowing_disallowing_methods.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ int main() {
3434

3535
hello_world_resource hwr;
3636
hwr.disallow_all();
37-
hwr.set_allowing("GET", true);
37+
hwr.set_allowing(httpserver::http_method::get, true);
3838
ws.register_resource("/hello", &hwr);
3939
ws.start(true);
4040

examples/custom_error.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ int main() {
4444

4545
hello_world_resource hwr;
4646
hwr.disallow_all();
47-
hwr.set_allowing("GET", true);
47+
hwr.set_allowing(httpserver::http_method::get, true);
4848
ws.register_resource("/hello", &hwr);
4949
ws.start(true);
5050

specs/tasks/M4-handlers/TASK-021.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88
Replace `http_resource`'s `std::map<std::string, bool> method_state` with a `method_set` bitmask, shrink `sizeof(http_resource)`, and make `is_allowed`/`get_allowed_methods` const.
99

1010
**Action Items:**
11-
- [ ] Replace `std::map<std::string, bool> method_state` with `method_set methods_allowed_;` member.
12-
- [ ] `bool is_allowed(http_method m) const noexcept` returns `methods_allowed_.contains(m)`.
13-
- [ ] `method_set get_allowed_methods() const noexcept` returns `methods_allowed_` by value.
14-
- [ ] `void set_allowing(http_method m, bool allow) noexcept` (mutator stays non-const).
15-
- [ ] `void allow_all() noexcept;` `void disallow_all() noexcept;`
16-
- [ ] Convert internal v1 callers that passed method names as strings to use `http_method` enum values; provide a string→enum helper if existing user-facing setters need to keep their string form.
11+
- [x] Replace `std::map<std::string, bool> method_state` with `method_set methods_allowed_;` member.
12+
- [x] `bool is_allowed(http_method m) const noexcept` returns `methods_allowed_.contains(m)`.
13+
- [x] `method_set get_allowed_methods() const noexcept` returns `methods_allowed_` by value.
14+
- [x] `void set_allowing(http_method m, bool allow) noexcept` (mutator stays non-const).
15+
- [x] `void allow_all() noexcept;` `void disallow_all() noexcept;`
16+
- [x] Convert internal v1 callers that passed method names as strings to use `http_method` enum values. The string-based public API (`set_allowing(const std::string&, bool)` / `is_allowed(const std::string&)` / vector-returning `get_allowed_methods()`) was removed outright on this branch (Decision 1 in plan). The wire-string-to-enum decode happens once at `webserver_impl::answer_to_connection` and the result is recorded on `modded_request::method_enum`; no public string-to-enum helper is shipped.
1717

1818
**Dependencies:**
1919
- Blocked by: TASK-005
@@ -29,4 +29,4 @@ Replace `http_resource`'s `std::map<std::string, bool> method_state` with a `met
2929
**Related Requirements:** PRD-REQ-REQ-002, PRD-REQ-REQ-003
3030
**Related Decisions:** DR-006, §4.4
3131

32-
**Status:** Not Started
32+
**Status:** Done

specs/tasks/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ Nominally: **13 sequential tasks**, each S–XL. Most other tasks parallelize of
103103
| TASK-018 | `http_request` single-key getters return `string_view`, all const | M3 | Done | TASK-015, TASK-016 |
104104
| TASK-019 | High-level GnuTLS accessors replacing `gnutls_session_t` | M3 | Done | TASK-015 |
105105
| TASK-020 | Final public-header backend-include sweep | M3 | Done | TASK-014, TASK-015, TASK-019 |
106-
| TASK-021 | `http_resource` allow-mask via `method_set` | M4 | Not Started | TASK-005 |
106+
| TASK-021 | `http_resource` allow-mask via `method_set` | M4 | Done | TASK-005 |
107107
| TASK-022 | Snake_case `render_*` overrides on `http_resource` | M4 | Not Started | TASK-021 |
108108
| TASK-023 | Smart-pointer `register_resource` overloads | M4 | Not Started | TASK-014 |
109109
| TASK-024 | `register_path` and `register_prefix` (replace `bool family`) | M4 | Not Started | TASK-023 |

src/http_resource.cpp

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,28 +19,13 @@
1919
*/
2020

2121
#include "httpserver/http_resource.hpp"
22-
#include <microhttpd.h>
23-
#include <iosfwd>
24-
#include <map>
22+
2523
#include <memory>
26-
#include <string>
24+
2725
#include "httpserver/http_response.hpp"
2826

2927
namespace httpserver {
3028

31-
// RESOURCE
32-
void resource_init(std::map<std::string, bool>* method_state) {
33-
(*method_state)[MHD_HTTP_METHOD_GET] = true;
34-
(*method_state)[MHD_HTTP_METHOD_POST] = true;
35-
(*method_state)[MHD_HTTP_METHOD_PUT] = true;
36-
(*method_state)[MHD_HTTP_METHOD_HEAD] = true;
37-
(*method_state)[MHD_HTTP_METHOD_DELETE] = true;
38-
(*method_state)[MHD_HTTP_METHOD_TRACE] = true;
39-
(*method_state)[MHD_HTTP_METHOD_CONNECT] = true;
40-
(*method_state)[MHD_HTTP_METHOD_OPTIONS] = true;
41-
(*method_state)[MHD_HTTP_METHOD_PATCH] = true;
42-
}
43-
4429
namespace detail {
4530

4631
std::shared_ptr<http_response> empty_render(const http_request&) {

src/httpserver/detail/modded_request.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <memory>
3232
#include <fstream>
3333

34+
#include "httpserver/http_method.hpp"
3435
#include "httpserver/http_request.hpp"
3536

3637
namespace httpserver {
@@ -45,6 +46,14 @@ struct modded_request {
4546

4647
std::shared_ptr<http_response> (httpserver::http_resource::*callback)(const httpserver::http_request&);
4748

49+
// TASK-021: enum form of the wire method, decoded once at the
50+
// dispatch boundary in webserver_impl::answer_to_connection. Used
51+
// by finalize_answer to ask http_resource::is_allowed without a
52+
// per-request string compare. Defaults to count_ — a sentinel
53+
// outside the valid_method_mask, so is_allowed returns false for
54+
// unrecognized verbs (the 405 path).
55+
httpserver::http_method method_enum = httpserver::http_method::count_;
56+
4857
std::unique_ptr<http_request> dhr = nullptr;
4958
std::shared_ptr<http_response> dhrs;
5059
bool has_body = false;

src/httpserver/detail/webserver_impl.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,10 @@ class webserver_impl {
235235
MHD_Result requests_answer_second_step(MHD_Connection* connection,
236236
const char* method, const char* version, const char* upload_data,
237237
size_t* upload_data_size, modded_request* mr);
238-
MHD_Result finalize_answer(MHD_Connection* connection, modded_request* mr,
239-
const char* method);
238+
// TASK-021: the wire-string `method` parameter was dropped because
239+
// finalize_answer now consults mr->method_enum (set once by
240+
// answer_to_connection) for the is_allowed check.
241+
MHD_Result finalize_answer(MHD_Connection* connection, modded_request* mr);
240242
MHD_Result complete_request(MHD_Connection* connection, modded_request* mr,
241243
const char* version, const char* method);
242244
struct MHD_Response* get_raw_response_with_fallback(modded_request* mr);

src/httpserver/http_resource.hpp

Lines changed: 45 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,9 @@
2525
#ifndef SRC_HTTPSERVER_HTTP_RESOURCE_HPP_
2626
#define SRC_HTTPSERVER_HTTP_RESOURCE_HPP_
2727

28-
#ifdef DEBUG
29-
#include <iostream>
30-
#endif
31-
32-
#include <map>
3328
#include <memory>
34-
#include <string>
35-
#include <utility>
36-
#include <vector>
29+
30+
#include "httpserver/http_method.hpp"
3731

3832
namespace httpserver { class http_request; }
3933
namespace httpserver { class http_response; }
@@ -44,8 +38,6 @@ namespace httpserver {
4438

4539
namespace detail { std::shared_ptr<http_response> empty_render(const http_request& r); }
4640

47-
void resource_init(std::map<std::string, bool>* res);
48-
4941
/**
5042
* Class representing a callable http resource.
5143
**/
@@ -147,81 +139,62 @@ class http_resource {
147139
}
148140

149141
/**
150-
* Method used to set if a specific method is allowed or not on this request
151-
* @param method method to set permission on
152-
* @param allowed boolean indicating if the method is allowed or not
142+
* Toggle whether a specific http_method is allowed on this resource.
143+
* @param method enum identifying the method (no string lookup)
144+
* @param allow true to enable the method, false to disable it
153145
**/
154-
void set_allowing(const std::string& method, bool allowed) {
155-
if (method_state.count(method)) {
156-
method_state[method] = allowed;
146+
void set_allowing(http_method method, bool allow) noexcept {
147+
if (allow) {
148+
methods_allowed_.set(method);
149+
} else {
150+
methods_allowed_.clear(method);
157151
}
158152
}
159153

160154
/**
161-
* Method used to implicitly allow all methods
155+
* Allow every defined http_method on this resource.
162156
**/
163-
void allow_all() {
164-
std::map<std::string, bool>::iterator it;
165-
for (it=method_state.begin(); it != method_state.end(); ++it) {
166-
method_state[(*it).first] = true;
167-
}
157+
void allow_all() noexcept {
158+
methods_allowed_.set_all();
168159
}
169160

170161
/**
171-
* Method used to implicitly disallow all methods
162+
* Disallow every http_method on this resource.
172163
**/
173-
void disallow_all() {
174-
std::map<std::string, bool>::iterator it;
175-
for (it=method_state.begin(); it != method_state.end(); ++it) {
176-
method_state[(*it).first] = false;
177-
}
164+
void disallow_all() noexcept {
165+
methods_allowed_.clear_all();
178166
}
179167

180168
/**
181-
* Method used to discover if an http method is allowed or not for this resource
182-
* @param method Method to discover allowings
183-
* @return true if the method is allowed
169+
* Test whether `method` is allowed on this resource. Const-noexcept
170+
* because the answer is a single bitmask test on a trivial member;
171+
* no string lookup, no allocation.
172+
* @param method enum identifying the method to query
173+
* @return true if the method is currently allowed
184174
**/
185-
bool is_allowed(const std::string& method) {
186-
if (method_state.count(method)) {
187-
return method_state[method];
188-
} else {
189-
#ifdef DEBUG
190-
std::map<std::string, bool>::iterator it;
191-
for (it = method_state.begin(); it != method_state.end(); ++it) {
192-
std::cout << (*it).first << " -> " << (*it).second << std::endl;
193-
}
194-
#endif // DEBUG
195-
return false;
196-
}
175+
bool is_allowed(http_method method) const noexcept {
176+
return methods_allowed_.contains(method);
197177
}
198178

199179
/**
200-
* Method used to return a list of currently allowed HTTP methods for this resource
201-
* @return vector of strings
180+
* Return the full allow-mask by value. The returned method_set is
181+
* trivially copyable (sizeof == 4) so by-value is the natural ABI.
202182
**/
203-
std::vector<std::string> get_allowed_methods() {
204-
std::vector<std::string> allowed_methods;
205-
206-
for (auto it = method_state.cbegin(); it != method_state.cend(); ++it) {
207-
if ( (*it).second ) {
208-
allowed_methods.push_back((*it).first);
209-
}
210-
}
211-
212-
return allowed_methods;
183+
method_set get_allowed_methods() const noexcept {
184+
return methods_allowed_;
213185
}
214186

215187
protected:
216188
/**
217-
* Constructor of the class
189+
* Constructor of the class. The default state allows every defined
190+
* http_method, matching the v1 behaviour where `resource_init`
191+
* marked all nine methods true.
218192
**/
219-
http_resource() {
220-
resource_init(&method_state);
221-
}
193+
http_resource() = default;
222194

223195
/**
224-
* Copy constructor
196+
* Copy / move special members are trivial — the only data member
197+
* is method_set (a 32-bit aggregate).
225198
**/
226199
http_resource(const http_resource& b) = default;
227200
http_resource(http_resource&& b) noexcept = default;
@@ -231,9 +204,19 @@ class http_resource {
231204
private:
232205
friend class webserver;
233206
friend class detail::webserver_impl; // TASK-014: dispatch helpers
234-
friend void resource_init(std::map<std::string, bool>* res);
235-
std::map<std::string, bool> method_state;
207+
208+
// Default-allow every valid method. method_set::set_all() is
209+
// constexpr, so the chained call is a constant expression and the
210+
// default member initialiser stays well-formed.
211+
method_set methods_allowed_ = method_set{}.set_all();
236212
};
237213

214+
// TASK-021 acceptance: http_resource is now a vptr plus a 32-bit
215+
// method_set plus padding. The cap below leaves headroom for one
216+
// future small member (e.g. an arena tag) without re-invalidating
217+
// PRD-REQ-REQ-002 / PRD-REQ-REQ-003.
218+
static_assert(sizeof(http_resource) <= sizeof(void*) + sizeof(method_set) * 2,
219+
"http_resource should be approximately vptr + method_set");
220+
238221
} // namespace httpserver
239222
#endif // SRC_HTTPSERVER_HTTP_RESOURCE_HPP_

src/webserver.cpp

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,7 +1329,7 @@ struct MHD_Response* webserver_impl::get_raw_response_with_fallback(detail::modd
13291329
}
13301330
}
13311331

1332-
MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection, struct detail::modded_request* mr, const char* method) {
1332+
MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection, struct detail::modded_request* mr) {
13331333
int to_ret = MHD_NO;
13341334

13351335
#ifdef HAVE_WEBSOCKET
@@ -1501,20 +1501,30 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection, struct de
15011501
MHD_destroy_post_processor(mr->pp);
15021502
mr->pp = nullptr;
15031503
}
1504-
if (hrm->is_allowed(method)) {
1504+
if (hrm->is_allowed(mr->method_enum)) {
15051505
mr->dhrs = ((hrm)->*(mr->callback))(*mr->dhr); // copy in memory (move in case)
15061506
if (mr->dhrs.get() == nullptr || mr->dhrs->get_status() == -1) {
15071507
mr->dhrs = internal_error_page(mr);
15081508
}
15091509
} else {
15101510
mr->dhrs = method_not_allowed_page(mr);
15111511

1512-
vector<string> allowed_methods = hrm->get_allowed_methods();
1513-
if (allowed_methods.size() > 0) {
1514-
string header_value = allowed_methods[0];
1515-
for (auto it = allowed_methods.cbegin() + 1; it != allowed_methods.cend(); ++it) {
1516-
header_value += ", " + (*it);
1512+
// TASK-021: serialize the allow-mask in enum-declaration
1513+
// order (GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS,
1514+
// TRACE, PATCH). v1 used std::map iteration order
1515+
// (alphabetical); the only existing assertion in-tree is
1516+
// "HEAD, POST" which is preserved by enum order.
1517+
method_set allowed = hrm->get_allowed_methods();
1518+
string header_value;
1519+
for (std::uint8_t i = 0;
1520+
i < static_cast<std::uint8_t>(http_method::count_); ++i) {
1521+
auto m = static_cast<http_method>(i);
1522+
if (allowed.contains(m)) {
1523+
if (!header_value.empty()) header_value += ", ";
1524+
header_value += std::string(to_string(m));
15171525
}
1526+
}
1527+
if (!header_value.empty()) {
15181528
mr->dhrs->with_header(http_utils::http_header_allow, header_value);
15191529
}
15201530
}
@@ -1550,7 +1560,7 @@ MHD_Result webserver_impl::complete_request(MHD_Connection* connection, struct d
15501560
mr->dhr->set_method(method);
15511561
mr->dhr->set_version(version);
15521562

1553-
return finalize_answer(connection, mr, method);
1563+
return finalize_answer(connection, mr);
15541564
}
15551565

15561566
MHD_Result webserver_impl::answer_to_connection(void* cls, MHD_Connection* connection, const char* url, const char* method,
@@ -1579,29 +1589,44 @@ MHD_Result webserver_impl::answer_to_connection(void* cls, MHD_Connection* conne
15791589
webserver_impl::access_log(impl->parent, mr->complete_uri + " METHOD: " + method);
15801590

15811591
// Case-sensitive per RFC 7230 §3.1.1: HTTP method is case-sensitive.
1592+
// TASK-021: also record the enum form once so finalize_answer can
1593+
// call hrm->is_allowed without re-scanning the wire string.
15821594
if (0 == strcmp(method, http_utils::http_method_get)) {
15831595
mr->callback = &http_resource::render_GET;
1596+
mr->method_enum = http_method::get;
15841597
} else if (0 == strcmp(method, http_utils::http_method_post)) {
15851598
mr->callback = &http_resource::render_POST;
1599+
mr->method_enum = http_method::post;
15861600
mr->has_body = true;
15871601
} else if (0 == strcmp(method, http_utils::http_method_put)) {
15881602
mr->callback = &http_resource::render_PUT;
1603+
mr->method_enum = http_method::put;
15891604
mr->has_body = true;
15901605
} else if (0 == strcmp(method, http_utils::http_method_delete)) {
15911606
mr->callback = &http_resource::render_DELETE;
1607+
mr->method_enum = http_method::del;
15921608
mr->has_body = true;
15931609
} else if (0 == strcmp(method, http_utils::http_method_patch)) {
15941610
mr->callback = &http_resource::render_PATCH;
1611+
mr->method_enum = http_method::patch;
15951612
mr->has_body = true;
15961613
} else if (0 == strcmp(method, http_utils::http_method_head)) {
15971614
mr->callback = &http_resource::render_HEAD;
1615+
mr->method_enum = http_method::head;
15981616
} else if (0 == strcmp(method, http_utils::http_method_connect)) {
15991617
mr->callback = &http_resource::render_CONNECT;
1618+
mr->method_enum = http_method::connect;
16001619
} else if (0 == strcmp(method, http_utils::http_method_trace)) {
16011620
mr->callback = &http_resource::render_TRACE;
1621+
mr->method_enum = http_method::trace;
16021622
} else if (0 == strcmp(method, http_utils::http_method_options)) {
16031623
mr->callback = &http_resource::render_OPTIONS;
1624+
mr->method_enum = http_method::options;
16041625
}
1626+
// Else: mr->method_enum stays at the default (count_), so
1627+
// is_allowed(count_) returns false and the request takes the 405
1628+
// path. Pre-existing latent bug: mr->callback may also be left
1629+
// un-set here; see TASK-027 for the dispatch redesign.
16051630

16061631
return impl->requests_answer_first_step(connection, mr);
16071632
}

0 commit comments

Comments
 (0)