Skip to content

Commit 6b195cf

Browse files
LeSpockyLeSpocky
authored
Fixed method not allowed to return Allow header (etr#243)
* test: Replace depracted CURL macro According to CURL source that old macro is long gone, and was replaced. Real issue is: you don't find the old macro in CURL's documentation anymore, which makes it harder to understand what libhttpserver is doing here. Signed-off-by: Alexander Dahl <ada@thorsis.com> * http_resource: Rename private member Preparation for adding an interface to get a list of only allowed methods aka methods where the boolean property is true. References: etr#240 Suggested-by: Sebastiano Merlino <sebastiano@hey.com> Signed-off-by: Alexander Dahl <ada@thorsis.com> * http_resource: Add new public class method 'get_allowed_methods' While the class had public methods for setting and testing the currently allowed HTTP methods, there was no way to discover all at once. A hypothetic user could have build such a list by calling 'is_allowed()' on each possible HTTP method, but the http_resource class itself contains the actual comprehensive list (map) of methods, which would make that approach error prone. Such a list is needed however for constructing the HTTP header 'Allow:' which is mandatory under certain conditions according to RFC. References: etr#240 Signed-off-by: Alexander Dahl <ada@thorsis.com> * test: Introduce new unit test suite for http_resource We want to test the recently introduced new API methods. References: etr#240 Signed-off-by: Alexander Dahl <ada@thorsis.com> * test: Add check for 'Allow:' header when returning 405 RFC 7231 states an additional header "Allow:" must be sent along with a response with code 405 (Method Not Allowed). References: etr#240 Link: https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.5 Link: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow Signed-off-by: Alexander Dahl <ada@thorsis.com> * webserver: Send 'Allow:' header along with 405 response RFC 7231 states an additional header "Allow:" MUST be sent along with a response with code 405 (Method Not Allowed). Fixes: etr#240 Link: https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.5 Link: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow Signed-off-by: Alexander Dahl <ada@thorsis.com>
1 parent 8901082 commit 6b195cf

6 files changed

Lines changed: 170 additions & 23 deletions

File tree

‎src/http_resource.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@ namespace httpserver { class http_response; }
2828
namespace httpserver {
2929

3030
// RESOURCE
31-
void resource_init(std::map<std::string, bool>* allowed_methods) {
32-
(*allowed_methods)[MHD_HTTP_METHOD_GET] = true;
33-
(*allowed_methods)[MHD_HTTP_METHOD_POST] = true;
34-
(*allowed_methods)[MHD_HTTP_METHOD_PUT] = true;
35-
(*allowed_methods)[MHD_HTTP_METHOD_HEAD] = true;
36-
(*allowed_methods)[MHD_HTTP_METHOD_DELETE] = true;
37-
(*allowed_methods)[MHD_HTTP_METHOD_TRACE] = true;
38-
(*allowed_methods)[MHD_HTTP_METHOD_CONNECT] = true;
39-
(*allowed_methods)[MHD_HTTP_METHOD_OPTIONS] = true;
40-
(*allowed_methods)[MHD_HTTP_METHOD_PATCH] = true;
31+
void resource_init(std::map<std::string, bool>* method_state) {
32+
(*method_state)[MHD_HTTP_METHOD_GET] = true;
33+
(*method_state)[MHD_HTTP_METHOD_POST] = true;
34+
(*method_state)[MHD_HTTP_METHOD_PUT] = true;
35+
(*method_state)[MHD_HTTP_METHOD_HEAD] = true;
36+
(*method_state)[MHD_HTTP_METHOD_DELETE] = true;
37+
(*method_state)[MHD_HTTP_METHOD_TRACE] = true;
38+
(*method_state)[MHD_HTTP_METHOD_CONNECT] = true;
39+
(*method_state)[MHD_HTTP_METHOD_OPTIONS] = true;
40+
(*method_state)[MHD_HTTP_METHOD_PATCH] = true;
4141
}
4242

4343
namespace details {

‎src/httpserver/http_resource.hpp‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <memory>
3434
#include <string>
3535
#include <utility>
36+
#include <vector>
3637

3738
namespace httpserver { class http_request; }
3839
namespace httpserver { class http_response; }
@@ -149,8 +150,8 @@ class http_resource {
149150
* @param allowed boolean indicating if the method is allowed or not
150151
**/
151152
void set_allowing(const std::string& method, bool allowed) {
152-
if (allowed_methods.count(method)) {
153-
allowed_methods[method] = allowed;
153+
if (method_state.count(method)) {
154+
method_state[method] = allowed;
154155
}
155156
}
156157

@@ -159,8 +160,8 @@ class http_resource {
159160
**/
160161
void allow_all() {
161162
std::map<std::string, bool>::iterator it;
162-
for (it=allowed_methods.begin(); it != allowed_methods.end(); ++it) {
163-
allowed_methods[(*it).first] = true;
163+
for (it=method_state.begin(); it != method_state.end(); ++it) {
164+
method_state[(*it).first] = true;
164165
}
165166
}
166167

@@ -169,8 +170,8 @@ class http_resource {
169170
**/
170171
void disallow_all() {
171172
std::map<std::string, bool>::iterator it;
172-
for (it=allowed_methods.begin(); it != allowed_methods.end(); ++it) {
173-
allowed_methods[(*it).first] = false;
173+
for (it=method_state.begin(); it != method_state.end(); ++it) {
174+
method_state[(*it).first] = false;
174175
}
175176
}
176177

@@ -180,25 +181,41 @@ class http_resource {
180181
* @return true if the method is allowed
181182
**/
182183
bool is_allowed(const std::string& method) {
183-
if (allowed_methods.count(method)) {
184-
return allowed_methods[method];
184+
if (method_state.count(method)) {
185+
return method_state[method];
185186
} else {
186187
#ifdef DEBUG
187188
std::map<std::string, bool>::iterator it;
188-
for (it = allowed_methods.begin(); it != allowed_methods.end(); ++it) {
189+
for (it = method_state.begin(); it != method_state.end(); ++it) {
189190
std::cout << (*it).first << " -> " << (*it).second << std::endl;
190191
}
191192
#endif // DEBUG
192193
return false;
193194
}
194195
}
195196

197+
/**
198+
* Method used to return a list of currently allowed HTTP methods for this resource
199+
* @return vector of strings
200+
**/
201+
std::vector<std::string> get_allowed_methods() {
202+
std::vector<std::string> allowed_methods;
203+
204+
for (auto it = method_state.cbegin(); it != method_state.cend(); ++it) {
205+
if ( (*it).second ) {
206+
allowed_methods.push_back((*it).first);
207+
}
208+
}
209+
210+
return allowed_methods;
211+
}
212+
196213
protected:
197214
/**
198215
* Constructor of the class
199216
**/
200217
http_resource() {
201-
resource_init(&allowed_methods);
218+
resource_init(&method_state);
202219
}
203220

204221
/**
@@ -212,7 +229,7 @@ class http_resource {
212229
private:
213230
friend class webserver;
214231
friend void resource_init(std::map<std::string, bool>* res);
215-
std::map<std::string, bool> allowed_methods;
232+
std::map<std::string, bool> method_state;
216233
};
217234

218235
} // namespace httpserver

‎src/webserver.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,15 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct details
601601
}
602602
} else {
603603
mr->dhrs = method_not_allowed_page(mr);
604+
605+
vector<string> allowed_methods = hrm->get_allowed_methods();
606+
if (allowed_methods.size() > 0) {
607+
string header_value = allowed_methods[0];
608+
for (auto it = allowed_methods.cbegin() + 1; it != allowed_methods.cend(); ++it) {
609+
header_value += ", " + (*it);
610+
}
611+
mr->dhrs->with_header(http_utils::http_header_allow, header_value);
612+
}
604613
}
605614
} catch(const std::exception& e) {
606615
mr->dhrs = internal_error_page(mr);

‎test/Makefile.am‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
LDADD = $(top_builddir)/src/libhttpserver.la
2020
AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/
2121
METASOURCES = AUTO
22-
check_PROGRAMS = basic http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred
22+
check_PROGRAMS = basic http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource
2323

2424
MOSTLYCLEANFILES = *.gcda *.gcno *.gcov
2525

@@ -33,6 +33,7 @@ http_utils_SOURCES = unit/http_utils_test.cpp
3333
string_utilities_SOURCES = unit/string_utilities_test.cpp
3434
http_endpoint_SOURCES = unit/http_endpoint_test.cpp
3535
nodelay_SOURCES = integ/nodelay.cpp
36+
http_resource_SOURCES = unit/http_resource_test.cpp
3637

3738
noinst_HEADERS = littletest.hpp
3839
AM_CXXFLAGS += -lcurl -Wall -fPIC

‎test/integ/basic.cpp‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ LT_BEGIN_AUTO_TEST(basic_suite, resource_setting_header)
366366
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc);
367367
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s);
368368
curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, headerfunc);
369-
curl_easy_setopt(curl, CURLOPT_WRITEHEADER, &ss);
369+
curl_easy_setopt(curl, CURLOPT_HEADERDATA, &ss);
370370
res = curl_easy_perform(curl);
371371
LT_ASSERT_EQ(res, 0);
372372
LT_CHECK_EQ(s, "OK");
@@ -1021,6 +1021,33 @@ LT_BEGIN_AUTO_TEST(basic_suite, url_with_regex_like_pieces)
10211021
curl_easy_cleanup(curl);
10221022
LT_END_AUTO_TEST(url_with_regex_like_pieces)
10231023

1024+
LT_BEGIN_AUTO_TEST(basic_suite, method_not_allowed_header)
1025+
simple_resource resource;
1026+
resource.disallow_all();
1027+
resource.set_allowing("POST", true);
1028+
resource.set_allowing("HEAD", true);
1029+
ws->register_resource("base", &resource);
1030+
curl_global_init(CURL_GLOBAL_ALL);
1031+
string s;
1032+
map<string, string> ss;
1033+
CURL *curl = curl_easy_init();
1034+
CURLcode res;
1035+
curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/base");
1036+
curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);
1037+
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc);
1038+
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s);
1039+
curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, headerfunc);
1040+
curl_easy_setopt(curl, CURLOPT_HEADERDATA, &ss);
1041+
res = curl_easy_perform(curl);
1042+
LT_ASSERT_EQ(res, 0);
1043+
int64_t http_code = 0;
1044+
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
1045+
LT_ASSERT_EQ(http_code, 405);
1046+
// elements in http_resource::method_state are sorted (std::map)
1047+
LT_CHECK_EQ(ss["Allow"], "HEAD, POST");
1048+
curl_easy_cleanup(curl);
1049+
LT_END_AUTO_TEST(method_not_allowed_header)
1050+
10241051
LT_BEGIN_AUTO_TEST_ENV()
10251052
AUTORUN_TESTS()
10261053
LT_END_AUTO_TEST_ENV()

‎test/unit/http_resource_test.cpp‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2021 Alexander Dahl
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+
#include <microhttpd.h>
22+
23+
#include <algorithm>
24+
#include <memory>
25+
#include <string>
26+
#include <vector>
27+
28+
#include "httpserver.hpp"
29+
#include "littletest.hpp"
30+
31+
using std::shared_ptr;
32+
using std::sort;
33+
using std::string;
34+
using std::vector;
35+
36+
using httpserver::http_request;
37+
using httpserver::http_resource;
38+
using httpserver::http_response;
39+
using httpserver::string_response;
40+
41+
class simple_resource : public http_resource {
42+
public:
43+
const shared_ptr<http_response> render_GET(const http_request&) {
44+
return shared_ptr<string_response>(new string_response("OK"));
45+
}
46+
};
47+
48+
LT_BEGIN_SUITE(http_resource_suite)
49+
void set_up() {
50+
}
51+
52+
void tear_down() {
53+
}
54+
LT_END_SUITE(http_resource_suite)
55+
56+
LT_BEGIN_AUTO_TEST(http_resource_suite, disallow_all_methods)
57+
simple_resource sr;
58+
sr.disallow_all();
59+
auto allowed_methods = sr.get_allowed_methods();
60+
LT_CHECK_EQ(allowed_methods.size(), 0);
61+
LT_END_AUTO_TEST(disallow_all_methods)
62+
63+
LT_BEGIN_AUTO_TEST(http_resource_suite, allow_some_methods)
64+
simple_resource sr;
65+
sr.disallow_all();
66+
sr.set_allowing(MHD_HTTP_METHOD_GET, true);
67+
sr.set_allowing(MHD_HTTP_METHOD_POST, true);
68+
auto allowed_methods = sr.get_allowed_methods();
69+
LT_CHECK_EQ(allowed_methods.size(), 2);
70+
// elements in http_resource::method_state are sorted (std::map)
71+
vector<string> some_methods{MHD_HTTP_METHOD_GET, MHD_HTTP_METHOD_POST};
72+
sort(some_methods.begin(), some_methods.end());
73+
LT_CHECK_COLLECTIONS_EQ(allowed_methods.cbegin(), allowed_methods.cend(),
74+
some_methods.cbegin())
75+
LT_END_AUTO_TEST(allow_some_methods)
76+
77+
LT_BEGIN_AUTO_TEST(http_resource_suite, allow_all_methods)
78+
simple_resource sr;
79+
sr.allow_all();
80+
auto allowed_methods = sr.get_allowed_methods();
81+
// elements in http_resource::method_state are sorted (std::map)
82+
vector<string> all_methods{MHD_HTTP_METHOD_GET, MHD_HTTP_METHOD_POST,
83+
MHD_HTTP_METHOD_PUT, MHD_HTTP_METHOD_HEAD, MHD_HTTP_METHOD_DELETE,
84+
MHD_HTTP_METHOD_TRACE, MHD_HTTP_METHOD_CONNECT,
85+
MHD_HTTP_METHOD_OPTIONS, MHD_HTTP_METHOD_PATCH};
86+
sort(all_methods.begin(), all_methods.end());
87+
LT_CHECK_COLLECTIONS_EQ(allowed_methods.cbegin(), allowed_methods.cend(),
88+
all_methods.cbegin())
89+
LT_END_AUTO_TEST(allow_all_methods)
90+
91+
LT_BEGIN_AUTO_TEST_ENV()
92+
AUTORUN_TESTS()
93+
LT_END_AUTO_TEST_ENV()

0 commit comments

Comments
 (0)