From 5c0553031e4bf611b12f9d6a707ba418c0ebb88d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 23 Jan 2025 12:06:31 +0100 Subject: [PATCH 1/2] Bugfix HTTP, oops --- Common/Net/HTTPClient.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Common/Net/HTTPClient.h b/Common/Net/HTTPClient.h index 072e8ed713ed..ec919675869a 100644 --- a/Common/Net/HTTPClient.h +++ b/Common/Net/HTTPClient.h @@ -112,7 +112,6 @@ class HTTPRequest : public Request { void SetFailed(int code); std::string postData_; - Buffer buffer_; std::thread thread_; std::string postMime_; bool completed_ = false; From eb719c43e8debd4aa86c15ef86bfc5da714f81cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 23 Jan 2025 12:09:33 +0100 Subject: [PATCH 2/2] HTTP: Replace ProgressBarMode with a new RequestFlags enum --- Common/Buffer.cpp | 8 +------- Common/Buffer.h | 4 ++-- Common/Net/HTTPClient.cpp | 13 +++++++------ Common/Net/HTTPClient.h | 12 ++++++------ Common/Net/HTTPNaettRequest.cpp | 2 +- Common/Net/HTTPNaettRequest.h | 2 +- Common/Net/HTTPRequest.cpp | 26 +++++++++++++------------- Common/Net/HTTPRequest.h | 22 ++++++++++++---------- Core/Config.cpp | 2 +- Core/Reporting.cpp | 2 +- Core/RetroAchievements.cpp | 6 +++--- Core/Util/GameManager.cpp | 2 +- UI/DevScreens.cpp | 2 +- UI/Store.cpp | 6 +++--- 14 files changed, 53 insertions(+), 56 deletions(-) diff --git a/Common/Buffer.cpp b/Common/Buffer.cpp index e66702057923..54ca26f2264d 100644 --- a/Common/Buffer.cpp +++ b/Common/Buffer.cpp @@ -15,19 +15,13 @@ char *Buffer::Append(size_t length) { } } -void Buffer::Append(const std::string &str) { +void Buffer::Append(std::string_view str) { char *ptr = Append(str.size()); if (ptr) { memcpy(ptr, str.data(), str.size()); } } -void Buffer::Append(const char *str) { - size_t len = strlen(str); - char *dest = Append(len); - memcpy(dest, str, len); -} - void Buffer::Append(const Buffer &other) { size_t len = other.size(); if (len > 0) { diff --git a/Common/Buffer.h b/Common/Buffer.h index 36f27324f5f3..79875bfd92fa 100644 --- a/Common/Buffer.h +++ b/Common/Buffer.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include "Common/Common.h" @@ -26,8 +27,7 @@ class Buffer { char *Append(size_t length); // These work pretty much like you'd expect. - void Append(const char *str); // str null-terminated. The null is not copied. - void Append(const std::string &str); + void Append(std::string_view str); void Append(const Buffer &other); // Various types. Useful for varz etc. Appends a string representation of the diff --git a/Common/Net/HTTPClient.cpp b/Common/Net/HTTPClient.cpp index 3ca4094b7dec..b4cd84563626 100644 --- a/Common/Net/HTTPClient.cpp +++ b/Common/Net/HTTPClient.cpp @@ -299,13 +299,14 @@ int Client::GET(const RequestParams &req, Buffer *output, net::RequestProgress * return code; } -int Client::POST(const RequestParams &req, const std::string &data, const std::string &mime, Buffer *output, net::RequestProgress *progress) { +int Client::POST(const RequestParams &req, std::string_view data, std::string_view mime, Buffer *output, net::RequestProgress *progress) { char otherHeaders[2048]; if (mime.empty()) { snprintf(otherHeaders, sizeof(otherHeaders), "Content-Length: %lld\r\n", (long long)data.size()); } else { - snprintf(otherHeaders, sizeof(otherHeaders), "Content-Length: %lld\r\nContent-Type: %s\r\n", (long long)data.size(), mime.c_str()); + snprintf(otherHeaders, sizeof(otherHeaders), "Content-Length: %lld\r\nContent-Type: %.*s\r\n", (long long)data.size(), (int)mime.size(), mime.data()); } + int err = SendRequestWithData("POST", req, data, otherHeaders, progress); if (err < 0) { return err; @@ -325,7 +326,7 @@ int Client::POST(const RequestParams &req, const std::string &data, const std::s return code; } -int Client::POST(const RequestParams &req, const std::string &data, Buffer *output, net::RequestProgress *progress) { +int Client::POST(const RequestParams &req, std::string_view data, Buffer *output, net::RequestProgress *progress) { return POST(req, data, "", output, progress); } @@ -333,7 +334,7 @@ int Client::SendRequest(const char *method, const RequestParams &req, const char return SendRequestWithData(method, req, "", otherHeaders, progress); } -int Client::SendRequestWithData(const char *method, const RequestParams &req, const std::string &data, const char *otherHeaders, net::RequestProgress *progress) { +int Client::SendRequestWithData(const char *method, const RequestParams &req, std::string_view data, const char *otherHeaders, net::RequestProgress *progress) { progress->Update(0, 0, false); net::Buffer buffer; @@ -485,7 +486,7 @@ int Client::ReadResponseEntity(net::Buffer *readbuf, const std::vector &responseHeaders, net::RequestProgress *progress); // Return value is the HTTP return code. - int POST(const RequestParams &req, const std::string &data, const std::string &mime, Buffer *output, net::RequestProgress *progress); - int POST(const RequestParams &req, const std::string &data, Buffer *output, net::RequestProgress *progress); + int POST(const RequestParams &req, std::string_view data, std::string_view mime, Buffer *output, net::RequestProgress *progress); + int POST(const RequestParams &req, std::string_view data, Buffer *output, net::RequestProgress *progress); // HEAD, PUT, DELETE aren't implemented yet, but can be done with SendRequest. int SendRequest(const char *method, const RequestParams &req, const char *otherHeaders, net::RequestProgress *progress); - int SendRequestWithData(const char *method, const RequestParams &req, const std::string &data, const char *otherHeaders, net::RequestProgress *progress); + int SendRequestWithData(const char *method, const RequestParams &req, std::string_view data, const char *otherHeaders, net::RequestProgress *progress); int ReadResponseHeaders(net::Buffer *readbuf, std::vector &responseHeaders, net::RequestProgress *progress, std::string *statusLine = nullptr); // If your response contains a response, you must read it. int ReadResponseEntity(net::Buffer *readbuf, const std::vector &responseHeaders, Buffer *output, net::RequestProgress *progress); @@ -79,7 +79,7 @@ class Client : public net::Connection { dataTimeout_ = t; } - void SetUserAgent(const std::string &value) { + void SetUserAgent(std::string_view value) { userAgent_ = value; } @@ -96,7 +96,7 @@ class Client : public net::Connection { // Really an asynchronous request. class HTTPRequest : public Request { public: - HTTPRequest(RequestMethod method, std::string_view url, std::string_view postData, std::string_view postMime, const Path &outfile, ProgressBarMode progressBarMode = ProgressBarMode::DELAYED, std::string_view name = ""); + HTTPRequest(RequestMethod method, std::string_view url, std::string_view postData, std::string_view postMime, const Path &outfile, RequestFlags progressBarMode = RequestFlags::ProgressBar | RequestFlags::ProgressBarDelayed, std::string_view name = ""); ~HTTPRequest(); void Start() override; @@ -119,4 +119,4 @@ class HTTPRequest : public Request { bool joined_ = false; }; -} // http +} // namespace http diff --git a/Common/Net/HTTPNaettRequest.cpp b/Common/Net/HTTPNaettRequest.cpp index be87fecd983d..4bfa76413c65 100644 --- a/Common/Net/HTTPNaettRequest.cpp +++ b/Common/Net/HTTPNaettRequest.cpp @@ -12,7 +12,7 @@ namespace http { -HTTPSRequest::HTTPSRequest(RequestMethod method, std::string_view url, std::string_view postData, std::string_view postMime, const Path &outfile, ProgressBarMode progressBarMode, std::string_view name) +HTTPSRequest::HTTPSRequest(RequestMethod method, std::string_view url, std::string_view postData, std::string_view postMime, const Path &outfile, RequestFlags progressBarMode, std::string_view name) : Request(method, url, name, &cancelled_, progressBarMode), method_(method), postData_(postData), postMime_(postMime) { outfile_ = outfile; } diff --git a/Common/Net/HTTPNaettRequest.h b/Common/Net/HTTPNaettRequest.h index 5cbbe8f3b0ce..0694e9e32253 100644 --- a/Common/Net/HTTPNaettRequest.h +++ b/Common/Net/HTTPNaettRequest.h @@ -14,7 +14,7 @@ namespace http { // Really an asynchronous request. class HTTPSRequest : public Request { public: - HTTPSRequest(RequestMethod method, std::string_view url, std::string_view postData, std::string_view postMime, const Path &outfile, ProgressBarMode progressBarMode = ProgressBarMode::DELAYED, std::string_view name = ""); + HTTPSRequest(RequestMethod method, std::string_view url, std::string_view postData, std::string_view postMime, const Path &outfile, RequestFlags progressBarMode = RequestFlags::ProgressBar | RequestFlags::ProgressBarDelayed, std::string_view name = ""); ~HTTPSRequest(); void Start() override; diff --git a/Common/Net/HTTPRequest.cpp b/Common/Net/HTTPRequest.cpp index 7b0fdffbf257..60623f2e1d13 100644 --- a/Common/Net/HTTPRequest.cpp +++ b/Common/Net/HTTPRequest.cpp @@ -9,8 +9,8 @@ namespace http { -Request::Request(RequestMethod method, std::string_view url, std::string_view name, bool *cancelled, ProgressBarMode mode) - : method_(method), url_(url), name_(name), progress_(cancelled), progressBarMode_(mode) { +Request::Request(RequestMethod method, std::string_view url, std::string_view name, bool *cancelled, RequestFlags mode) + : method_(method), url_(url), name_(name), progress_(cancelled), flags_(mode) { INFO_LOG(Log::HTTP, "HTTP %s request: %.*s (%.*s)", RequestMethodToString(method), (int)url.size(), url.data(), (int)name.size(), name.data()); progress_.callback = [=](int64_t bytes, int64_t contentLength, bool done) { @@ -25,9 +25,9 @@ Request::Request(RequestMethod method, std::string_view url, std::string_view na message = url_; } } - if (progressBarMode_ != ProgressBarMode::NONE) { + if (flags_ & RequestFlags::ProgressBar) { if (!done) { - g_OSD.SetProgressBar(url_, std::move(message), 0.0f, (float)contentLength, (float)bytes, progressBarMode_ == ProgressBarMode::DELAYED ? 3.0f : 0.0f); // delay 3 seconds before showing. + g_OSD.SetProgressBar(url_, std::move(message), 0.0f, (float)contentLength, (float)bytes, flags_ & RequestFlags::ProgressBarDelayed ? 3.0f : 0.0f); // delay 3 seconds before showing. } else { g_OSD.RemoveProgressBar(url_, Failed() ? false : true, 0.5f); } @@ -39,20 +39,20 @@ static bool IsHttpsUrl(std::string_view url) { return startsWith(url, "https:"); } -std::shared_ptr CreateRequest(RequestMethod method, std::string_view url, std::string_view postdata, std::string_view postMime, const Path &outfile, ProgressBarMode mode, std::string_view name) { +std::shared_ptr CreateRequest(RequestMethod method, std::string_view url, std::string_view postdata, std::string_view postMime, const Path &outfile, RequestFlags flags, std::string_view name) { if (IsHttpsUrl(url) && System_GetPropertyBool(SYSPROP_SUPPORTS_HTTPS)) { #ifndef HTTPS_NOT_AVAILABLE - return std::shared_ptr(new HTTPSRequest(method, url, postdata, postMime, outfile, mode, name)); + return std::shared_ptr(new HTTPSRequest(method, url, postdata, postMime, outfile, flags, name)); #else return std::shared_ptr(); #endif } else { - return std::shared_ptr(new HTTPRequest(method, url, postdata, postMime, outfile, mode, name)); + return std::shared_ptr(new HTTPRequest(method, url, postdata, postMime, outfile, flags, name)); } } -std::shared_ptr RequestManager::StartDownload(std::string_view url, const Path &outfile, ProgressBarMode mode, const char *acceptMime) { - std::shared_ptr dl = CreateRequest(RequestMethod::GET, url, "", "", outfile, mode, ""); +std::shared_ptr RequestManager::StartDownload(std::string_view url, const Path &outfile, RequestFlags flags, const char *acceptMime) { + std::shared_ptr dl = CreateRequest(RequestMethod::GET, url, "", "", outfile, flags, ""); if (!userAgent_.empty()) dl->SetUserAgent(userAgent_); @@ -66,11 +66,11 @@ std::shared_ptr RequestManager::StartDownload(std::string_view url, con std::shared_ptr RequestManager::StartDownloadWithCallback( std::string_view url, const Path &outfile, - ProgressBarMode mode, + RequestFlags flags, std::function callback, std::string_view name, const char *acceptMime) { - std::shared_ptr dl = CreateRequest(RequestMethod::GET, url, "", "", outfile, mode, name); + std::shared_ptr dl = CreateRequest(RequestMethod::GET, url, "", "", outfile, flags, name); if (!userAgent_.empty()) dl->SetUserAgent(userAgent_); @@ -86,10 +86,10 @@ std::shared_ptr RequestManager::AsyncPostWithCallback( std::string_view url, std::string_view postData, std::string_view postMime, - ProgressBarMode mode, + RequestFlags flags, std::function callback, std::string_view name) { - std::shared_ptr dl = CreateRequest(RequestMethod::POST, url, postData, postMime, Path(), mode, name); + std::shared_ptr dl = CreateRequest(RequestMethod::POST, url, postData, postMime, Path(), flags, name); if (!userAgent_.empty()) dl->SetUserAgent(userAgent_); dl->SetCallback(callback); diff --git a/Common/Net/HTTPRequest.h b/Common/Net/HTTPRequest.h index b63b4746e41f..35706b66c8bc 100644 --- a/Common/Net/HTTPRequest.h +++ b/Common/Net/HTTPRequest.h @@ -14,23 +14,25 @@ enum class RequestMethod { POST, }; -enum class ProgressBarMode { - NONE, - VISIBLE, - DELAYED, +enum class RequestFlags { + Default = 0, + ProgressBar = 1, + ProgressBarDelayed = 2, + // Cached = 4 etc }; +ENUM_CLASS_BITOPS(RequestFlags); // Abstract request. class Request { public: - Request(RequestMethod method, std::string_view url, std::string_view name, bool *cancelled, ProgressBarMode mode); + Request(RequestMethod method, std::string_view url, std::string_view name, bool *cancelled, RequestFlags mode); virtual ~Request() {} void SetAccept(const char *mime) { acceptMime_ = mime; } - void SetUserAgent(const std::string &userAgent) { + void SetUserAgent(std::string_view userAgent) { userAgent_ = userAgent; } @@ -81,7 +83,7 @@ class Request { std::vector responseHeaders_; net::RequestProgress progress_; - ProgressBarMode progressBarMode_; + RequestFlags flags_; private: std::function callback_; @@ -95,12 +97,12 @@ class RequestManager { CancelAll(); } - std::shared_ptr StartDownload(std::string_view url, const Path &outfile, ProgressBarMode mode, const char *acceptMime = nullptr); + std::shared_ptr StartDownload(std::string_view url, const Path &outfile, RequestFlags flags, const char *acceptMime = nullptr); std::shared_ptr StartDownloadWithCallback( std::string_view url, const Path &outfile, - ProgressBarMode mode, + RequestFlags flags, std::function callback, std::string_view name = "", const char *acceptMime = nullptr); @@ -109,7 +111,7 @@ class RequestManager { std::string_view url, std::string_view postData, std::string_view postMime, // Use postMime = "application/x-www-form-urlencoded" for standard form-style posts, such as used by retroachievements. For encoding form data manually we have MultipartFormDataEncoder. - ProgressBarMode mode, + RequestFlags flags, std::function callback, std::string_view name = ""); diff --git a/Core/Config.cpp b/Core/Config.cpp index d6c338dbb04e..d6d15e22f34f 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -1309,7 +1309,7 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename) { if (iRunCount % 10 == 0 && bCheckForNewVersion) { const char *versionUrl = "http://www.ppsspp.org/version.json"; const char *acceptMime = "application/json, text/*; q=0.9, */*; q=0.8"; - g_DownloadManager.StartDownloadWithCallback(versionUrl, Path(), http::ProgressBarMode::NONE, &DownloadCompletedCallback, "version", acceptMime); + g_DownloadManager.StartDownloadWithCallback(versionUrl, Path(), http::RequestFlags::Default, &DownloadCompletedCallback, "version", acceptMime); } INFO_LOG(Log::Loader, "Loading controller config: %s", controllerIniFilename_.c_str()); diff --git a/Core/Reporting.cpp b/Core/Reporting.cpp index d6041885f0d9..8727fa8bb3c1 100644 --- a/Core/Reporting.cpp +++ b/Core/Reporting.cpp @@ -294,7 +294,7 @@ namespace Reporting std::string hostname = ServerHostname(); int port = ServerPort(); snprintf(url, sizeof(url), "http://%s:%d%s", hostname.c_str(), port, uri); - g_DownloadManager.AsyncPostWithCallback(url, data, mimeType, http::ProgressBarMode::NONE, callback); + g_DownloadManager.AsyncPostWithCallback(url, data, mimeType, http::RequestFlags::Default, callback); } std::string StripTrailingNull(const std::string &str) { diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index ffc65f2316c4..6aaaad0a64e1 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -289,7 +289,7 @@ static void server_call_callback(const rc_api_request_t *request, // If post data is provided, we need to make a POST request, otherwise, a GET request will suffice. auto ac = GetI18NCategory(I18NCat::ACHIEVEMENTS); if (request->post_data) { - std::shared_ptr download = g_DownloadManager.AsyncPostWithCallback(std::string(request->url), std::string(request->post_data), "application/x-www-form-urlencoded", http::ProgressBarMode::DELAYED, [=](http::Request &download) { + std::shared_ptr download = g_DownloadManager.AsyncPostWithCallback(std::string(request->url), std::string(request->post_data), "application/x-www-form-urlencoded", http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, [=](http::Request &download) { std::string buffer; download.buffer().TakeAll(&buffer); rc_api_server_response_t response{}; @@ -299,7 +299,7 @@ static void server_call_callback(const rc_api_request_t *request, callback(&response, callback_data); }, ac->T("Contacting RetroAchievements server...")); } else { - std::shared_ptr download = g_DownloadManager.StartDownloadWithCallback(std::string(request->url), Path(), http::ProgressBarMode::DELAYED, [=](http::Request &download) { + std::shared_ptr download = g_DownloadManager.StartDownloadWithCallback(std::string(request->url), Path(), http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, [=](http::Request &download) { std::string buffer; download.buffer().TakeAll(&buffer); rc_api_server_response_t response{}; @@ -874,7 +874,7 @@ bool HasAchievementsOrLeaderboards() { void DownloadImageIfMissing(const std::string &cache_key, std::string &&url) { if (g_iconCache.MarkPending(cache_key)) { INFO_LOG(Log::Achievements, "Downloading image: %s (%s)", url.c_str(), cache_key.c_str()); - g_DownloadManager.StartDownloadWithCallback(url, Path(), http::ProgressBarMode::NONE, [cache_key](http::Request &download) { + g_DownloadManager.StartDownloadWithCallback(url, Path(), http::RequestFlags::Default, [cache_key](http::Request &download) { if (download.ResultCode() != 200) return; std::string data; diff --git a/Core/Util/GameManager.cpp b/Core/Util/GameManager.cpp index a5109bd113ea..2b74fe82a42a 100644 --- a/Core/Util/GameManager.cpp +++ b/Core/Util/GameManager.cpp @@ -107,7 +107,7 @@ bool GameManager::DownloadAndInstall(const std::string &storeFileUrl) { Path filename = GetTempFilename(); const char *acceptMime = "application/zip, application/x-cso, application/x-iso9660-image, application/octet-stream; q=0.9, */*; q=0.8"; - curDownload_ = g_DownloadManager.StartDownload(storeFileUrl, filename, http::ProgressBarMode::VISIBLE, acceptMime); + curDownload_ = g_DownloadManager.StartDownload(storeFileUrl, filename, http::RequestFlags::ProgressBar, acceptMime); return true; } diff --git a/UI/DevScreens.cpp b/UI/DevScreens.cpp index 26ebf2ec11a1..536d97f3a6df 100644 --- a/UI/DevScreens.cpp +++ b/UI/DevScreens.cpp @@ -1079,7 +1079,7 @@ void FrameDumpTestScreen::update() { if (!listing_) { const char *acceptMime = "text/html, */*; q=0.8"; - listing_ = g_DownloadManager.StartDownload(framedumpsBaseUrl, Path(), http::ProgressBarMode::DELAYED, acceptMime); + listing_ = g_DownloadManager.StartDownload(framedumpsBaseUrl, Path(), http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, acceptMime); } if (listing_ && listing_->Done() && files_.empty()) { diff --git a/UI/Store.cpp b/UI/Store.cpp index c6969c5753fe..9ab15a087bf1 100644 --- a/UI/Store.cpp +++ b/UI/Store.cpp @@ -63,7 +63,7 @@ class HttpImageFileView : public UI::View { if (useIconCache && g_iconCache.MarkPending(path_)) { const char *acceptMime = "image/png, image/jpeg, image/*; q=0.9, */*; q=0.8"; - requestManager_->StartDownloadWithCallback(path_, Path(), http::ProgressBarMode::DELAYED, [](http::Request &download) { + requestManager_->StartDownloadWithCallback(path_, Path(), http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, [](http::Request &download) { // Can't touch 'this' in this function! Don't use captures! std::string path = download.url(); if (download.ResultCode() == 200) { @@ -179,7 +179,7 @@ void HttpImageFileView::Draw(UIContext &dc) { if (!texture_ && !textureFailed_ && !path_.empty() && !download_) { auto cb = std::bind(&HttpImageFileView::DownloadCompletedCallback, this, std::placeholders::_1); const char *acceptMime = "image/png, image/jpeg, image/*; q=0.9, */*; q=0.8"; - requestManager_->StartDownloadWithCallback(path_, Path(), http::ProgressBarMode::NONE, cb, acceptMime); + requestManager_->StartDownloadWithCallback(path_, Path(), http::RequestFlags::Default, cb, acceptMime); } if (!textureData_.empty()) { @@ -433,7 +433,7 @@ StoreScreen::StoreScreen() { std::string indexPath = StoreBaseUrl() + "index.json"; const char *acceptMime = "application/json, */*; q=0.8"; - listing_ = g_DownloadManager.StartDownload(indexPath, Path(), http::ProgressBarMode::DELAYED, acceptMime); + listing_ = g_DownloadManager.StartDownload(indexPath, Path(), http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, acceptMime); } StoreScreen::~StoreScreen() {