From 050a46ee18b708de8101b16d6242202acf9ca93e Mon Sep 17 00:00:00 2001 From: Fabian Sauter Date: Sun, 17 May 2020 15:05:26 +0200 Subject: [PATCH] Fixed string.data() returning not null terminated string --- cpr/auth.cpp | 2 +- cpr/session.cpp | 69 +++++++++++++++++++++++++++---------------- test/delete_tests.cpp | 4 +-- test/httpServer.cpp | 32 +++++++------------- test/httpServer.hpp | 1 - test/patch_tests.cpp | 8 ++--- test/post_tests.cpp | 6 ++-- 7 files changed, 65 insertions(+), 57 deletions(-) diff --git a/cpr/auth.cpp b/cpr/auth.cpp index ff7c3a9c5..7bd357329 100644 --- a/cpr/auth.cpp +++ b/cpr/auth.cpp @@ -3,7 +3,7 @@ namespace cpr { const char* Authentication::GetAuthString() const noexcept { - return auth_string_.data(); + return auth_string_.c_str(); } } // namespace cpr diff --git a/cpr/session.cpp b/cpr/session.cpp index d32e96190..517c41569 100644 --- a/cpr/session.cpp +++ b/cpr/session.cpp @@ -53,6 +53,8 @@ class Session::Impl { Response Put(); private: + bool hasBodyOrPayload_{false}; + struct CurlHolderDeleter { void operator()(CurlHolder* holder) { freeHolder(holder); @@ -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); @@ -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; } @@ -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()); } } @@ -224,25 +228,26 @@ void Session::Impl::SetMultipart(Multipart&& multipart) { for (auto& part : multipart.parts) { std::vector 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(part.data)}); formdata.push_back( {CURLFORM_CONTENTSLENGTH, reinterpret_cast(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; @@ -257,24 +262,25 @@ void Session::Impl::SetMultipart(const Multipart& multipart) { for (auto& part : multipart.parts) { std::vector 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(part.data)}); formdata.push_back( {CURLFORM_BUFFERLENGTH, reinterpret_cast(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; @@ -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()); } } @@ -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); @@ -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); @@ -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); @@ -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, ""); } @@ -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); } @@ -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(response_code), std::move(response_string), std::move(header), diff --git a/test/delete_tests.cpp b/test/delete_tests.cpp index 59ecfe7ee..41c640b33 100644 --- a/test/delete_tests.cpp +++ b/test/delete_tests.cpp @@ -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"}; @@ -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); } diff --git a/test/httpServer.cpp b/test/httpServer.cpp index 94f03b7f8..c1e2b5d75 100644 --- a/test/httpServer.cpp +++ b/test/httpServer.cpp @@ -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; } @@ -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) { @@ -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) { @@ -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) { @@ -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"); } @@ -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()); } } @@ -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"); } @@ -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()); } } @@ -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(conn->mgr_data); - server->OnChunk(conn, static_cast(event_data)); + /** Do nothing. Just for housekeeping. **/ } break; case MG_EV_HTTP_REQUEST: { diff --git a/test/httpServer.hpp b/test/httpServer.hpp index 8e033217c..ba1877718 100644 --- a/test/httpServer.hpp +++ b/test/httpServer.hpp @@ -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 serverThread{nullptr}; diff --git a/test/patch_tests.cpp b/test/patch_tests.cpp index 045840d5c..3a5cd3b09 100644 --- a/test/patch_tests.cpp +++ b/test/patch_tests.cpp @@ -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(); @@ -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); } @@ -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); } @@ -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); } diff --git a/test/post_tests.cpp b/test/post_tests.cpp index 3e689ecb6..ff95948e4 100644 --- a/test/post_tests.cpp +++ b/test/post_tests.cpp @@ -148,7 +148,7 @@ TEST(UrlEncodedPostTests, FormPostFileTest) { content + "\n" "}"}; - std::remove(filename.data()); + std::remove(filename.c_str()); EXPECT_EQ(expected_text, response.text); EXPECT_EQ(url, response.url); EXPECT_EQ(std::string{"application/json"}, response.header["content-type"]); @@ -172,7 +172,7 @@ TEST(UrlEncodedPostTests, FormPostFileNoCopyTest) { content + "\n" "}"}; - std::remove(filename.data()); + std::remove(filename.c_str()); EXPECT_EQ(expected_text, response.text); EXPECT_EQ(url, response.url); EXPECT_EQ(std::string{"application/json"}, response.header["content-type"]); @@ -378,7 +378,7 @@ TEST(UrlEncodedPostTests, UrlReflectTest) { } TEST(UrlEncodedPostTests, PostWithNoBodyTest) { - auto url = Url{server->GetBaseUrl() + "/form_post_no_body.html"}; + auto url = Url{server->GetBaseUrl() + "/form_post.html"}; auto response = cpr::Post(url); auto expected_text = std::string{ "{\n"