Skip to content

Commit

Permalink
Fixed string.data() returning not null terminated string
Browse files Browse the repository at this point in the history
  • Loading branch information
@COM8
COM8 committed May 17, 2020
1 parent 1ae2424 commit 050a46e
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 57 deletions.
2 changes: 1 addition & 1 deletion cpr/auth.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace cpr {

const char* Authentication::GetAuthString() const noexcept {
return auth_string_.data();
return auth_string_.c_str();
}

} // namespace cpr
69 changes: 44 additions & 25 deletions cpr/session.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class Session::Impl {
Response Put();

private:
bool hasBodyOrPayload_{false};

struct CurlHolderDeleter {
void operator()(CurlHolder* holder) {
freeHolder(holder);
Expand All @@ -77,7 +79,7 @@ Session::Impl::Impl() {
// Set up some sensible defaults
auto version_info = curl_version_info(CURLVERSION_NOW);
auto version = std::string{"curl/"} + std::string{version_info->version};
curl_easy_setopt(curl, CURLOPT_USERAGENT, version.data());
curl_easy_setopt(curl, CURLOPT_USERAGENT, version.c_str());
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L);
curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 50L);
Expand Down Expand Up @@ -136,7 +138,7 @@ void Session::Impl::SetHeader(const Header& header) {
header_string += ": " + item->second;
}

auto temp = curl_slist_append(chunk, header_string.data());
auto temp = curl_slist_append(chunk, header_string.c_str());
if (temp) {
chunk = temp;
}
Expand Down Expand Up @@ -193,18 +195,20 @@ void Session::Impl::SetUserAgent(const UserAgent& ua) {
}

void Session::Impl::SetPayload(Payload&& payload) {
hasBodyOrPayload_ = true;
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE_LARGE, payload.content.length());
curl_easy_setopt(curl, CURLOPT_COPYPOSTFIELDS, payload.content.data());
curl_easy_setopt(curl, CURLOPT_COPYPOSTFIELDS, payload.content.c_str());
}
}

void Session::Impl::SetPayload(const Payload& payload) {
hasBodyOrPayload_ = true;
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE_LARGE, payload.content.length());
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, payload.content.data());
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, payload.content.c_str());
}
}

Expand All @@ -224,25 +228,26 @@ void Session::Impl::SetMultipart(Multipart&& multipart) {

for (auto& part : multipart.parts) {
std::vector<struct curl_forms> formdata;
formdata.push_back({CURLFORM_COPYNAME, part.name.data()});
formdata.push_back({CURLFORM_COPYNAME, part.name.c_str()});
if (part.is_buffer) {
formdata.push_back({CURLFORM_BUFFER, part.value.data()});
formdata.push_back({CURLFORM_BUFFER, part.value.c_str()});
formdata.push_back(
{CURLFORM_COPYCONTENTS, reinterpret_cast<const char*>(part.data)});
formdata.push_back(
{CURLFORM_CONTENTSLENGTH, reinterpret_cast<const char*>(part.datalen)});
} else if (part.is_file) {
formdata.push_back({CURLFORM_FILE, part.value.data()});
formdata.push_back({CURLFORM_FILE, part.value.c_str()});
} else {
formdata.push_back({CURLFORM_COPYCONTENTS, part.value.data()});
formdata.push_back({CURLFORM_COPYCONTENTS, part.value.c_str()});
}
if (!part.content_type.empty()) {
formdata.push_back({CURLFORM_CONTENTTYPE, part.content_type.data()});
formdata.push_back({CURLFORM_CONTENTTYPE, part.content_type.c_str()});
}
formdata.push_back({CURLFORM_END, nullptr});
curl_formadd(&formpost, &lastptr, CURLFORM_ARRAY, formdata.data(), CURLFORM_END);
}
curl_easy_setopt(curl, CURLOPT_HTTPPOST, formpost);
hasBodyOrPayload_ = true;

curl_formfree(curl_->formpost);
curl_->formpost = formpost;
Expand All @@ -257,24 +262,25 @@ void Session::Impl::SetMultipart(const Multipart& multipart) {

for (auto& part : multipart.parts) {
std::vector<struct curl_forms> formdata;
formdata.push_back({CURLFORM_PTRNAME, part.name.data()});
formdata.push_back({CURLFORM_PTRNAME, part.name.c_str()});
if (part.is_buffer) {
formdata.push_back({CURLFORM_BUFFER, part.value.data()});
formdata.push_back({CURLFORM_BUFFER, part.value.c_str()});
formdata.push_back({CURLFORM_BUFFERPTR, reinterpret_cast<const char*>(part.data)});
formdata.push_back(
{CURLFORM_BUFFERLENGTH, reinterpret_cast<const char*>(part.datalen)});
} else if (part.is_file) {
formdata.push_back({CURLFORM_FILE, part.value.data()});
formdata.push_back({CURLFORM_FILE, part.value.c_str()});
} else {
formdata.push_back({CURLFORM_PTRCONTENTS, part.value.data()});
formdata.push_back({CURLFORM_PTRCONTENTS, part.value.c_str()});
}
if (!part.content_type.empty()) {
formdata.push_back({CURLFORM_CONTENTTYPE, part.content_type.data()});
formdata.push_back({CURLFORM_CONTENTTYPE, part.content_type.c_str()});
}
formdata.push_back({CURLFORM_END, nullptr});
curl_formadd(&formpost, &lastptr, CURLFORM_ARRAY, formdata.data(), CURLFORM_END);
}
curl_easy_setopt(curl, CURLOPT_HTTPPOST, formpost);
hasBodyOrPayload_ = true;

curl_formfree(curl_->formpost);
curl_->formpost = formpost;
Expand Down Expand Up @@ -307,23 +313,25 @@ void Session::Impl::SetCookies(const Cookies& cookies) {
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_COOKIELIST, "ALL");
curl_easy_setopt(curl, CURLOPT_COOKIE, cookies.GetEncoded().data());
curl_easy_setopt(curl, CURLOPT_COOKIE, cookies.GetEncoded().c_str());
}
}

