Skip to content

Commit

Permalink
Fix post processor breaking large fields in multiple parts. (#337)
Browse files Browse the repository at this point in the history
* Fix post processor breaking large fields in multiple parts.

* Include cstring in basic test

* Avoid using strcpy and strcat
  • Loading branch information
etr authored Jul 1, 2024
1 parent d249ba6 commit 1b5fe8f
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 2 deletions.
15 changes: 15 additions & 0 deletions src/http_request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
3 changes: 3 additions & 0 deletions src/httpserver/http_request.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion src/webserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
109 changes: 108 additions & 1 deletion test/integ/basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/

#include <curl/curl.h>
#include <cstring>
#include <map>
#include <memory>
#include <numeric>
Expand Down Expand Up @@ -69,6 +70,26 @@ class simple_resource : public http_resource {
}
};

class large_post_resource_last_value : public http_resource {
public:
shared_ptr<http_response> render_GET(const http_request&) {
return std::make_shared<string_response>("OK", 200, "text/plain");
}
shared_ptr<http_response> render_POST(const http_request& req) {
return std::make_shared<string_response>(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<http_response> render_GET(const http_request&) {
return std::make_shared<string_response>("OK", 200, "text/plain");
}
shared_ptr<http_response> render_POST(const http_request& req) {
return std::make_shared<string_response>(std::string(req.get_arg("arg1").get_all_values().front()), 200, "text/plain");
}
};

class arg_value_resource : public http_resource {
public:
shared_ptr<http_response> render_GET(const http_request&) {
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down

0 comments on commit 1b5fe8f

Please sign in to comment.