From 1b5fe8fb1389f6f9707b7b89c988ef37213b76b6 Mon Sep 17 00:00:00 2001 From: Sebastiano Merlino Date: Sun, 30 Jun 2024 21:04:51 -0700 Subject: [PATCH] Fix post processor breaking large fields in multiple parts. (#337) * Fix post processor breaking large fields in multiple parts. * Include cstring in basic test * Avoid using strcpy and strcat --- src/http_request.cpp | 15 +++++ src/httpserver/http_request.hpp | 3 + src/webserver.cpp | 6 +- test/integ/basic.cpp | 109 +++++++++++++++++++++++++++++++- 4 files changed, 131 insertions(+), 2 deletions(-) diff --git a/src/http_request.cpp b/src/http_request.cpp index 342450e3..c49c3b8e 100644 --- a/src/http_request.cpp +++ b/src/http_request.cpp @@ -116,6 +116,21 @@ void http_request::populate_args() const { cache->args_populated = true; } + +void http_request::grow_last_arg(const std::string& key, const std::string& value) { + auto it = cache->unescaped_args.find(key); + + if (it != cache->unescaped_args.end()) { + if (!it->second.empty()) { + it->second.back() += value; + } else { + it->second.push_back(value); + } + } else { + cache->unescaped_args[key] = {value}; + } +} + http_arg_value http_request::get_arg(std::string_view key) const { populate_args(); diff --git a/src/httpserver/http_request.hpp b/src/httpserver/http_request.hpp index 1281c972..f01743dc 100644 --- a/src/httpserver/http_request.hpp +++ b/src/httpserver/http_request.hpp @@ -335,6 +335,9 @@ class http_request { void set_arg_flat(const std::string& key, const std::string& value) { cache->unescaped_args[key] = { (value.substr(0, content_size_limit)) }; } + + void grow_last_arg(const std::string& key, const std::string& value); + /** * Method used to set the content of the request * @param content The content to set. diff --git a/src/webserver.cpp b/src/webserver.cpp index 86a34c75..6e0c7ead 100644 --- a/src/webserver.cpp +++ b/src/webserver.cpp @@ -467,12 +467,16 @@ MHD_Result webserver::post_iterator(void *cls, enum MHD_ValueKind kind, const char *transfer_encoding, const char *data, uint64_t off, size_t size) { // Parameter needed to respect MHD interface, but not needed here. std::ignore = kind; - std::ignore = off; struct details::modded_request* mr = (struct details::modded_request*) cls; if (!filename) { // There is no actual file, just set the arg key/value and return. + if (off > 0) { + mr->dhr->grow_last_arg(key, std::string(data, size)); + return MHD_YES; + } + mr->dhr->set_arg(key, std::string(data, size)); return MHD_YES; } diff --git a/test/integ/basic.cpp b/test/integ/basic.cpp index 29c9dac8..3e680cb6 100644 --- a/test/integ/basic.cpp +++ b/test/integ/basic.cpp @@ -19,6 +19,7 @@ */ #include +#include #include #include #include @@ -69,6 +70,26 @@ class simple_resource : public http_resource { } }; +class large_post_resource_last_value : public http_resource { + public: + shared_ptr render_GET(const http_request&) { + return std::make_shared("OK", 200, "text/plain"); + } + shared_ptr render_POST(const http_request& req) { + return std::make_shared(std::string(req.get_arg("arg1").get_all_values().back()), 200, "text/plain"); + } +}; + +class large_post_resource_first_value : public http_resource { + public: + shared_ptr render_GET(const http_request&) { + return std::make_shared("OK", 200, "text/plain"); + } + shared_ptr render_POST(const http_request& req) { + return std::make_shared(std::string(req.get_arg("arg1").get_all_values().front()), 200, "text/plain"); + } +}; + class arg_value_resource : public http_resource { public: shared_ptr render_GET(const http_request&) { @@ -824,6 +845,93 @@ LT_BEGIN_AUTO_TEST(basic_suite, postprocessor) curl_easy_cleanup(curl); LT_END_AUTO_TEST(postprocessor) +LT_BEGIN_AUTO_TEST(basic_suite, postprocessor_large_field_last_field) + large_post_resource_last_value resource; + LT_ASSERT_EQ(true, ws->register_resource("base", &resource)); + curl_global_init(CURL_GLOBAL_ALL); + string s; + CURL *curl = curl_easy_init(); + CURLcode res; + + const int size = 20000; + const char value = 'A'; + const char* prefix = "arg1=BB&arg1="; + + // Calculate the total length of the string + int totalLength = std::strlen(prefix) + size; + + // Allocate memory for the char array + char* cString = new char[totalLength + 1]; // +1 for the null terminator + + // Copy the prefix + int offset = std::snprintf(cString, totalLength + 1, "%s", prefix); + + // Append 20,000 times the character 'A' to the string + for (int i = 0; i < size; ++i) { + cString[offset++] = value; + } + + // Append the suffix + cString[offset] = '\0'; + + curl_easy_setopt(curl, CURLOPT_URL, "localhost:" PORT_STRING "/base"); + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, cString); + 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, std::string(20000, 'A')); + + curl_easy_cleanup(curl); + delete[] cString; +LT_END_AUTO_TEST(postprocessor_large_field_last_field) + +LT_BEGIN_AUTO_TEST(basic_suite, postprocessor_large_field_first_field) + large_post_resource_first_value resource; + LT_ASSERT_EQ(true, ws->register_resource("base", &resource)); + curl_global_init(CURL_GLOBAL_ALL); + string s; + CURL *curl = curl_easy_init(); + CURLcode res; + + const int size = 20000; + const char value = 'A'; + const char* prefix = "arg1="; + const char* middle = "&arg1="; + const char* suffix = "BB"; + + // Calculate the total length of the string + int totalLength = std::strlen(prefix) + size + std::strlen(middle) + std::strlen(suffix); + + // Allocate memory for the char array + char* cString = new char[totalLength + 1]; // +1 for the null terminator + + // Copy the prefix + int offset = std::snprintf(cString, totalLength + 1, "%s", prefix); + + // Append 20,000 times the character 'A' to the string + for (int i = 0; i < size; ++i) { + cString[offset++] = value; + } + + // Append the middle part + offset += std::snprintf(cString + offset, totalLength + 1 - offset, "%s", middle); + + // Append the suffix + std::snprintf(cString + offset, totalLength + 1 - offset, "%s", suffix); + + curl_easy_setopt(curl, CURLOPT_URL, "localhost:" PORT_STRING "/base"); + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, cString); + 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, std::string(20000, 'A')); + + curl_easy_cleanup(curl); + delete[] cString; +LT_END_AUTO_TEST(postprocessor_large_field_first_field) + LT_BEGIN_AUTO_TEST(basic_suite, same_key_different_value) arg_value_resource resource; LT_ASSERT_EQ(true, ws->register_resource("base", &resource)); @@ -844,7 +952,6 @@ LT_BEGIN_AUTO_TEST(basic_suite, same_key_different_value) curl_easy_cleanup(curl); LT_END_AUTO_TEST(same_key_different_value) - LT_BEGIN_AUTO_TEST(basic_suite, same_key_different_value_plain_content) arg_value_resource resource; LT_ASSERT_EQ(true, ws->register_resource("base", &resource));