void Session::Impl::SetBody(Body&& body) {
hasBodyOrPayload_ = true;
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE_LARGE, body.length());
curl_easy_setopt(curl, CURLOPT_COPYPOSTFIELDS, body.data());
curl_easy_setopt(curl, CURLOPT_COPYPOSTFIELDS, body.c_str());
}
}

void Session::Impl::SetBody(const Body& body) {
hasBodyOrPayload_ = true;
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE_LARGE, body.length());
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, body.data());
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, body.c_str());
}
}

Expand Down Expand Up @@ -364,7 +372,8 @@ Response Session::Impl::Delete() {
Response Session::Impl::Download(std::ofstream& file) {
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0L);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "GET");
}

return makeDownloadRequest(curl, file);
Expand All @@ -373,7 +382,8 @@ Response Session::Impl::Download(std::ofstream& file) {
Response Session::Impl::Get() {
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0L);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "GET");
}

return makeRequest(curl);
Expand Down Expand Up @@ -412,7 +422,13 @@ Response Session::Impl::Patch() {
Response Session::Impl::Post() {
auto curl = curl_->handle;
if (curl) {
curl_easy_setopt(curl, CURLOPT_NOBODY, 0L);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "POST");

// In case there is no body or payload set it to an empty post:
if (!hasBodyOrPayload_) {
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, "");
}
}

