From d2de18f1b86d8cf9380aebbfb58cb35d945b9672 Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Wed, 10 Nov 2021 10:08:55 +0100 Subject: [PATCH 1/6] 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 --- test/integ/basic.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integ/basic.cpp b/test/integ/basic.cpp index 0334317a..c2ae779c 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -366,7 +366,7 @@ LT_BEGIN_AUTO_TEST(basic_suite, resource_setting_header) curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc); curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s); curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, headerfunc); - curl_easy_setopt(curl, CURLOPT_WRITEHEADER, &ss); + curl_easy_setopt(curl, CURLOPT_HEADERDATA, &ss); res = curl_easy_perform(curl); LT_ASSERT_EQ(res, 0); LT_CHECK_EQ(s, "OK"); From a81eb6ed07988df1881e4700424b1a229d497a92 Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Thu, 11 Nov 2021 16:31:13 +0100 Subject: [PATCH 2/6] 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: #240 Suggested-by: Sebastiano Merlino Signed-off-by: Alexander Dahl --- src/http_resource.cpp | 20 ++++++++++---------- src/httpserver/http_resource.hpp | 22 +++++++++++----------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/http_resource.cpp b/src/http_resource.cpp index 6e75b542..eb1fe976 100644 --- a/src/http_resource.cpp +++ b/src/http_resource.cpp @@ -28,16 +28,16 @@ namespace httpserver { class http_response; } namespace httpserver { // RESOURCE -void resource_init(std::map* allowed_methods) { - (*allowed_methods)[MHD_HTTP_METHOD_GET] = true; - (*allowed_methods)[MHD_HTTP_METHOD_POST] = true; - (*allowed_methods)[MHD_HTTP_METHOD_PUT] = true; - (*allowed_methods)[MHD_HTTP_METHOD_HEAD] = true; - (*allowed_methods)[MHD_HTTP_METHOD_DELETE] = true; - (*allowed_methods)[MHD_HTTP_METHOD_TRACE] = true; - (*allowed_methods)[MHD_HTTP_METHOD_CONNECT] = true; - (*allowed_methods)[MHD_HTTP_METHOD_OPTIONS] = true; - (*allowed_methods)[MHD_HTTP_METHOD_PATCH] = true; +void resource_init(std::map* method_state) { + (*method_state)[MHD_HTTP_METHOD_GET] = true; + (*method_state)[MHD_HTTP_METHOD_POST] = true; + (*method_state)[MHD_HTTP_METHOD_PUT] = true; + (*method_state)[MHD_HTTP_METHOD_HEAD] = true; + (*method_state)[MHD_HTTP_METHOD_DELETE] = true; + (*method_state)[MHD_HTTP_METHOD_TRACE] = true; + (*method_state)[MHD_HTTP_METHOD_CONNECT] = true; + (*method_state)[MHD_HTTP_METHOD_OPTIONS] = true; + (*method_state)[MHD_HTTP_METHOD_PATCH] = true; } namespace details { diff --git a/src/httpserver/http_resource.hpp b/src/httpserver/http_resource.hpp index 0def56d7..c396eaac 100644 --- a/src/httpserver/http_resource.hpp +++ b/src/httpserver/http_resource.hpp @@ -149,8 +149,8 @@ class http_resource { * @param allowed boolean indicating if the method is allowed or not **/ void set_allowing(const std::string& method, bool allowed) { - if (allowed_methods.count(method)) { - allowed_methods[method] = allowed; + if (method_state.count(method)) { + method_state[method] = allowed; } } @@ -159,8 +159,8 @@ class http_resource { **/ void allow_all() { std::map::iterator it; - for (it=allowed_methods.begin(); it != allowed_methods.end(); ++it) { - allowed_methods[(*it).first] = true; + for (it=method_state.begin(); it != method_state.end(); ++it) { + method_state[(*it).first] = true; } } @@ -169,8 +169,8 @@ class http_resource { **/ void disallow_all() { std::map::iterator it; - for (it=allowed_methods.begin(); it != allowed_methods.end(); ++it) { - allowed_methods[(*it).first] = false; + for (it=method_state.begin(); it != method_state.end(); ++it) { + method_state[(*it).first] = false; } } @@ -180,12 +180,12 @@ class http_resource { * @return true if the method is allowed **/ bool is_allowed(const std::string& method) { - if (allowed_methods.count(method)) { - return allowed_methods[method]; + if (method_state.count(method)) { + return method_state[method]; } else { #ifdef DEBUG std::map::iterator it; - for (it = allowed_methods.begin(); it != allowed_methods.end(); ++it) { + for (it = method_state.begin(); it != method_state.end(); ++it) { std::cout << (*it).first << " -> " << (*it).second << std::endl; } #endif // DEBUG @@ -198,7 +198,7 @@ class http_resource { * Constructor of the class **/ http_resource() { - resource_init(&allowed_methods); + resource_init(&method_state); } /** @@ -212,7 +212,7 @@ class http_resource { private: friend class webserver; friend void resource_init(std::map* res); - std::map allowed_methods; + std::map method_state; }; } // namespace httpserver From 51c85a4669d048fffbfcae2721fe38c567804922 Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Thu, 11 Nov 2021 17:19:07 +0100 Subject: [PATCH 3/6] 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: #240 Signed-off-by: Alexander Dahl --- src/httpserver/http_resource.hpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/httpserver/http_resource.hpp b/src/httpserver/http_resource.hpp index c396eaac..9a754718 100644 --- a/src/httpserver/http_resource.hpp +++ b/src/httpserver/http_resource.hpp @@ -33,6 +33,7 @@ #include #include #include +#include namespace httpserver { class http_request; } namespace httpserver { class http_response; } @@ -193,6 +194,22 @@ class http_resource { } } + /** + * Method used to return a list of currently allowed HTTP methods for this resource + * @return vector of strings + **/ + std::vector get_allowed_methods() { + std::vector allowed_methods; + + for (auto it = method_state.cbegin(); it != method_state.cend(); ++it) { + if ( (*it).second ) { + allowed_methods.push_back((*it).first); + } + } + + return allowed_methods; + } + protected: /** * Constructor of the class From dc3c27fc0581b7e9cbde99359223dc9177703458 Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Thu, 11 Nov 2021 17:57:53 +0100 Subject: [PATCH 4/6] test: Introduce new unit test suite for http_resource We want to test the recently introduced new API methods. References: #240 Signed-off-by: Alexander Dahl --- test/Makefile.am | 3 +- test/unit/http_resource_test.cpp | 93 ++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 test/unit/http_resource_test.cpp diff --git a/test/Makefile.am b/test/Makefile.am index e6c91b02..7b8aba08 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -19,7 +19,7 @@ LDADD = $(top_builddir)/src/libhttpserver.la AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/ METASOURCES = AUTO -check_PROGRAMS = basic http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred +check_PROGRAMS = basic http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource MOSTLYCLEANFILES = *.gcda *.gcno *.gcov @@ -33,6 +33,7 @@ http_utils_SOURCES = unit/http_utils_test.cpp string_utilities_SOURCES = unit/string_utilities_test.cpp http_endpoint_SOURCES = unit/http_endpoint_test.cpp nodelay_SOURCES = integ/nodelay.cpp +http_resource_SOURCES = unit/http_resource_test.cpp noinst_HEADERS = littletest.hpp AM_CXXFLAGS += -lcurl -Wall -fPIC diff --git a/test/unit/http_resource_test.cpp b/test/unit/http_resource_test.cpp new file mode 100644 index 00000000..054f21ee --- /dev/null +++ b/test/unit/http_resource_test.cpp @@ -0,0 +1,93 @@ +/* + This file is part of libhttpserver + Copyright (C) 2021 Alexander Dahl + + This library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with this library; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 + USA +*/ + +#include + +#include +#include +#include +#include + +#include "httpserver.hpp" +#include "littletest.hpp" + +using std::shared_ptr; +using std::sort; +using std::string; +using std::vector; + +using httpserver::http_request; +using httpserver::http_resource; +using httpserver::http_response; +using httpserver::string_response; + +class simple_resource : public http_resource { + public: + const shared_ptr render_GET(const http_request&) { + return shared_ptr(new string_response("OK")); + } +}; + +LT_BEGIN_SUITE(http_resource_suite) + void set_up() { + } + + void tear_down() { + } +LT_END_SUITE(http_resource_suite) + +LT_BEGIN_AUTO_TEST(http_resource_suite, disallow_all_methods) + simple_resource sr; + sr.disallow_all(); + auto allowed_methods = sr.get_allowed_methods(); + LT_CHECK_EQ(allowed_methods.size(), 0); +LT_END_AUTO_TEST(disallow_all_methods) + +LT_BEGIN_AUTO_TEST(http_resource_suite, allow_some_methods) + simple_resource sr; + sr.disallow_all(); + sr.set_allowing(MHD_HTTP_METHOD_GET, true); + sr.set_allowing(MHD_HTTP_METHOD_POST, true); + auto allowed_methods = sr.get_allowed_methods(); + LT_CHECK_EQ(allowed_methods.size(), 2); + // elements in http_resource::method_state are sorted (std::map) + vector some_methods{MHD_HTTP_METHOD_GET, MHD_HTTP_METHOD_POST}; + sort(some_methods.begin(), some_methods.end()); + LT_CHECK_COLLECTIONS_EQ(allowed_methods.cbegin(), allowed_methods.cend(), + some_methods.cbegin()) +LT_END_AUTO_TEST(allow_some_methods) + +LT_BEGIN_AUTO_TEST(http_resource_suite, allow_all_methods) + simple_resource sr; + sr.allow_all(); + auto allowed_methods = sr.get_allowed_methods(); + // elements in http_resource::method_state are sorted (std::map) + vector all_methods{MHD_HTTP_METHOD_GET, MHD_HTTP_METHOD_POST, + MHD_HTTP_METHOD_PUT, MHD_HTTP_METHOD_HEAD, MHD_HTTP_METHOD_DELETE, + MHD_HTTP_METHOD_TRACE, MHD_HTTP_METHOD_CONNECT, + MHD_HTTP_METHOD_OPTIONS, MHD_HTTP_METHOD_PATCH}; + sort(all_methods.begin(), all_methods.end()); + LT_CHECK_COLLECTIONS_EQ(allowed_methods.cbegin(), allowed_methods.cend(), + all_methods.cbegin()) +LT_END_AUTO_TEST(allow_all_methods) + +LT_BEGIN_AUTO_TEST_ENV() + AUTORUN_TESTS() +LT_END_AUTO_TEST_ENV() From 883b5059ab34d309819796d3323d97a4674b9256 Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Wed, 10 Nov 2021 10:04:54 +0100 Subject: [PATCH 5/6] 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: #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 --- test/integ/basic.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/integ/basic.cpp b/test/integ/basic.cpp index c2ae779c..e6a0d31f 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -1021,6 +1021,33 @@ LT_BEGIN_AUTO_TEST(basic_suite, url_with_regex_like_pieces) curl_easy_cleanup(curl); LT_END_AUTO_TEST(url_with_regex_like_pieces) +LT_BEGIN_AUTO_TEST(basic_suite, method_not_allowed_header) + simple_resource resource; + resource.disallow_all(); + resource.set_allowing("POST", true); + resource.set_allowing("HEAD", true); + ws->register_resource("base", &resource); + curl_global_init(CURL_GLOBAL_ALL); + string s; + map ss; + CURL *curl = curl_easy_init(); + CURLcode res; + curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/base"); + curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s); + curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, headerfunc); + curl_easy_setopt(curl, CURLOPT_HEADERDATA, &ss); + res = curl_easy_perform(curl); + LT_ASSERT_EQ(res, 0); + int64_t http_code = 0; + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); + LT_ASSERT_EQ(http_code, 405); + // elements in http_resource::method_state are sorted (std::map) + LT_CHECK_EQ(ss["Allow"], "HEAD, POST"); + curl_easy_cleanup(curl); +LT_END_AUTO_TEST(method_not_allowed_header) + LT_BEGIN_AUTO_TEST_ENV() AUTORUN_TESTS() LT_END_AUTO_TEST_ENV() From 538c56006912a07d57c9713232cdc5d862b372ef Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Fri, 12 Nov 2021 10:45:26 +0100 Subject: [PATCH 6/6] 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: #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 --- src/webserver.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/webserver.cpp b/src/webserver.cpp index 40ce85cb..4a1672e7 100644 --- a/src/webserver.cpp +++ b/src/webserver.cpp @@ -601,6 +601,15 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct details } } else { mr->dhrs = method_not_allowed_page(mr); + + vector allowed_methods = hrm->get_allowed_methods(); + if (allowed_methods.size() > 0) { + string header_value = allowed_methods[0]; + for (auto it = allowed_methods.cbegin() + 1; it != allowed_methods.cend(); ++it) { + header_value += ", " + (*it); + } + mr->dhrs->with_header(http_utils::http_header_allow, header_value); + } } } catch(const std::exception& e) { mr->dhrs = internal_error_page(mr);