Fix map downloading overriding more recent map change messages

Following sequence of events was possible:

1. HTTP map download for map X is started (e.g. client initially connects to server).
2. Map change message for map Y arrives while downloading (e.g. vote for another map has passed on the server).
3. Client already has map Y and loads it directly without starting or resetting the map download.
4. HTTP map download task for map X finishes.
5. Map X is loaded, overriding the map Y that should be loaded. This is one cause for #2804.

This could potentially also happen with the map download from the game-server, if the final map data chunk for a downloaded map is received directly after another map change message for a map which does not need to be downloaded, though this seems more unlikely.

This is fixed by resetting the current map download immediately after receiving a valid map change message instead of only when starting a new map download.

The relevant code to reset the map download is moved to `CClient::ResetMapDownload` to avoid duplicate code and ensure that the temporary map download file is always deleted and closed.
This commit is contained in:
Robert Müller 2024-08-28 23:04:53 +02:00
parent 3d5b533816
commit 05970178b2

View file

@ -247,11 +247,7 @@ void CClient::SendReady(int Conn)
void CClient::SendMapRequest()
{
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
dbg_assert(!m_MapdownloadFileTemp, "Map download already in progress");
m_MapdownloadFileTemp = Storage()->OpenFile(m_aMapdownloadFilenameTemp, IOFLAG_WRITE, IStorage::TYPE_SAVE);
if(IsSixup())
{
@ -656,6 +652,7 @@ void CClient::DisconnectWithReason(const char *pReason)
m_aRconAuthed[0] = 0;
mem_zero(m_aRconUsername, sizeof(m_aRconUsername));
mem_zero(m_aRconPassword, sizeof(m_aRconPassword));
m_MapDetailsPresent = false;
m_ServerSentCapabilities = false;
m_UseTempRconCommands = 0;
m_ExpectedRconCommands = -1;
@ -671,22 +668,10 @@ void CClient::DisconnectWithReason(const char *pReason)
m_CurrentServerCurrentPingTime = -1;
m_CurrentServerNextPingTime = -1;
// disable all downloads
m_MapdownloadChunk = 0;
if(m_pMapdownloadTask)
m_pMapdownloadTask->Abort();
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
m_MapdownloadFileTemp = 0;
m_MapdownloadSha256Present = false;
m_MapdownloadSha256 = SHA256_ZEROED;
m_MapdownloadCrc = 0;
m_MapdownloadTotalsize = -1;
m_MapdownloadAmount = 0;
m_MapDetailsPresent = false;
ResetMapDownload();
m_aMapdownloadFilename[0] = '\0';
m_aMapdownloadFilenameTemp[0] = '\0';
m_aMapdownloadName[0] = '\0';
// clear the current server info
mem_zero(&m_CurrentServerInfo, sizeof(m_CurrentServerInfo));
@ -1543,6 +1528,8 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
DummyDisconnect(0);
}
ResetMapDownload();
SHA256_DIGEST *pMapSha256 = nullptr;
const char *pMapUrl = nullptr;
if(MapDetailsWerePresent && str_comp(m_aMapDetailsName, pMap) == 0 && m_MapDetailsCrc == MapCrc)
@ -1559,12 +1546,6 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
}
else
{
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
// start map download
FormatMapDownloadFilename(pMap, pMapSha256, MapCrc, false, m_aMapdownloadFilename, sizeof(m_aMapdownloadFilename));
FormatMapDownloadFilename(pMap, pMapSha256, MapCrc, true, m_aMapdownloadFilenameTemp, sizeof(m_aMapdownloadFilenameTemp));
@ -1573,16 +1554,11 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
str_format(aBuf, sizeof(aBuf), "starting to download map to '%s'", m_aMapdownloadFilenameTemp);
m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "client/network", aBuf);
m_MapdownloadChunk = 0;
str_copy(m_aMapdownloadName, pMap);
m_MapdownloadSha256Present = (bool)pMapSha256;
m_MapdownloadSha256 = pMapSha256 ? *pMapSha256 : SHA256_ZEROED;
m_MapdownloadCrc = MapCrc;
m_MapdownloadTotalsize = MapSize;
m_MapdownloadAmount = 0;
ResetMapDownload();
if(pMapSha256)
{
@ -2225,9 +2201,25 @@ void CClient::ResetMapDownload()
if(m_pMapdownloadTask)
{
m_pMapdownloadTask->Abort();
m_pMapdownloadTask = NULL;
m_pMapdownloadTask = nullptr;
}
m_MapdownloadFileTemp = 0;
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
m_MapdownloadFileTemp = 0;
}
if(Storage()->FileExists(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE))
{
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
m_MapdownloadChunk = 0;
m_MapdownloadSha256Present = false;
m_MapdownloadSha256 = SHA256_ZEROED;
m_MapdownloadCrc = 0;
m_MapdownloadTotalsize = -1;
m_MapdownloadAmount = 0;
}
@ -2235,9 +2227,8 @@ void CClient::FinishMapDownload()
{
m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "client/network", "download complete, loading map");
int Prev = m_MapdownloadTotalsize;
m_MapdownloadTotalsize = -1;
SHA256_DIGEST *pSha256 = m_MapdownloadSha256Present ? &m_MapdownloadSha256 : 0;
const int PrevMapdownloadTotalsize = m_MapdownloadTotalsize;
SHA256_DIGEST *pSha256 = m_MapdownloadSha256Present ? &m_MapdownloadSha256 : nullptr;
bool FileSuccess = true;
if(Storage()->FileExists(m_aMapdownloadFilename, IStorage::TYPE_SAVE))
@ -2252,7 +2243,6 @@ void CClient::FinishMapDownload()
return;
}
// load map
const char *pError = LoadMap(m_aMapdownloadName, m_aMapdownloadFilename, pSha256, m_MapdownloadCrc);
if(!pError)
{
@ -2263,17 +2253,11 @@ void CClient::FinishMapDownload()
else if(m_pMapdownloadTask) // fallback
{
ResetMapDownload();
m_MapdownloadTotalsize = Prev;
m_MapdownloadTotalsize = PrevMapdownloadTotalsize;
SendMapRequest();
}
else
{
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
m_MapdownloadFileTemp = 0;
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
ResetMapDownload();
DisconnectWithReason(pError);
}