From 62834b33d74a54942668b54ec9349ee39a11c27f Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Wed, 5 Apr 2023 19:32:28 -0700 Subject: [PATCH 1/6] Add some test of how registired resources interactwith eachother. --- test/integ/basic.cpp | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/test/integ/basic.cpp b/test/integ/basic.cpp index e4e63f474..a983e1be6 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -204,6 +204,17 @@ class nok_resource : public http_resource { } }; +class static_resource : public http_resource { + public: + static_resource(std::string r) : resp(std::move(r)) {} + + shared_ptr render_GET(const http_request& req) { + return shared_ptr(new string_response(resp, 200, "text/plain")); + } + + std::string resp; +}; + class no_response_resource : public http_resource { public: shared_ptr render_GET(const http_request&) { @@ -358,6 +369,68 @@ LT_BEGIN_AUTO_TEST(basic_suite, two_endpoints) } LT_END_AUTO_TEST(two_endpoints) +LT_BEGIN_AUTO_TEST(basic_suite, duplicate_endpoints) + ok_resource ok1, ok2; + LT_CHECK_EQ(true, ws->register_resource("OK", &ok1)); + + // All of these collide and the registration fails + LT_CHECK_EQ(false, ws->register_resource("OK", &ok2)); + LT_CHECK_EQ(false, ws->register_resource("/OK", &ok2)); + LT_CHECK_EQ(false, ws->register_resource("/OK/", &ok2)); + LT_CHECK_EQ(false, ws->register_resource("OK/", &ok2)); + + // Check how family interacts. + LT_CHECK_EQ(false, ws->register_resource("OK", &ok2, true)); + + // Check that switched case does the right thing, whatever that is here. +#ifdef CASE_INSENSITIVE + LT_CHECK_EQ(true, ws->register_resource("ok", &ok2)); +#else + LT_CHECK_EQ(false, ws->register_resource("ok", &ok2)); +#endif +LT_END_AUTO_TEST(duplicate_endpoints) + +LT_BEGIN_AUTO_TEST(basic_suite, overlapping_endpoints) + // Setup two different resources that can both match the same URL. + static_resource ok1("1"), ok2("2"); + LT_CHECK_EQ(true, ws->register_resource("/foo/{var|([a-z]+)}/", &ok1)); + LT_CHECK_EQ(true, ws->register_resource("/{var|([a-z]+)}/bar/", &ok2)); + + curl_global_init(CURL_GLOBAL_ALL); + + { + string s; + CURL *curl = curl_easy_init(); + CURLcode res; + curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/foo/bar/"); + curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s); + res = curl_easy_perform(curl); + LT_ASSERT_EQ(res, 0); + LT_CHECK_EQ(s, "2"); // Not sure why regex wins, but it does... + curl_easy_cleanup(curl); + } + + static_resource ok3("3"); + LT_CHECK_EQ(true, ws->register_resource("/foo/bar/", &ok3)); + + { + // Check that an exact, non-RE match overrides both patterns. + string s; + CURL *curl = curl_easy_init(); + CURLcode res; + curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/foo/bar/"); + curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s); + res = curl_easy_perform(curl); + LT_ASSERT_EQ(res, 0); + LT_CHECK_EQ(s, "3"); + curl_easy_cleanup(curl); + } +LT_END_AUTO_TEST(overlapping_endpoints) + LT_BEGIN_AUTO_TEST(basic_suite, read_body) simple_resource resource; ws->register_resource("base", &resource); @@ -1166,6 +1239,28 @@ 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, non_family_url_with_regex_like_pieces) + ok_resource resource; + ws->register_resource("/settings", &resource); + curl_global_init(CURL_GLOBAL_ALL); + + string s; + CURL *curl = curl_easy_init(); + CURLcode res; + curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/settings/{}"); + curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s); + 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, 404) ; + + curl_easy_cleanup(curl); +LT_END_AUTO_TEST(non_family_url_with_regex_like_pieces) + LT_BEGIN_AUTO_TEST(basic_suite, method_not_allowed_header) simple_resource resource; resource.disallow_all(); From 2ea297f680f63649715eababf2badb824e467638 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Wed, 5 Apr 2023 19:53:49 -0700 Subject: [PATCH 2/6] Add a test case for https://github.com/etr/libhttpserver/issues/308 --- test/integ/basic.cpp | 48 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/test/integ/basic.cpp b/test/integ/basic.cpp index a983e1be6..3775cb78e 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -1256,11 +1256,57 @@ LT_BEGIN_AUTO_TEST(basic_suite, non_family_url_with_regex_like_pieces) int64_t http_code = 0; curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE , &http_code); - LT_ASSERT_EQ(http_code, 404) ; + LT_ASSERT_EQ(http_code, 404); curl_easy_cleanup(curl); LT_END_AUTO_TEST(non_family_url_with_regex_like_pieces) +LT_BEGIN_AUTO_TEST(basic_suite, regex_url_exact_match) + ok_resource resource; + ws->register_resource("/foo/{v|[a-z]}/bar", &resource); + curl_global_init(CURL_GLOBAL_ALL); + + { + string s; + CURL *curl = curl_easy_init(); + CURLcode res; + curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/foo/a/bar/"); + curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s); + 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, 200); + + LT_CHECK_EQ(s, "OK"); + curl_easy_cleanup(curl); + } + + { + string s; + CURL *curl = curl_easy_init(); + CURLcode res; + curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/foo/{v|[a-z]}/bar/"); + curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s); + res = curl_easy_perform(curl); + LT_ASSERT_EQ(res, 0); + + int64_t http_code = 0; + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE , &http_code); +#if 0 // https://github.com/etr/libhttpserver/issues/308 + LT_ASSERT_EQ(http_code, 404); +#else + LT_ASSERT_EQ(http_code, 200); +#endif + curl_easy_cleanup(curl); + } +LT_END_AUTO_TEST(regex_url_exact_match) + LT_BEGIN_AUTO_TEST(basic_suite, method_not_allowed_header) simple_resource resource; resource.disallow_all(); From b4da51d11191d5ff82a7f0bc8b55ea5a1ccb18d6 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Wed, 5 Apr 2023 20:05:18 -0700 Subject: [PATCH 3/6] Make more picky warning settings happy. --- 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 3775cb78e..29e69c188 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -208,7 +208,7 @@ class static_resource : public http_resource { public: static_resource(std::string r) : resp(std::move(r)) {} - shared_ptr render_GET(const http_request& req) { + shared_ptr render_GET(const http_request&) { return shared_ptr(new string_response(resp, 200, "text/plain")); } From 735a9afa1f6ac3a5d0c031f7b8b12208692ad3d9 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Wed, 5 Apr 2023 20:53:56 -0700 Subject: [PATCH 4/6] Make lint happy. --- 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 29e69c188..8ef99472c 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -206,7 +206,7 @@ class nok_resource : public http_resource { class static_resource : public http_resource { public: - static_resource(std::string r) : resp(std::move(r)) {} + explicit static_resource(std::string r) : resp(std::move(r)) {} shared_ptr render_GET(const http_request&) { return shared_ptr(new string_response(resp, 200, "text/plain")); From 518a966890d595b5889dd9966f488ac65adda319 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Wed, 5 Apr 2023 21:58:05 -0700 Subject: [PATCH 5/6] Correct a test cases for -DCASE_INSENSITIVE Also add a comment that the non `-DCASE_INSENSITIVE` testcases is probably wrong. --- test/integ/basic.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integ/basic.cpp b/test/integ/basic.cpp index 8ef99472c..d9c1477b3 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -384,8 +384,10 @@ LT_BEGIN_AUTO_TEST(basic_suite, duplicate_endpoints) // Check that switched case does the right thing, whatever that is here. #ifdef CASE_INSENSITIVE - LT_CHECK_EQ(true, ws->register_resource("ok", &ok2)); + LT_CHECK_EQ(false, ws->register_resource("ok", &ok2)); #else + // TODO: this should be true. + // However, http_endpoint::operator< is always case-insensitive LT_CHECK_EQ(false, ws->register_resource("ok", &ok2)); #endif LT_END_AUTO_TEST(duplicate_endpoints) From 2660852375aba7e4d0a9dfbc74239aaa8020e552 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Wed, 5 Apr 2023 22:10:51 -0700 Subject: [PATCH 6/6] Make lint happy about the TODO Really this should be assigned to an issue report, not a person, because nobody has agreed to fix it yet. --- 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 d9c1477b3..9dc67b27b 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -386,7 +386,7 @@ LT_BEGIN_AUTO_TEST(basic_suite, duplicate_endpoints) #ifdef CASE_INSENSITIVE LT_CHECK_EQ(false, ws->register_resource("ok", &ok2)); #else - // TODO: this should be true. + // TODO(etr): this should be true. // However, http_endpoint::operator< is always case-insensitive LT_CHECK_EQ(false, ws->register_resource("ok", &ok2)); #endif