return makeRequest(curl);
Expand All @@ -431,14 +447,14 @@ Response Session::Impl::Put() {
Response Session::Impl::makeDownloadRequest(CURL* curl, std::ofstream& file) {
if (!parameters_.content.empty()) {
Url new_url{url_ + "?" + parameters_.content};
curl_easy_setopt(curl, CURLOPT_URL, new_url.data());
curl_easy_setopt(curl, CURLOPT_URL, new_url.c_str());
} else {
curl_easy_setopt(curl, CURLOPT_URL, url_.data());
curl_easy_setopt(curl, CURLOPT_URL, url_.c_str());
}

auto protocol = url_.substr(0, url_.find(':'));
if (proxies_.has(protocol)) {
curl_easy_setopt(curl, CURLOPT_PROXY, proxies_[protocol].data());
curl_easy_setopt(curl, CURLOPT_PROXY, proxies_[protocol].c_str());
} else {
curl_easy_setopt(curl, CURLOPT_PROXY, "");
}
Expand Down Expand Up @@ -486,14 +502,14 @@ Response Session::Impl::makeDownloadRequest(CURL* curl, std::ofstream& file) {
Response Session::Impl::makeRequest(CURL* curl) {
if (!parameters_.content.empty()) {
Url new_url{url_ + "?" + parameters_.content};
curl_easy_setopt(curl, CURLOPT_URL, new_url.data());
curl_easy_setopt(curl, CURLOPT_URL, new_url.c_str());
} else {
curl_easy_setopt(curl, CURLOPT_URL, url_.data());
curl_easy_setopt(curl, CURLOPT_URL, url_.c_str());
}

auto protocol = url_.substr(0, url_.find(':'));
if (proxies_.has(protocol)) {
curl_easy_setopt(curl, CURLOPT_PROXY, proxies_[protocol].data());
curl_easy_setopt(curl, CURLOPT_PROXY, proxies_[protocol].c_str());
} else {
curl_easy_setopt(curl, CURLOPT_PROXY, nullptr);
}
Expand Down Expand Up @@ -537,6 +553,9 @@ Response Session::Impl::makeRequest(CURL* curl) {
std::string reason;
Header header = cpr::util::parseHeader(header_string, &status_line, &reason);

// Reset the has no body property:
hasBodyOrPayload_ = false;

return Response{static_cast<std::int32_t>(response_code),
std::move(response_string),
std::move(header),
Expand Down
4 changes: 2 additions & 2 deletions test/delete_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ TEST(DeleteTests, SessionDeleteAfterPostTest) {
session.SetUrl(url);
auto response = session.Post();
}
auto url = Url{server->GetBaseUrl() + "/delete.html"};
auto url = Url{server->GetBaseUrl() + "/patch_unallowed.html"};
session.SetUrl(url);
auto response = session.Delete();
auto expected_text = std::string{"Delete success"};
Expand All @@ -190,7 +190,7 @@ TEST(DeleteTests, SessionDeleteUnallowedAfterPostTest) {
auto expected_text = std::string{"Method Not Allowed"};
EXPECT_EQ(expected_text, response.text);
EXPECT_EQ(url, response.url);
EXPECT_EQ(std::string{"text/html"}, response.header["content-type"]);
EXPECT_EQ(std::string{"text/plain"}, response.header["content-type"]);
EXPECT_EQ(405, response.status_code);
EXPECT_EQ(ErrorCode::OK, response.error.code);
}
Expand Down
32 changes: 11 additions & 21 deletions test/httpServer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ void HttpServer::OnRequestBasicAuth(mg_connection* conn, http_message* msg) {
mg_str* requested_auth;
auto auth = std::string{"Basic"};
if ((requested_auth = mg_get_http_header(msg, "Authorization")) == nullptr ||
mg_ncasecmp(requested_auth->p, auth.data(), auth.length()) != 0) {
mg_ncasecmp(requested_auth->p, auth.c_str(), auth.length()) != 0) {
mg_http_send_error(conn, 401, "Unauthorized");
return;
}
Expand Down Expand Up @@ -227,7 +227,7 @@ void HttpServer::OnRequestHeaderReflect(mg_connection* conn, http_message* msg)
}
}
mg_send_head(conn, 200, response.length(), headers.c_str());
mg_send(conn, response.data(), response.length());
mg_send(conn, response.c_str(), response.length());
}

void HttpServer::OnRequestTempRedirect(mg_connection* conn, http_message* msg) {
Expand Down Expand Up @@ -283,7 +283,7 @@ void HttpServer::OnRequestUrlPost(mg_connection* conn, http_message* msg) {
"}"};
}
mg_send_head(conn, 201, response.length(), headers.c_str());
mg_send(conn, response.data(), response.length());
mg_send(conn, response.c_str(), response.length());
}

void HttpServer::OnRequestBodyGet(mg_connection* conn, http_message* msg) {
Expand Down Expand Up @@ -345,12 +345,12 @@ void HttpServer::OnRequestFormPost(mg_connection* conn, http_message* msg) {
forms["y"] +
",\n"
" \"sum\": " +
std::to_string(atoi(forms["x"].data()) + atoi(forms["y"].data())) +
std::to_string(atoi(forms["x"].c_str()) + atoi(forms["y"].c_str())) +
"\n"
"}"};
}
mg_send_head(conn, 201, response.length(), headers.c_str());
mg_send(conn, response.data(), response.length());
mg_send(conn, response.c_str(), response.length());
} // namespace cpr

void HttpServer::OnRequestDelete(mg_connection* conn, http_message* msg) {
Expand Down Expand Up @@ -378,7 +378,7 @@ void HttpServer::OnRequestDelete(mg_connection* conn, http_message* msg) {
response = std::string{msg->body.p, msg->body.len};
}
mg_send_head(conn, 200, response.length(), headers.c_str());
mg_send(conn, response.data(), response.length());
mg_send(conn, response.c_str(), response.length());
} else {
mg_http_send_error(conn, 405, "Method Not Allowed");
}
Expand All @@ -391,7 +391,7 @@ void HttpServer::OnRequestDeleteNotAllowed(mg_connection* conn, http_message* ms
std::string headers = "Content-Type: text/html";
std::string response = "Delete success";
mg_send_head(conn, 200, response.length(), headers.c_str());
mg_send(conn, response.data(), response.length());
mg_send(conn, response.c_str(), response.length());
}
}

Expand Down Expand Up @@ -437,7 +437,7 @@ void HttpServer::OnRequestPatch(mg_connection* conn, http_message* msg) {
"}"};
}
mg_send_head(conn, 200, response.length(), headers.c_str());
mg_send(conn, response.data(), response.length());
mg_send(conn, response.c_str(), response.length());
} else {
mg_http_send_error(conn, 405, "Method Not Allowed");
}
Expand All @@ -448,18 +448,9 @@ void HttpServer::OnRequestPatchNotAllowed(mg_connection* conn, http_message* msg
mg_http_send_error(conn, 405, "Method Not Allowed");
} else {
std::string headers = "Content-Type: text/html";
std::string response = "Patch success";
std::string response = "Delete success";
mg_send_head(conn, 200, response.length(), headers.c_str());
mg_send(conn, response.data(), response.length());
}
}

