From 2d17097c915125e09dba1347005d962ab6cdf377 Mon Sep 17 00:00:00 2001 From: heinrich5991 Date: Thu, 14 Mar 2024 11:19:32 +0100 Subject: [PATCH 1/3] Use `log_*` instead of `Console()->Log()` in HTTP serverbrowser --- src/engine/client/serverbrowser.cpp | 2 +- src/engine/client/serverbrowser_http.cpp | 29 ++++++++++++------------ src/engine/client/serverbrowser_http.h | 3 +-- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/engine/client/serverbrowser.cpp b/src/engine/client/serverbrowser.cpp index 2d4fc3d84..031d8c812 100644 --- a/src/engine/client/serverbrowser.cpp +++ b/src/engine/client/serverbrowser.cpp @@ -102,7 +102,7 @@ void CServerBrowser::SetBaseInfo(class CNetClient *pClient, const char *pNetVers void CServerBrowser::OnInit() { - m_pHttp = CreateServerBrowserHttp(m_pEngine, m_pConsole, m_pStorage, m_pHttpClient, g_Config.m_BrCachedBestServerinfoUrl); + m_pHttp = CreateServerBrowserHttp(m_pEngine, m_pStorage, m_pHttpClient, g_Config.m_BrCachedBestServerinfoUrl); } void CServerBrowser::RegisterCommands() diff --git a/src/engine/client/serverbrowser_http.cpp b/src/engine/client/serverbrowser_http.cpp index c33fbc560..71b7ba54c 100644 --- a/src/engine/client/serverbrowser_http.cpp +++ b/src/engine/client/serverbrowser_http.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -200,7 +201,7 @@ void CChooseMaster::CJob::Run() pHead->Wait(); if(pHead->State() == EHttpState::ABORTED || State() == IJob::STATE_ABORTED) { - dbg_msg("serverbrowse_http", "master chooser aborted"); + log_debug("serverbrowse_http", "master chooser aborted"); return; } if(pHead->State() != EHttpState::DONE) @@ -223,7 +224,7 @@ void CChooseMaster::CJob::Run() auto Time = std::chrono::duration_cast(time_get_nanoseconds() - StartTime); if(pGet->State() == EHttpState::ABORTED || State() == IJob::STATE_ABORTED) { - dbg_msg("serverbrowse_http", "master chooser aborted"); + log_debug("serverbrowse_http", "master chooser aborted"); return; } if(pGet->State() != EHttpState::DONE) @@ -242,7 +243,7 @@ void CChooseMaster::CJob::Run() { continue; } - dbg_msg("serverbrowse_http", "found master, url='%s' time=%dms", pUrl, (int)Time.count()); + log_info("serverbrowse_http", "found master, url='%s' time=%dms", pUrl, (int)Time.count()); aTimeMs[i] = Time.count(); } @@ -263,18 +264,18 @@ void CChooseMaster::CJob::Run() } if(BestIndex == -1) { - dbg_msg("serverbrowse_http", "WARNING: no usable masters found"); + log_error("serverbrowse_http", "WARNING: no usable masters found"); return; } - dbg_msg("serverbrowse_http", "determined best master, url='%s' time=%dms", m_pData->m_aaUrls[BestIndex], BestTime); + log_info("serverbrowse_http", "determined best master, url='%s' time=%dms", m_pData->m_aaUrls[BestIndex], BestTime); m_pData->m_BestIndex.store(BestIndex); } class CServerBrowserHttp : public IServerBrowserHttp { public: - CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex); + CServerBrowserHttp(IEngine *pEngine, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex); ~CServerBrowserHttp() override; void Update() override; bool IsRefreshing() override { return m_State != STATE_DONE; } @@ -310,7 +311,6 @@ private: static bool Validate(json_value *pJson); static bool Parse(json_value *pJson, std::vector *pvServers, std::vector *pvLegacyServers); - IConsole *m_pConsole; IHttp *m_pHttp; int m_State = STATE_DONE; @@ -321,8 +321,7 @@ private: std::vector m_vLegacyServers; }; -CServerBrowserHttp::CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex) : - m_pConsole(pConsole), +CServerBrowserHttp::CServerBrowserHttp(IEngine *pEngine, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex) : m_pHttp(pHttp), m_pChooseMaster(new CChooseMaster(pEngine, pHttp, Validate, ppUrls, NumUrls, PreviousBestIndex)) { @@ -346,7 +345,7 @@ void CServerBrowserHttp::Update() { if(!m_pChooseMaster->IsRefreshing()) { - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "serverbrowse_http", "no working serverlist URL found"); + log_error("serverbrowse_http", "no working serverlist URL found"); m_State = STATE_NO_MASTER; } return; @@ -374,7 +373,7 @@ void CServerBrowserHttp::Update() json_value_free(pJson); if(!Success) { - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "serverbrowse_http", "failed getting serverlist, trying to find best URL"); + log_error("serverbrowse_http", "failed getting serverlist, trying to find best URL"); m_pChooseMaster->Reset(); m_pChooseMaster->Refresh(); } @@ -435,7 +434,7 @@ bool CServerBrowserHttp::Parse(json_value *pJson, std::vector *pvSe } if(CServerInfo2::FromJson(&ParsedInfo, &Info)) { - //dbg_msg("dbg/serverbrowser", "skipped due to info, i=%d", i); + log_debug("serverbrowser_http", "skipped due to info, i=%d", i); // Only skip the current server on parsing // failure; the server info is "user input" by // the game server and can be set to arbitrary @@ -455,7 +454,7 @@ bool CServerBrowserHttp::Parse(json_value *pJson, std::vector *pvSe NETADDR ParsedAddr; if(ServerbrowserParseUrl(&ParsedAddr, Addresses[a])) { - //dbg_msg("dbg/serverbrowser", "unknown address, i=%d a=%d", i, a); + log_debug("dbg/serverbrowser", "unknown address, i=%d a=%d", i, a); // Skip unknown addresses. continue; } @@ -495,7 +494,7 @@ static const char *DEFAULT_SERVERLIST_URLS[] = { "https://master4.ddnet.org/ddnet/15/servers.json", }; -IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IStorage *pStorage, IHttp *pHttp, const char *pPreviousBestUrl) +IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IStorage *pStorage, IHttp *pHttp, const char *pPreviousBestUrl) { char aaUrls[CChooseMaster::MAX_URLS][256]; const char *apUrls[CChooseMaster::MAX_URLS] = {0}; @@ -532,5 +531,5 @@ IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole break; } } - return new CServerBrowserHttp(pEngine, pConsole, pHttp, ppUrls, NumUrls, PreviousBestIndex); + return new CServerBrowserHttp(pEngine, pHttp, ppUrls, NumUrls, PreviousBestIndex); } diff --git a/src/engine/client/serverbrowser_http.h b/src/engine/client/serverbrowser_http.h index 64c83d8a6..91404341d 100644 --- a/src/engine/client/serverbrowser_http.h +++ b/src/engine/client/serverbrowser_http.h @@ -3,7 +3,6 @@ #include class CServerInfo; -class IConsole; class IEngine; class IStorage; class IHttp; @@ -26,5 +25,5 @@ public: virtual const NETADDR &LegacyServer(int Index) const = 0; }; -IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IStorage *pStorage, IHttp *pHttp, const char *pPreviousBestUrl); +IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IStorage *pStorage, IHttp *pHttp, const char *pPreviousBestUrl); #endif // ENGINE_CLIENT_SERVERBROWSER_HTTP_H From 5603d284bf4af763169d3c37f4ff17f68203fdf8 Mon Sep 17 00:00:00 2001 From: heinrich5991 Date: Thu, 14 Mar 2024 12:18:05 +0100 Subject: [PATCH 2/3] Parse `Date` and `Last-Modified` HTTP headers --- ddnet-libs | 2 +- src/engine/shared/http.cpp | 70 ++++++++++++++++++++++++++++++++++++++ src/engine/shared/http.h | 9 +++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/ddnet-libs b/ddnet-libs index a4fbe0e52..87fbb5783 160000 --- a/ddnet-libs +++ b/ddnet-libs @@ -1 +1 @@ -Subproject commit a4fbe0e52338a129bc3ac2e3a1de54de1d175279 +Subproject commit 87fbb57839080f40d16cd77d8ad11ae91bb9c849 diff --git a/src/engine/shared/http.cpp b/src/engine/shared/http.cpp index 76bbec657..6b72e980d 100644 --- a/src/engine/shared/http.cpp +++ b/src/engine/shared/http.cpp @@ -148,6 +148,8 @@ bool CHttpRequest::ConfigureHandle(void *pHandle) curl_easy_setopt(pH, CURLOPT_USERAGENT, GAME_NAME " " GAME_RELEASE_VERSION " (" CONF_PLATFORM_STRING "; " CONF_ARCH_STRING ")"); curl_easy_setopt(pH, CURLOPT_ACCEPT_ENCODING, ""); // Use any compression algorithm supported by libcurl. + curl_easy_setopt(pH, CURLOPT_HEADERDATA, this); + curl_easy_setopt(pH, CURLOPT_HEADERFUNCTION, HeaderCallback); curl_easy_setopt(pH, CURLOPT_WRITEDATA, this); curl_easy_setopt(pH, CURLOPT_WRITEFUNCTION, WriteCallback); curl_easy_setopt(pH, CURLOPT_NOPROGRESS, 0L); @@ -206,6 +208,52 @@ bool CHttpRequest::ConfigureHandle(void *pHandle) return true; } +size_t CHttpRequest::OnHeader(char *pHeader, size_t HeaderSize) +{ + // `pHeader` is NOT null-terminated. + // `pHeader` has a trailing newline. + + if(HeaderSize <= 1) + { + m_HeadersEnded = true; + return HeaderSize; + } + if(m_HeadersEnded) + { + // redirect, clear old headers + m_HeadersEnded = false; + m_ResultDate = {}; + m_ResultLastModified = {}; + } + + static const char DATE[] = "Date: "; + static const char LAST_MODIFIED[] = "Last-Modified: "; + + // Trailing newline and null termination evens out. + if(HeaderSize - 1 >= sizeof(DATE) - 1 && str_startswith_nocase(pHeader, DATE)) + { + char aValue[128]; + str_truncate(aValue, sizeof(aValue), pHeader + (sizeof(DATE) - 1), HeaderSize - (sizeof(DATE) - 1) - 1); + int64_t Value = curl_getdate(aValue, nullptr); + if(Value != -1) + { + m_ResultDate = Value; + } + } + if(HeaderSize - 1 >= sizeof(LAST_MODIFIED) - 1 && str_startswith_nocase(pHeader, LAST_MODIFIED)) + { + char aValue[128]; + str_truncate(aValue, sizeof(aValue), pHeader + (sizeof(LAST_MODIFIED) - 1), HeaderSize - (sizeof(LAST_MODIFIED) - 1) - 1); + int64_t Value = curl_getdate(aValue, nullptr); + if(Value != -1) + { + m_ResultLastModified = Value; + } + } + + return HeaderSize; +} + size_t CHttpRequest::OnData(char *pData, size_t DataSize) { // Need to check for the maximum response size here as curl can only @@ -244,6 +292,12 @@ size_t CHttpRequest::OnData(char *pData, size_t DataSize) } } +size_t CHttpRequest::HeaderCallback(char *pData, size_t Size, size_t Number, void *pUser) +{ + dbg_assert(Size == 1, "invalid size parameter passed to header callback"); + return ((CHttpRequest *)pUser)->OnHeader(pData, Number); +} + size_t CHttpRequest::WriteCallback(char *pData, size_t Size, size_t Number, void *pUser) { return ((CHttpRequest *)pUser)->OnData(pData, Size * Number); @@ -390,6 +444,22 @@ int CHttpRequest::StatusCode() const return m_StatusCode; } +std::optional CHttpRequest::ResultAgeSeconds() const +{ + dbg_assert(State() == EHttpState::DONE, "Request not done"); + if(!m_ResultDate || !m_ResultLastModified) + { + return {}; + } + return *m_ResultDate - *m_ResultLastModified; +} + +std::optional CHttpRequest::ResultLastModified() const +{ + dbg_assert(State() == EHttpState::DONE, "Request not done"); + return m_ResultLastModified; +} + bool CHttp::Init(std::chrono::milliseconds ShutdownDelay) { m_ShutdownDelay = ShutdownDelay; diff --git a/src/engine/shared/http.h b/src/engine/shared/http.h index 87b808be9..86e53fac9 100644 --- a/src/engine/shared/http.h +++ b/src/engine/shared/http.h @@ -118,6 +118,9 @@ class CHttpRequest : public IHttpRequest std::atomic m_Abort{false}; int m_StatusCode = 0; + bool m_HeadersEnded = false; + std::optional m_ResultDate = {}; + std::optional m_ResultLastModified = {}; // Abort the request with an error if `BeforeInit()` returns false. bool BeforeInit(); @@ -125,11 +128,15 @@ class CHttpRequest : public IHttpRequest // `pHandle` can be nullptr if no handle was ever created for this request. void OnCompletionInternal(void *pHandle, unsigned int Result); // void * == CURL *, unsigned int == CURLcode + // Abort the request if `OnHeader()` returns something other than + // `DataSize`. `pHeader` is NOT null-terminated. + size_t OnHeader(char *pHeader, size_t HeaderSize); // Abort the request if `OnData()` returns something other than // `DataSize`. size_t OnData(char *pData, size_t DataSize); static int ProgressCallback(void *pUser, double DlTotal, double DlCurr, double UlTotal, double UlCurr); + static size_t HeaderCallback(char *pData, size_t Size, size_t Number, void *pUser); static size_t WriteCallback(char *pData, size_t Size, size_t Number, void *pUser); protected: @@ -207,6 +214,8 @@ public: const SHA256_DIGEST &ResultSha256() const; int StatusCode() const; + std::optional ResultAgeSeconds() const; + std::optional ResultLastModified() const; }; inline std::unique_ptr HttpHead(const char *pUrl) From ff7b6ffe14c8cecde91005314b814208208b3198 Mon Sep 17 00:00:00 2001 From: heinrich5991 Date: Thu, 14 Mar 2024 12:19:18 +0100 Subject: [PATCH 3/3] Take serverlist age into account when choosing master Prefer masters with newer server lists, and try re-selecting the chosen master if the result is older than 5 minutes. --- src/engine/client/serverbrowser_http.cpp | 43 ++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/src/engine/client/serverbrowser_http.cpp b/src/engine/client/serverbrowser_http.cpp index 71b7ba54c..4a64bacb8 100644 --- a/src/engine/client/serverbrowser_http.cpp +++ b/src/engine/client/serverbrowser_http.cpp @@ -21,6 +21,28 @@ using namespace std::chrono_literals; +static int SanitizeAge(std::optional Age) +{ + // A year is of course pi*10**7 seconds. + if(!(Age && 0 <= *Age && *Age < 31415927)) + { + return 31415927; + } + return *Age; +} + +// Classify HTTP responses into buckets, treat 15 seconds as fresh, 1 minute as +// less fresh, etc. This ensures that differences in the order of seconds do +// not affect master choice. +static int ClassifyAge(int AgeSeconds) +{ + return 0 // + + (AgeSeconds >= 15) // 15 seconds + + (AgeSeconds >= 60) // 1 minute + + (AgeSeconds >= 300) // 5 minutes + + (AgeSeconds / 3600); // 1 hour +} + class CChooseMaster { public: @@ -185,9 +207,11 @@ void CChooseMaster::CJob::Run() // fail. CTimeout Timeout{10000, 0, 8000, 10}; int aTimeMs[MAX_URLS]; + int aAgeS[MAX_URLS]; for(int i = 0; i < m_pData->m_NumUrls; i++) { aTimeMs[i] = -1; + aAgeS[i] = SanitizeAge({}); const char *pUrl = m_pData->m_aaUrls[aRandomized[i]]; std::shared_ptr pHead = HttpHead(pUrl); pHead->Timeout(Timeout); @@ -243,22 +267,27 @@ void CChooseMaster::CJob::Run() { continue; } - log_info("serverbrowse_http", "found master, url='%s' time=%dms", pUrl, (int)Time.count()); + int AgeS = SanitizeAge(pGet->ResultAgeSeconds()); + log_info("serverbrowse_http", "found master, url='%s' time=%dms age=%ds", pUrl, (int)Time.count(), AgeS); + aTimeMs[i] = Time.count(); + aAgeS[i] = AgeS; } // Determine index of the minimum time. int BestIndex = -1; int BestTime = 0; + int BestAge = 0; for(int i = 0; i < m_pData->m_NumUrls; i++) { if(aTimeMs[i] < 0) { continue; } - if(BestIndex == -1 || aTimeMs[i] < BestTime) + if(BestIndex == -1 || std::tuple(ClassifyAge(aAgeS[i]), aTimeMs[i]) < std::tuple(ClassifyAge(BestAge), BestTime)) { BestTime = aTimeMs[i]; + BestAge = aAgeS[i]; BestIndex = aRandomized[i]; } } @@ -268,7 +297,7 @@ void CChooseMaster::CJob::Run() return; } - log_info("serverbrowse_http", "determined best master, url='%s' time=%dms", m_pData->m_aaUrls[BestIndex], BestTime); + log_info("serverbrowse_http", "determined best master, url='%s' time=%dms age=%ds", m_pData->m_aaUrls[BestIndex], BestTime, BestAge); m_pData->m_BestIndex.store(BestIndex); } @@ -371,12 +400,20 @@ void CServerBrowserHttp::Update() Success = Success && pJson; Success = Success && !Parse(pJson, &m_vServers, &m_vLegacyServers); json_value_free(pJson); + int Age = SanitizeAge(pGetServers->ResultAgeSeconds()); if(!Success) { log_error("serverbrowse_http", "failed getting serverlist, trying to find best URL"); m_pChooseMaster->Reset(); m_pChooseMaster->Refresh(); } + // Try to find new master if the current one returns results + // that are 5 minutes old. + else if(Age > 300) + { + log_info("serverbrowse_http", "got stale serverlist, age=%ds, trying to find best URL", Age); + m_pChooseMaster->Refresh(); + } } } void CServerBrowserHttp::Refresh()