Skip to content

Commit 1b5fe8f

Browse files
authored
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
1 parent d249ba6 commit 1b5fe8f

File tree

4 files changed

+131
-2
lines changed

4 files changed

+131
-2
lines changed

src/http_request.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,21 @@ void http_request::populate_args() const {
116116
cache->args_populated = true;
117117
}
118118

119+
120+
void http_request::grow_last_arg(const std::string& key, const std::string& value) {
121+
auto it = cache->unescaped_args.find(key);
122+
123+
if (it != cache->unescaped_args.end()) {
124+
if (!it->second.empty()) {
125+
it->second.back() += value;
126+
} else {
127+
it->second.push_back(value);
128+
}
129+
} else {
130+
cache->unescaped_args[key] = {value};
131+
}
132+
}
133+
119134
http_arg_value http_request::get_arg(std::string_view key) const {
120135
populate_args();
121136

src/httpserver/http_request.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,9 @@ class http_request {
335335
void set_arg_flat(const std::string& key, const std::string& value) {
336336
cache->unescaped_args[key] = { (value.substr(0, content_size_limit)) };
337337
}
338+
339+
void grow_last_arg(const std::string& key, const std::string& value);
340+
338341
/**
339342
* Method used to set the content of the request
340343
* @param content The content to set.

src/webserver.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,12 +467,16 @@ MHD_Result webserver::post_iterator(void *cls, enum MHD_ValueKind kind,
467467
const char *transfer_encoding, const char *data, uint64_t off, size_t size) {
468468
// Parameter needed to respect MHD interface, but not needed here.
469469
std::ignore = kind;
470-
std::ignore = off;
471470

472471
struct details::modded_request* mr = (struct details::modded_request*) cls;
473472

474473
if (!filename) {
475474
// There is no actual file, just set the arg key/value and return.
475+
if (off > 0) {
476+
mr->dhr->grow_last_arg(key, std::string(data, size));
477+
return MHD_YES;
478+
}
479+
476480
mr->dhr->set_arg(key, std::string(data, size));
477481
return MHD_YES;
478482
}

test/integ/basic.cpp

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
*/
2020

2121
#include <curl/curl.h>
22+
#include <cstring>
2223
#include <map>
2324
#include <memory>
2425
#include <numeric>
@@ -69,6 +70,26 @@ class simple_resource : public http_resource {
6970
}
7071
};
7172

73+
class large_post_resource_last_value : public http_resource {
74+
public:
75+
shared_ptr<http_response> render_GET(const http_request&) {
76+
return std::make_shared<string_response>("OK", 200, "text/plain");
77+
}
78+
shared_ptr<http_response> render_POST(const http_request& req) {
79+
return std::make_shared<string_response>(std::string(req.get_arg("arg1").get_all_values().back()), 200, "text/plain");
80+
}
81+
};
82+
83+
class large_post_resource_first_value : public http_resource {
84+
public:
85+
shared_ptr<http_response> render_GET(const http_request&) {
86+
return std::make_shared<string_response>("OK", 200, "text/plain");
87+
}
88+
shared_ptr<http_response> render_POST(const http_request& req) {
89+
return std::make_shared<string_response>(std::string(req.get_arg("arg1").get_all_values().front()), 200, "text/plain");
90+
}
91+
};
92+
7293
class arg_value_resource : public http_resource {
7394
public:
7495
shared_ptr<http_response> render_GET(const http_request&) {
@@ -824,6 +845,93 @@ LT_BEGIN_AUTO_TEST(basic_suite, postprocessor)
824845
curl_easy_cleanup(curl);
825846
LT_END_AUTO_TEST(postprocessor)
826847

848+
LT_BEGIN_AUTO_TEST(basic_suite, postprocessor_large_field_last_field)
849+
large_post_resource_last_value resource;
850+
LT_ASSERT_EQ(true, ws->register_resource("base", &resource));
851+
curl_global_init(CURL_GLOBAL_ALL);
852+
string s;
853+
CURL *curl = curl_easy_init();
854+
CURLcode res;
855+
856+
const int size = 20000;
857+
const char value = 'A';
858+
const char* prefix = "arg1=BB&arg1=";
859+
860+
// Calculate the total length of the string
861+
int totalLength = std::strlen(prefix) + size;
862+
863+
// Allocate memory for the char array
864+
char* cString = new char[totalLength + 1]; // +1 for the null terminator
865+
866+
// Copy the prefix
867+
int offset = std::snprintf(cString, totalLength + 1, "%s", prefix);
868+
869+
// Append 20,000 times the character 'A' to the string
870+
for (int i = 0; i < size; ++i) {
871+
cString[offset++] = value;
872+
}
873+
874+
// Append the suffix
875+
cString[offset] = '\0';
876+
877+
curl_easy_setopt(curl, CURLOPT_URL, "localhost:" PORT_STRING "/base");
878+
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, cString);
879+
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc);
880+
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s);
881+
res = curl_easy_perform(curl);
882+
LT_ASSERT_EQ(res, 0);
883+
LT_CHECK_EQ(s, std::string(20000, 'A'));
884+
885+
curl_easy_cleanup(curl);
886+
delete[] cString;
887+
LT_END_AUTO_TEST(postprocessor_large_field_last_field)
888+
889+
LT_BEGIN_AUTO_TEST(basic_suite, postprocessor_large_field_first_field)
890+
large_post_resource_first_value resource;
891+
LT_ASSERT_EQ(true, ws->register_resource("base", &resource));
892+
curl_global_init(CURL_GLOBAL_ALL);
893+
string s;
894+
CURL *curl = curl_easy_init();
895+
CURLcode res;
896+
897+
const int size = 20000;
898+
const char value = 'A';
899+
const char* prefix = "arg1=";
900+
const char* middle = "&arg1=";
901+
const char* suffix = "BB";
902+
903+
// Calculate the total length of the string
904+
int totalLength = std::strlen(prefix) + size + std::strlen(middle) + std::strlen(suffix);
905+
906+
// Allocate memory for the char array
907+
char* cString = new char[totalLength + 1]; // +1 for the null terminator
908+
909+
// Copy the prefix
910+
int offset = std::snprintf(cString, totalLength + 1, "%s", prefix);
911+
912+
// Append 20,000 times the character 'A' to the string
913+
for (int i = 0; i < size; ++i) {
914+
cString[offset++] = value;
915+
}
916+
917+
// Append the middle part
918+
offset += std::snprintf(cString + offset, totalLength + 1 - offset, "%s", middle);
919+
920+
// Append the suffix
921+
std::snprintf(cString + offset, totalLength + 1 - offset, "%s", suffix);
922+
923+
curl_easy_setopt(curl, CURLOPT_URL, "localhost:" PORT_STRING "/base");
924+
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, cString);
925+
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc);
926+
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s);
927+
res = curl_easy_perform(curl);
928+
LT_ASSERT_EQ(res, 0);
929+
LT_CHECK_EQ(s, std::string(20000, 'A'));
930+
931+
curl_easy_cleanup(curl);
932+
delete[] cString;
933+
LT_END_AUTO_TEST(postprocessor_large_field_first_field)
934+
827935
LT_BEGIN_AUTO_TEST(basic_suite, same_key_different_value)
828936
arg_value_resource resource;
829937
LT_ASSERT_EQ(true, ws->register_resource("base", &resource));
@@ -844,7 +952,6 @@ LT_BEGIN_AUTO_TEST(basic_suite, same_key_different_value)
844952
curl_easy_cleanup(curl);
845953
LT_END_AUTO_TEST(same_key_different_value)
846954

847-
848955
LT_BEGIN_AUTO_TEST(basic_suite, same_key_different_value_plain_content)
849956
arg_value_resource resource;
850957
LT_ASSERT_EQ(true, ws->register_resource("base", &resource));

0 commit comments

Comments
 (0)