void HttpServer::OnChunk(mg_connection* conn, http_message* msg) {
std::string uri = std::string(msg->uri.p, msg->uri.len);
if (uri == "/form_post_no_body.html") {
OnRequestFormPost(conn, msg);
} else if (uri == "/url_post_no_body.html") {
OnRequestUrlPost(conn, msg);
mg_send(conn, response.c_str(), response.length());
}
}

Expand Down Expand Up @@ -547,8 +538,7 @@ static void EventHandler(mg_connection* conn, int event, void* event_data) {
break;

case MG_EV_HTTP_CHUNK: {
HttpServer* server = static_cast<HttpServer*>(conn->mgr_data);
server->OnChunk(conn, static_cast<http_message*>(event_data));
/** Do nothing. Just for housekeeping. **/
} break;

case MG_EV_HTTP_REQUEST: {
Expand Down
1 change: 0 additions & 1 deletion test/httpServer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class HttpServer : public testing::Environment {
void Stop();

void OnRequest(mg_connection* conn, http_message* msg);
void OnChunk(mg_connection* conn, http_message* msg);

private:
std::shared_ptr<std::thread> serverThread{nullptr};
Expand Down
8 changes: 4 additions & 4 deletions test/patch_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ TEST(PatchTests, SessionPatchUnallowedAfterHeadTest) {
TEST(PatchTests, SessionPatchAfterPostTest) {
Session session;
{
auto url = Url{server->GetBaseUrl() + "/url_post_no_body.html"};
auto url = Url{server->GetBaseUrl() + "/url_post.html"};
auto payload = Payload{{"x", "5"}};
session.SetUrl(url);
auto response = session.Post();
Expand Down Expand Up @@ -196,7 +196,7 @@ TEST(PatchTests, SessionPatchUnallowedAfterPostTest) {
auto expected_text = std::string{"Method Not Allowed"};
EXPECT_EQ(expected_text, response.text);
EXPECT_EQ(url, response.url);
EXPECT_EQ(std::string{"text/html"}, response.header["content-type"]);
EXPECT_EQ(std::string{"text/plain"}, response.header["content-type"]);
EXPECT_EQ(405, response.status_code);
EXPECT_EQ(ErrorCode::OK, response.error.code);
}
Expand Down Expand Up @@ -225,7 +225,7 @@ TEST(PatchTests, AsyncPatchUnallowedTest) {
auto expected_text = std::string{"Method Not Allowed"};
EXPECT_EQ(expected_text, response.text);
EXPECT_EQ(url, response.url);
EXPECT_EQ(std::string{"text/html"}, response.header["content-type"]);
EXPECT_EQ(std::string{"text/plain"}, response.header["content-type"]);
EXPECT_EQ(405, response.status_code);
EXPECT_EQ(ErrorCode::OK, response.error.code);
}
Expand Down Expand Up @@ -263,7 +263,7 @@ TEST(PatchTests, AsyncMultiplePatchUnallowedTest) {
auto expected_text = std::string{"Method Not Allowed"};
EXPECT_EQ(expected_text, response.text);
EXPECT_EQ(url, response.url);
EXPECT_EQ(std::string{"text/html"}, response.header["content-type"]);
EXPECT_EQ(std::string{"text/plain"}, response.header["content-type"]);
EXPECT_EQ(405, response.status_code);
EXPECT_EQ(ErrorCode::OK, response.error.code);
}
Expand Down
Loading

0 comments on commit 050a46e

Please sign in to comment.