From a860cbeabf7fd1c9795f1eeb44320afc56aa0b37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20B=C3=A1lint=20Misius?= Date: Fri, 16 Jun 2023 13:31:25 +0200 Subject: [PATCH] Use name-value pairs for HTTP post data and headers And fuse them only if needed (e.g. in Libcurl.cpp). Also finally stop specifying the filename for a form item with the : separator hack. --- src/client/http/PostData.h | 16 +++- src/client/http/Request.cpp | 10 +-- src/client/http/Request.h | 4 +- src/client/http/UploadSaveRequest.cpp | 2 +- src/client/http/requestmanager/Libcurl.cpp | 58 +++++++----- .../http/requestmanager/RequestManager.h | 7 +- src/lua/LuaHttp.cpp | 90 ++++++++++++++++--- 7 files changed, 140 insertions(+), 47 deletions(-) diff --git a/src/client/http/PostData.h b/src/client/http/PostData.h index 20bf75525..3ce84cf0b 100644 --- a/src/client/http/PostData.h +++ b/src/client/http/PostData.h @@ -1,11 +1,23 @@ #pragma once #include "common/String.h" -#include +#include #include +#include namespace http { + struct Header + { + ByteString name; + ByteString value; + }; + struct FormItem + { + ByteString name; + ByteString value; + std::optional filename; + }; using StringData = ByteString; - using FormData = std::map; + using FormData = std::vector; using PostData = std::variant; }; diff --git a/src/client/http/Request.cpp b/src/client/http/Request.cpp index 5e1c05b91..deb21b877 100644 --- a/src/client/http/Request.cpp +++ b/src/client/http/Request.cpp @@ -43,7 +43,7 @@ namespace http handle->verb = newVerb; } - void Request::AddHeader(ByteString header) + void Request::AddHeader(Header header) { assert(handle->state == RequestHandle::ready); handle->headers.push_back(header); @@ -64,12 +64,12 @@ namespace http { if (session.size()) { - AddHeader("X-Auth-User-Id: " + ID); - AddHeader("X-Auth-Session-Key: " + session); + AddHeader({ "X-Auth-User-Id", ID }); + AddHeader({ "X-Auth-Session-Key", session }); } else { - AddHeader("X-Auth-User: " + ID); + AddHeader({ "X-Auth-User", ID }); } } } @@ -95,7 +95,7 @@ namespace http return { handle->bytesTotal, handle->bytesDone }; } - const std::vector &Request::ResponseHeaders() const + const std::vector
&Request::ResponseHeaders() const { std::lock_guard lk(handle->stateMx); assert(handle->state == RequestHandle::done); diff --git a/src/client/http/Request.h b/src/client/http/Request.h index 4f75d5666..b9d580ff0 100644 --- a/src/client/http/Request.h +++ b/src/client/http/Request.h @@ -31,7 +31,7 @@ namespace http void FailEarly(ByteString error); void Verb(ByteString newVerb); - void AddHeader(ByteString header); + void AddHeader(Header header); void AddPostData(PostData data); void AuthHeaders(ByteString ID, ByteString session); @@ -40,7 +40,7 @@ namespace http bool CheckDone() const; std::pair CheckProgress() const; // total, done - const std::vector &ResponseHeaders() const; + const std::vector
&ResponseHeaders() const; void Wait(); int StatusCode() const; // status diff --git a/src/client/http/UploadSaveRequest.cpp b/src/client/http/UploadSaveRequest.cpp index ccb961066..c181f680a 100644 --- a/src/client/http/UploadSaveRequest.cpp +++ b/src/client/http/UploadSaveRequest.cpp @@ -29,7 +29,7 @@ namespace http AddPostData(FormData{ { "Name", saveInfo.GetName().ToUtf8() }, { "Description", saveInfo.GetDescription().ToUtf8() }, - { "Data:save.bin", ByteString(gameData.begin(), gameData.end()) }, + { "Data", ByteString(gameData.begin(), gameData.end()), "save.bin" }, { "Publish", saveInfo.GetPublished() ? "Public" : "Private" }, { "Key", user.SessionKey }, }); diff --git a/src/client/http/requestmanager/Libcurl.cpp b/src/client/http/requestmanager/Libcurl.cpp index 641bbb50f..274924c4f 100644 --- a/src/client/http/requestmanager/Libcurl.cpp +++ b/src/client/http/requestmanager/Libcurl.cpp @@ -3,6 +3,7 @@ #include "client/http/Request.h" #include "CurlError.h" #include "Config.h" +#include #if defined(CURL_AT_LEAST_VERSION) && CURL_AT_LEAST_VERSION(7, 55, 0) # define REQUEST_USE_CURL_OFFSET_T @@ -58,6 +59,7 @@ namespace http CURL *curlEasy = NULL; char curlErrorBuffer[CURL_ERROR_SIZE]; bool curlAddedToMulti = false; + bool gotStatusLine = false; RequestHandleHttp() : RequestHandle(CtorTag{}) { @@ -69,10 +71,28 @@ namespace http auto bytes = size * count; if (bytes >= 2 && ptr[bytes - 2] == '\r' && ptr[bytes - 1] == '\n') { - if (bytes > 2) // Don't include header list terminator (but include the status line). + if (bytes > 2 && handle->gotStatusLine) // Don't include header list terminator or the status line. { - handle->responseHeaders.push_back(ByteString(ptr, ptr + bytes - 2)); + auto line = ByteString(ptr, ptr + bytes - 2); + if (auto split = line.SplitBy(':')) + { + auto value = split.After(); + while (value.size() && (value.front() == ' ' || value.front() == '\t')) + { + value = value.Substr(1); + } + while (value.size() && (value.back() == ' ' || value.back() == '\t')) + { + value = value.Substr(0, value.size() - 1); + } + handle->responseHeaders.push_back({ split.Before().ToLower(), value }); + } + else + { + std::cerr << "skipping weird header: " << line << std::endl; + } } + handle->gotStatusLine = true; return bytes; } return 0; @@ -327,7 +347,7 @@ namespace http } for (auto &header : handle->headers) { - auto *newHeaders = curl_slist_append(handle->curlHeaders, header.c_str()); + auto *newHeaders = curl_slist_append(handle->curlHeaders, (header.name + ": " + header.value).c_str()); if (!newHeaders) { // Hopefully this is what a NULL from curl_slist_append means. @@ -355,36 +375,32 @@ namespace http // Hopefully this is what a NULL from curl_mime_addpart means. HandleCURLcode(CURLE_OUT_OF_MEMORY); } - HandleCURLcode(curl_mime_data(part, &field.second[0], field.second.size())); - if (auto split = field.first.SplitBy(':')) + HandleCURLcode(curl_mime_data(part, &field.value[0], field.value.size())); + HandleCURLcode(curl_mime_name(part, field.name.c_str())); + if (field.filename.has_value()) { - HandleCURLcode(curl_mime_name(part, split.Before().c_str())); - HandleCURLcode(curl_mime_filename(part, split.After().c_str())); - } - else - { - HandleCURLcode(curl_mime_name(part, field.first.c_str())); + HandleCURLcode(curl_mime_filename(part, field.filename->c_str())); } } HandleCURLcode(curl_easy_setopt(handle->curlEasy, CURLOPT_MIMEPOST, handle->curlPostFields)); #else for (auto &field : formData) { - if (auto split = field.first.SplitBy(':')) + if (field.filename.has_value()) { HandleCURLFORMcode(curl_formadd(&handle->curlPostFieldsFirst, &handle->curlPostFieldsLast, - CURLFORM_COPYNAME, split.Before().c_str(), - CURLFORM_BUFFER, split.After().c_str(), - CURLFORM_BUFFERPTR, &field.second[0], - CURLFORM_BUFFERLENGTH, field.second.size(), + CURLFORM_COPYNAME, field.name.c_str(), + CURLFORM_BUFFER, field.filename->c_str(), + CURLFORM_BUFFERPTR, &field.value[0], + CURLFORM_BUFFERLENGTH, field.value.size(), CURLFORM_END)); } else { HandleCURLFORMcode(curl_formadd(&handle->curlPostFieldsFirst, &handle->curlPostFieldsLast, - CURLFORM_COPYNAME, field.first.c_str(), - CURLFORM_PTRCONTENTS, &field.second[0], - CURLFORM_CONTENTLEN, field.second.size(), + CURLFORM_COPYNAME, field.name.c_str(), + CURLFORM_PTRCONTENTS, &field.value[0], + CURLFORM_CONTENTLEN, field.value.size(), CURLFORM_END)); } } @@ -406,9 +422,9 @@ namespace http { HandleCURLcode(curl_easy_setopt(handle->curlEasy, CURLOPT_HTTPGET, 1L)); } - if (handle->verb.size()) + if (handle->verb) { - HandleCURLcode(curl_easy_setopt(handle->curlEasy, CURLOPT_CUSTOMREQUEST, handle->verb.c_str())); + HandleCURLcode(curl_easy_setopt(handle->curlEasy, CURLOPT_CUSTOMREQUEST, handle->verb->c_str())); } HandleCURLcode(curl_easy_setopt(handle->curlEasy, CURLOPT_FOLLOWLOCATION, 1L)); if constexpr (ENFORCE_HTTPS) diff --git a/src/client/http/requestmanager/RequestManager.h b/src/client/http/requestmanager/RequestManager.h index 4f56d5b79..addd05805 100644 --- a/src/client/http/requestmanager/RequestManager.h +++ b/src/client/http/requestmanager/RequestManager.h @@ -10,6 +10,7 @@ #include #include #include +#include namespace http { @@ -24,10 +25,10 @@ namespace http public: ByteString uri; - ByteString verb; + std::optional verb; bool isPost = false; PostData postData; - std::vector headers; + std::vector
headers; enum State { @@ -43,7 +44,7 @@ namespace http std::atomic bytesDone = 0; int statusCode = 0; ByteString responseData; - std::vector responseHeaders; + std::vector
responseHeaders; std::optional error; std::optional failEarly; diff --git a/src/lua/LuaHttp.cpp b/src/lua/LuaHttp.cpp index 194ad05c6..065afd806 100644 --- a/src/lua/LuaHttp.cpp +++ b/src/lua/LuaHttp.cpp @@ -46,7 +46,7 @@ private: } public: - static int Make(lua_State *l, const ByteString &uri, bool isPost, const ByteString &verb, RequestType type, const http::PostData &postData, const std::vector &headers) + static int Make(lua_State *l, const ByteString &uri, bool isPost, const ByteString &verb, RequestType type, const http::PostData &postData, const std::vector &headers) { auto authUser = Client::Ref().GetAuthUser(); if (type == getAuthToken && !authUser.UserID) @@ -67,7 +67,7 @@ public: { rh->request->Verb(verb); } - for (auto &header : headers) + for (const auto &header : headers) { rh->request->AddHeader(header); } @@ -120,7 +120,7 @@ public: } } - std::pair Finish(std::vector &headers) + std::pair Finish(std::vector &headers) { int status = 0; ByteString data; @@ -212,14 +212,18 @@ static int http_request_finish(lua_State *l) auto *rh = (RequestHandle *)luaL_checkudata(l, 1, "HTTPRequest"); if (!rh->Dead()) { - std::vector headers; + std::vector headers; auto [ status, data ] = rh->Finish(headers); tpt_lua_pushByteString(l, data); lua_pushinteger(l, status); lua_newtable(l); for (auto i = 0; i < int(headers.size()); ++i) { - lua_pushlstring(l, headers[i].data(), headers[i].size()); + lua_newtable(l); + lua_pushlstring(l, headers[i].name.data(), headers[i].name.size()); + lua_rawseti(l, -2, 1); + lua_pushlstring(l, headers[i].value.data(), headers[i].value.size()); + lua_rawseti(l, -2, 1); lua_rawseti(l, -2, i + 1); } return 3; @@ -246,17 +250,59 @@ static int http_request(lua_State *l, bool isPost) { postData = http::FormData{}; auto &formData = std::get(postData); - lua_pushnil(l); - while (lua_next(l, 2)) + auto size = lua_objlen(l, headersIndex); + if (size) { - lua_pushvalue(l, -2); - formData.emplace(tpt_lua_toByteString(l, -1), tpt_lua_toByteString(l, -2)); - lua_pop(l, 2); + for (auto i = 0U; i < size; ++i) + { + lua_rawgeti(l, headersIndex, i + 1); + if (!lua_istable(l, -1)) + { + luaL_error(l, "form item %i is not a table", i + 1); + } + lua_rawgeti(l, -1, 1); + if (!lua_isstring(l, -1)) + { + luaL_error(l, "name of form item %i is not a string", i + 1); + } + auto name = tpt_lua_toByteString(l, -1); + lua_pop(l, 1); + lua_rawgeti(l, -1, 2); + if (!lua_isstring(l, -1)) + { + luaL_error(l, "value of form item %i is not a string", i + 1); + } + auto value = tpt_lua_toByteString(l, -1); + lua_pop(l, 1); + std::optional filename; + lua_rawgeti(l, -1, 3); + if (!lua_isnoneornil(l, -1)) + { + if (!lua_isstring(l, -1)) + { + luaL_error(l, "filename of form item %i is not a string", i + 1); + } + filename = tpt_lua_toByteString(l, -1); + } + lua_pop(l, 1); + formData.push_back({ name, value, filename }); + lua_pop(l, 1); + } + } + else + { + lua_pushnil(l); + while (lua_next(l, 2)) + { + lua_pushvalue(l, -2); + formData.push_back({ tpt_lua_toByteString(l, -1), tpt_lua_toByteString(l, -2) }); + lua_pop(l, 2); + } } } } - std::vector headers; + std::vector headers; if (lua_istable(l, headersIndex)) { auto size = lua_objlen(l, headersIndex); @@ -265,7 +311,25 @@ static int http_request(lua_State *l, bool isPost) for (auto i = 0U; i < size; ++i) { lua_rawgeti(l, headersIndex, i + 1); - headers.push_back(tpt_lua_toByteString(l, -1)); + if (!lua_istable(l, -1)) + { + luaL_error(l, "header %i is not a table", i + 1); + } + lua_rawgeti(l, -1, 1); + if (!lua_isstring(l, -1)) + { + luaL_error(l, "name of header %i is not a string", i + 1); + } + auto name = tpt_lua_toByteString(l, -1); + lua_pop(l, 1); + lua_rawgeti(l, -1, 2); + if (!lua_isstring(l, -1)) + { + luaL_error(l, "value of header %i is not a string", i + 1); + } + auto value = tpt_lua_toByteString(l, -1); + lua_pop(l, 1); + headers.push_back({ name, value }); lua_pop(l, 1); } } @@ -276,7 +340,7 @@ static int http_request(lua_State *l, bool isPost) while (lua_next(l, headersIndex)) { lua_pushvalue(l, -2); - headers.push_back(tpt_lua_toByteString(l, -1) + ByteString(": ") + tpt_lua_toByteString(l, -2)); + headers.push_back({ tpt_lua_toByteString(l, -1), tpt_lua_toByteString(l, -2) }); lua_pop(l, 2); } }