From 60807b5de9dcd3782ba360e20ef366ff93e6ea0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Wed, 18 Oct 2023 21:00:23 +0200 Subject: [PATCH 1/4] Fix `io_skip` return type and documentation The function returns 0 on success and -1 on error, as it delegates to `io_seek`, instead of returning the number of bytes skipped, which the documentation previously suggested. The return type is changed from `unsigned` to `int`, as this is also the type of `io_seek` and this better handles -1 as a return value. --- src/base/system.cpp | 2 +- src/base/system.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/base/system.cpp b/src/base/system.cpp index 1bd0b64a7..480fac2ad 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -389,7 +389,7 @@ char *io_read_all_str(IOHANDLE io) return (char *)buffer; } -unsigned io_skip(IOHANDLE io, int size) +int io_skip(IOHANDLE io, int size) { return io_seek(io, size, IOSEEK_CUR); } diff --git a/src/base/system.h b/src/base/system.h index ba1518c15..19b9ce5f6 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -292,9 +292,9 @@ char *io_read_all_str(IOHANDLE io); * @param io Handle to the file. * @param size Number of bytes to skip. * - * @return Number of bytes skipped. + * @return 0 on success. */ -unsigned io_skip(IOHANDLE io, int size); +int io_skip(IOHANDLE io, int size); /** * Writes data from a buffer to file. From 69517956194c8506248af86c0c9749b3620456e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Thu, 19 Oct 2023 21:01:00 +0200 Subject: [PATCH 2/4] Add title to engine warnings and make auto-hiding optional Make the title of warnings adjustable, with the default title being "Warning" to preserve existing code. Make auto-hiding configurable, so the automatic closing of warning popups after 10 seconds can be toggled. --- src/engine/warning.h | 18 ++++++++++++------ src/game/client/components/menus.cpp | 2 +- src/game/client/gameclient.cpp | 4 ++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/engine/warning.h b/src/engine/warning.h index 64941b504..0214a6b0f 100644 --- a/src/engine/warning.h +++ b/src/engine/warning.h @@ -3,15 +3,21 @@ struct SWarning { - SWarning() : - m_WasShown(false) {} - SWarning(const char *pMsg) : - m_WasShown(false) + SWarning() {} + SWarning(const char *pMsg) { - str_copy(m_aWarningMsg, pMsg, sizeof(m_aWarningMsg)); + str_copy(m_aWarningTitle, ""); + str_copy(m_aWarningMsg, pMsg); } + SWarning(const char *pTitle, const char *pMsg) + { + str_copy(m_aWarningTitle, pTitle); + str_copy(m_aWarningMsg, pMsg); + } + char m_aWarningTitle[128]; char m_aWarningMsg[256]; - bool m_WasShown; + bool m_WasShown = false; + bool m_AutoHide = true; }; #endif diff --git a/src/game/client/components/menus.cpp b/src/game/client/components/menus.cpp index d12523b7c..1d9aeb4bf 100644 --- a/src/game/client/components/menus.cpp +++ b/src/game/client/components/menus.cpp @@ -1786,7 +1786,7 @@ int CMenus::Render() Part.VMargin(120.0f, &Part); static CButtonContainer s_Button; - if(DoButton_Menu(&s_Button, pButtonText, 0, &Part) || UI()->ConsumeHotkey(CUI::HOTKEY_ESCAPE) || UI()->ConsumeHotkey(CUI::HOTKEY_ENTER) || (time_get_nanoseconds() - m_PopupWarningLastTime >= m_PopupWarningDuration)) + if(DoButton_Menu(&s_Button, pButtonText, 0, &Part) || UI()->ConsumeHotkey(CUI::HOTKEY_ESCAPE) || UI()->ConsumeHotkey(CUI::HOTKEY_ENTER) || (m_PopupWarningDuration > 0s && time_get_nanoseconds() - m_PopupWarningLastTime >= m_PopupWarningDuration)) { m_Popup = POPUP_NONE; SetActive(false); diff --git a/src/game/client/gameclient.cpp b/src/game/client/gameclient.cpp index c9e5c7439..e23fb57e1 100644 --- a/src/game/client/gameclient.cpp +++ b/src/game/client/gameclient.cpp @@ -673,9 +673,9 @@ void CGameClient::OnRender() // display gfx & client warnings for(SWarning *pWarning : {Graphics()->GetCurWarning(), Client()->GetCurWarning()}) { - if(pWarning != NULL && m_Menus.CanDisplayWarning()) + if(pWarning != nullptr && m_Menus.CanDisplayWarning()) { - m_Menus.PopupWarning(Localize("Warning"), pWarning->m_aWarningMsg, "Ok", 10s); + m_Menus.PopupWarning(pWarning->m_aWarningTitle[0] == '\0' ? Localize("Warning") : pWarning->m_aWarningTitle, pWarning->m_aWarningMsg, "Ok", pWarning->m_AutoHide ? 10s : 0s); pWarning->m_WasShown = true; } } From 92e2e17f0f889b0ae15ebc5c73235226ec2b31ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sun, 15 Oct 2023 13:49:15 +0200 Subject: [PATCH 3/4] Validate ticks when reading demo chunk headers Add checks to ensure that the ticks read from demo chunk headers are in the valid range (cf. `MIN_TICK` and `MAX_TICK` constants). Playing demos with invalid ticks is prevented entirely, as the invalid ticks would cause issues in other places. The server never uses ticks outside the valid range, so invalid ticks should never occur in correctly recorded demos. Additionally, checks are added to detect if a tickmarker chunk with a tick delta occurs before a tickmarker chunk defining an initial tick. The tick delta is only meaningful when an initial tick has already been defined, so if this is not the case the demo is also considered invalid. --- src/engine/shared/demo.cpp | 63 +++++++++++++++++++++++++++++--------- src/engine/shared/demo.h | 10 ++++-- 2 files changed, 56 insertions(+), 17 deletions(-) diff --git a/src/engine/shared/demo.cpp b/src/engine/shared/demo.cpp index df02c9781..e62089f30 100644 --- a/src/engine/shared/demo.cpp +++ b/src/engine/shared/demo.cpp @@ -418,14 +418,14 @@ void CDemoPlayer::SetListener(IListener *pListener) m_pListener = pListener; } -int CDemoPlayer::ReadChunkHeader(int *pType, int *pSize, int *pTick) +CDemoPlayer::EReadChunkHeaderResult CDemoPlayer::ReadChunkHeader(int *pType, int *pSize, int *pTick) { *pSize = 0; *pType = 0; unsigned char Chunk = 0; if(io_read(m_File, &Chunk, sizeof(Chunk)) != sizeof(Chunk)) - return -1; + return CHUNKHEADER_EOF; if(Chunk & CHUNKTYPEFLAG_TICKMARKER) { @@ -433,22 +433,30 @@ int CDemoPlayer::ReadChunkHeader(int *pType, int *pSize, int *pTick) int Tickdelta_legacy = Chunk & CHUNKMASK_TICK_LEGACY; // compatibility *pType = Chunk & (CHUNKTYPEFLAG_TICKMARKER | CHUNKTICKFLAG_KEYFRAME); + int NewTick; if(m_Info.m_Header.m_Version < gs_VersionTickCompression && Tickdelta_legacy != 0) { - *pTick += Tickdelta_legacy; + if(*pTick < 0) // initial tick not initialized before a tick delta + return CHUNKHEADER_ERROR; + NewTick = *pTick + Tickdelta_legacy; } else if(Chunk & CHUNKTICKFLAG_TICK_COMPRESSED) { + if(*pTick < 0) // initial tick not initialized before a tick delta + return CHUNKHEADER_ERROR; int Tickdelta = Chunk & CHUNKMASK_TICK; - *pTick += Tickdelta; + NewTick = *pTick + Tickdelta; } else { unsigned char aTickdata[sizeof(int32_t)]; if(io_read(m_File, aTickdata, sizeof(aTickdata)) != sizeof(aTickdata)) - return -1; - *pTick = bytes_be_to_uint(aTickdata); + return CHUNKHEADER_ERROR; + NewTick = bytes_be_to_uint(aTickdata); } + if(NewTick < MIN_TICK || NewTick >= MAX_TICK) // invalid tick + return CHUNKHEADER_ERROR; + *pTick = NewTick; } else { @@ -460,34 +468,42 @@ int CDemoPlayer::ReadChunkHeader(int *pType, int *pSize, int *pTick) { unsigned char aSizedata[1]; if(io_read(m_File, aSizedata, sizeof(aSizedata)) != sizeof(aSizedata)) - return -1; + return CHUNKHEADER_ERROR; *pSize = aSizedata[0]; } else if(*pSize == 31) { unsigned char aSizedata[2]; if(io_read(m_File, aSizedata, sizeof(aSizedata)) != sizeof(aSizedata)) - return -1; + return CHUNKHEADER_ERROR; *pSize = (aSizedata[1] << 8) | aSizedata[0]; } } - return 0; + return CHUNKHEADER_SUCCESS; } -void CDemoPlayer::ScanFile() +bool CDemoPlayer::ScanFile() { const long StartPos = io_tell(m_File); m_vKeyFrames.clear(); - int ChunkTick = 0; + int ChunkTick = -1; while(true) { const long CurrentPos = io_tell(m_File); int ChunkType, ChunkSize; - if(ReadChunkHeader(&ChunkType, &ChunkSize, &ChunkTick)) + const EReadChunkHeaderResult Result = ReadChunkHeader(&ChunkType, &ChunkSize, &ChunkTick); + if(Result == CHUNKHEADER_EOF) + { break; + } + else if(Result == CHUNKHEADER_ERROR) + { + m_vKeyFrames.clear(); + return false; + } if(ChunkType & CHUNKTYPEFLAG_TICKMARKER) { @@ -505,6 +521,7 @@ void CDemoPlayer::ScanFile() } io_seek(m_File, StartPos, IOSEEK_START); + return true; } void CDemoPlayer::DoTick() @@ -527,11 +544,18 @@ void CDemoPlayer::DoTick() while(true) { int ChunkType, ChunkSize; - if(ReadChunkHeader(&ChunkType, &ChunkSize, &ChunkTick)) + const EReadChunkHeaderResult Result = ReadChunkHeader(&ChunkType, &ChunkSize, &ChunkTick); + if(Result != CHUNKHEADER_SUCCESS) { // stop on error or eof if(m_pConsole) - m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", "end of file"); + { + if(Result == CHUNKHEADER_EOF) + m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", "end of file"); + else + m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", "error reading chunk header"); + } + #if defined(CONF_VIDEORECORDER) if(m_UseVideo && IVideo::Current()) Stop(); @@ -782,7 +806,16 @@ int CDemoPlayer::Load(class IStorage *pStorage, class IConsole *pConsole, const } // scan the file for interesting points - ScanFile(); + if(!ScanFile()) + { + if(m_pConsole) + { + m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", "Error scanning demo file"); + } + io_close(m_File); + m_File = 0; + return -1; + } // reset slice markers g_Config.m_ClDemoSliceBegin = -1; diff --git a/src/engine/shared/demo.h b/src/engine/shared/demo.h index 0a4a9a457..14b127d0e 100644 --- a/src/engine/shared/demo.h +++ b/src/engine/shared/demo.h @@ -123,9 +123,15 @@ private: bool m_UseVideo; bool m_WasRecording = false; - int ReadChunkHeader(int *pType, int *pSize, int *pTick); + enum EReadChunkHeaderResult + { + CHUNKHEADER_SUCCESS, + CHUNKHEADER_ERROR, + CHUNKHEADER_EOF, + }; + EReadChunkHeaderResult ReadChunkHeader(int *pType, int *pSize, int *pTick); void DoTick(); - void ScanFile(); + bool ScanFile(); int64_t Time(); From 0e4f174f786c74a75720d90e0abfca1ef15331f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Wed, 18 Oct 2023 21:21:26 +0200 Subject: [PATCH 4/4] Check for all file errors in demo player, show demo error popup Handle the return values of all uses of the `io_read`, `io_skip` and `io_tell` functions in `CDemoPlayer`, to handle truncated demo files and other unexpected file reading and seeking errors during demo playback. Show more detailed error message popups when demos cannot be loaded due to errors and when demo playback is stopped unexpectedly due to errors. Previously, only a generic message "Error loading demo" was shown when loading failed and no error message was shown when demo playback is stopped due to errors. --- src/engine/client/client.cpp | 28 ++- src/engine/demo.h | 6 +- src/engine/shared/demo.cpp | 284 +++++++++++----------- src/engine/shared/demo.h | 8 +- src/game/client/components/menus.cpp | 2 +- src/game/client/components/menus_demo.cpp | 6 +- 6 files changed, 176 insertions(+), 158 deletions(-) diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index 7f76aeafc..90862b313 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -609,6 +609,9 @@ void CClient::Connect(const char *pAddress, const char *pPassword) void CClient::DisconnectWithReason(const char *pReason) { + if(pReason != nullptr && pReason[0] == '\0') + pReason = nullptr; + char aBuf[512]; str_format(aBuf, sizeof(aBuf), "disconnecting. reason='%s'", pReason ? pReason : "unknown"); m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "client", aBuf, gs_ClientNetworkPrintColor); @@ -2437,7 +2440,13 @@ void CClient::Update() else { // disconnect on error - Disconnect(); + DisconnectWithReason(m_DemoPlayer.ErrorMessage()); + if(m_DemoPlayer.ErrorMessage()[0] != '\0') + { + SWarning Warning(Localize("Error playing demo"), m_DemoPlayer.ErrorMessage()); + Warning.m_AutoHide = false; + m_vWarnings.emplace_back(Warning); + } } } else if(State() == IClient::STATE_ONLINE) @@ -3581,27 +3590,23 @@ void CClient::DemoSlice(const char *pDstPath, CLIENTFUNC_FILTER pfnFilter, void { if(m_DemoPlayer.IsPlaying()) { - const char *pDemoFileName = m_DemoPlayer.GetDemoFileName(); - m_DemoEditor.Slice(pDemoFileName, pDstPath, g_Config.m_ClDemoSliceBegin, g_Config.m_ClDemoSliceEnd, pfnFilter, pUser); + m_DemoEditor.Slice(m_DemoPlayer.Filename(), pDstPath, g_Config.m_ClDemoSliceBegin, g_Config.m_ClDemoSliceEnd, pfnFilter, pUser); } } const char *CClient::DemoPlayer_Play(const char *pFilename, int StorageType) { - IOHANDLE File = Storage()->OpenFile(pFilename, IOFLAG_READ, StorageType); - if(!File) - return "error opening demo file"; - - io_close(File); + // Don't disconnect unless the file exists (only for play command) + if(!Storage()->FileExists(pFilename, StorageType)) + return "No demo with this filename exists"; Disconnect(); m_aNetClient[CONN_MAIN].ResetErrorString(); // try to start playback m_DemoPlayer.SetListener(this); - if(m_DemoPlayer.Load(Storage(), m_pConsole, pFilename, StorageType)) - return "error loading demo"; + return m_DemoPlayer.ErrorMessage(); // load map const CMapInfo *pMapInfo = m_DemoPlayer.GetMapInfo(); @@ -3669,8 +3674,7 @@ const char *CClient::DemoPlayer_Render(const char *pFilename, int StorageType, c { m_DemoPlayer.Pause(); } - //m_pConsole->Print(IConsole::OUTPUT_LEVEL_DEBUG, "demo_recorder", "demo eof"); - return 0; + return nullptr; } #endif diff --git a/src/engine/demo.h b/src/engine/demo.h index 2bd27eefe..8e5da1b39 100644 --- a/src/engine/demo.h +++ b/src/engine/demo.h @@ -58,8 +58,8 @@ struct CMapInfo { char m_aName[MAX_MAP_LENGTH]; SHA256_DIGEST m_Sha256; - int m_Crc; - int m_Size; + unsigned m_Crc; + unsigned m_Size; }; class IDemoPlayer : public IInterface @@ -100,7 +100,7 @@ public: virtual bool IsPlaying() const = 0; virtual const CInfo *BaseInfo() const = 0; virtual void GetDemoName(char *pBuffer, size_t BufferSize) const = 0; - virtual bool GetDemoInfo(class IStorage *pStorage, const char *pFilename, int StorageType, CDemoHeader *pDemoHeader, CTimelineMarkers *pTimelineMarkers, CMapInfo *pMapInfo) const = 0; + virtual bool GetDemoInfo(class IStorage *pStorage, class IConsole *pConsole, const char *pFilename, int StorageType, CDemoHeader *pDemoHeader, CTimelineMarkers *pTimelineMarkers, CMapInfo *pMapInfo, IOHANDLE *pFile = nullptr, char *pErrorMessage = nullptr, size_t ErrorMessageSize = 0) const = 0; }; class IDemoRecorder : public IInterface diff --git a/src/engine/shared/demo.cpp b/src/engine/shared/demo.cpp index e62089f30..6666752a3 100644 --- a/src/engine/shared/demo.cpp +++ b/src/engine/shared/demo.cpp @@ -411,6 +411,9 @@ void CDemoPlayer::Construct(class CSnapshotDelta *pSnapshotDelta, bool UseVideo) m_LastSnapshotDataSize = -1; m_pListener = nullptr; m_UseVideo = UseVideo; + + m_aFilename[0] = '\0'; + m_aErrorMessage[0] = '\0'; } void CDemoPlayer::SetListener(IListener *pListener) @@ -487,11 +490,18 @@ bool CDemoPlayer::ScanFile() { const long StartPos = io_tell(m_File); m_vKeyFrames.clear(); + if(StartPos < 0) + return false; int ChunkTick = -1; while(true) { const long CurrentPos = io_tell(m_File); + if(CurrentPos < 0) + { + m_vKeyFrames.clear(); + return false; + } int ChunkType, ChunkSize; const EReadChunkHeaderResult Result = ReadChunkHeader(&ChunkType, &ChunkSize, &ChunkTick); @@ -517,10 +527,20 @@ bool CDemoPlayer::ScanFile() m_Info.m_Info.m_LastTick = ChunkTick; } else if(ChunkSize) - io_skip(m_File, ChunkSize); + { + if(io_skip(m_File, ChunkSize) != 0) + { + m_vKeyFrames.clear(); + return false; + } + } } - io_seek(m_File, StartPos, IOSEEK_START); + if(io_seek(m_File, StartPos, IOSEEK_START) != 0) + { + m_vKeyFrames.clear(); + return false; + } return true; } @@ -545,29 +565,26 @@ void CDemoPlayer::DoTick() { int ChunkType, ChunkSize; const EReadChunkHeaderResult Result = ReadChunkHeader(&ChunkType, &ChunkSize, &ChunkTick); - if(Result != CHUNKHEADER_SUCCESS) + if(Result == CHUNKHEADER_EOF) { - // stop on error or eof - if(m_pConsole) - { - if(Result == CHUNKHEADER_EOF) - m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", "end of file"); - else - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", "error reading chunk header"); - } - -#if defined(CONF_VIDEORECORDER) - if(m_UseVideo && IVideo::Current()) - Stop(); -#endif if(m_Info.m_PreviousTick == -1) { - if(m_pConsole) - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", "empty demo"); - Stop(); + Stop("Empty demo"); } else + { Pause(); + // Stop rendering when reaching end of file +#if defined(CONF_VIDEORECORDER) + if(m_UseVideo && IVideo::Current()) + Stop(); +#endif + } + break; + } + else if(Result == CHUNKHEADER_ERROR) + { + Stop("Error reading chunk header"); break; } @@ -577,30 +594,21 @@ void CDemoPlayer::DoTick() { if(io_read(m_File, m_aCompressedSnapshotData, ChunkSize) != (unsigned)ChunkSize) { - // stop on error or eof - if(m_pConsole) - m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", "error reading chunk"); - Stop(); + Stop("Error reading chunk data"); break; } DataSize = CNetBase::Decompress(m_aCompressedSnapshotData, ChunkSize, m_aDecompressedSnapshotData, sizeof(m_aDecompressedSnapshotData)); if(DataSize < 0) { - // stop on error or eof - if(m_pConsole) - m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", "error during network decompression"); - Stop(); + Stop("Error during network decompression"); break; } DataSize = CVariableInt::Decompress(m_aDecompressedSnapshotData, DataSize, m_aCurrentSnapshotData, sizeof(m_aCurrentSnapshotData)); - if(DataSize < 0) { - if(m_pConsole) - m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", "error during intpack decompression"); - Stop(); + Stop("Error during intpack decompression"); break; } } @@ -615,8 +623,8 @@ void CDemoPlayer::DoTick() { if(m_pConsole) { - char aBuf[256]; - str_format(aBuf, sizeof(aBuf), "error during unpacking of delta, err=%d", DataSize); + char aBuf[64]; + str_format(aBuf, sizeof(aBuf), "Error unpacking snapshot delta. DataSize=%d", DataSize); m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", aBuf); } } @@ -624,8 +632,8 @@ void CDemoPlayer::DoTick() { if(m_pConsole) { - char aBuf[256]; - str_format(aBuf, sizeof(aBuf), "snapshot delta invalid. DataSize=%d", DataSize); + char aBuf[64]; + str_format(aBuf, sizeof(aBuf), "Snapshot delta invalid. DataSize=%d", DataSize); m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", aBuf); } } @@ -647,8 +655,8 @@ void CDemoPlayer::DoTick() { if(m_pConsole) { - char aBuf[256]; - str_format(aBuf, sizeof(aBuf), "snapshot invalid. DataSize=%d", DataSize); + char aBuf[64]; + str_format(aBuf, sizeof(aBuf), "Snapshot invalid. DataSize=%d", DataSize); m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", aBuf); } } @@ -709,20 +717,15 @@ int CDemoPlayer::Load(class IStorage *pStorage, class IConsole *pConsole, const dbg_assert(m_File == 0, "Demo player already playing"); m_pConsole = pConsole; - m_File = pStorage->OpenFile(pFilename, IOFLAG_READ, StorageType); - if(!m_File) - { - if(m_pConsole) - { - char aBuf[32 + IO_MAX_PATH_LENGTH]; - str_format(aBuf, sizeof(aBuf), "could not open '%s'", pFilename); - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", aBuf); - } - return -1; - } - - // store the filename str_copy(m_aFilename, pFilename); + str_copy(m_aErrorMessage, ""); + + if(m_pConsole) + { + char aBuf[32 + IO_MAX_PATH_LENGTH]; + str_format(aBuf, sizeof(aBuf), "Loading demo '%s'", pFilename); + m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", aBuf); + } // clear the playback info mem_zero(&m_Info, sizeof(m_Info)); @@ -733,66 +736,21 @@ int CDemoPlayer::Load(class IStorage *pStorage, class IConsole *pConsole, const m_Info.m_PreviousTick = -1; m_Info.m_Info.m_Speed = 1; m_SpeedIndex = 4; - m_LastSnapshotDataSize = -1; - // read the header - if(io_read(m_File, &m_Info.m_Header, sizeof(m_Info.m_Header)) != sizeof(m_Info.m_Header) || !m_Info.m_Header.Valid()) + if(!GetDemoInfo(pStorage, m_pConsole, pFilename, StorageType, &m_Info.m_Header, &m_Info.m_TimelineMarkers, &m_MapInfo, &m_File, m_aErrorMessage, sizeof(m_aErrorMessage))) { - if(m_pConsole) - { - char aBuf[32 + IO_MAX_PATH_LENGTH]; - str_format(aBuf, sizeof(aBuf), "'%s' is not a valid demo file", pFilename); - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", aBuf); - } - io_close(m_File); - m_File = 0; + str_copy(m_aFilename, ""); return -1; } - if(m_Info.m_Header.m_Version < gs_OldVersion) - { - if(m_pConsole) - { - char aBuf[256]; - str_format(aBuf, sizeof(aBuf), "demo version %d is not supported", m_Info.m_Header.m_Version); - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", aBuf); - } - io_close(m_File); - m_File = 0; - return -1; - } - else if(m_Info.m_Header.m_Version > gs_OldVersion) - io_read(m_File, &m_Info.m_TimelineMarkers, sizeof(m_Info.m_TimelineMarkers)); - - SHA256_DIGEST Sha256 = SHA256_ZEROED; - if(m_Info.m_Header.m_Version >= gs_Sha256Version) - { - CUuid ExtensionUuid = {}; - io_read(m_File, &ExtensionUuid.m_aData, sizeof(ExtensionUuid.m_aData)); - - if(ExtensionUuid == SHA256_EXTENSION) - { - io_read(m_File, &Sha256, sizeof(SHA256_DIGEST)); // need a safe read - } - else - { - // This hopes whatever happened during the version increment didn't add something here - dbg_msg("demo", "demo version incremented, but not by ddnet"); - io_seek(m_File, -(int)sizeof(ExtensionUuid.m_aData), IOSEEK_CUR); - } - } - // save byte offset of map for later use - const unsigned MapSize = bytes_be_to_uint(m_Info.m_Header.m_aMapSize); m_MapOffset = io_tell(m_File); - io_skip(m_File, MapSize); - - // store map information - m_MapInfo.m_Crc = bytes_be_to_uint(m_Info.m_Header.m_aMapCrc); - m_MapInfo.m_Sha256 = Sha256; - m_MapInfo.m_Size = MapSize; - str_copy(m_MapInfo.m_aName, m_Info.m_Header.m_aMapName); + if(m_MapOffset < 0 || io_skip(m_File, m_MapInfo.m_Size) != 0) + { + Stop("Error skipping map data"); + return -1; + } if(m_Info.m_Header.m_Version > gs_OldVersion) { @@ -808,12 +766,7 @@ int CDemoPlayer::Load(class IStorage *pStorage, class IConsole *pConsole, const // scan the file for interesting points if(!ScanFile()) { - if(m_pConsole) - { - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", "Error scanning demo file"); - } - io_close(m_File); - m_File = 0; + Stop("Error scanning demo file"); return -1; } @@ -828,15 +781,18 @@ int CDemoPlayer::Load(class IStorage *pStorage, class IConsole *pConsole, const unsigned char *CDemoPlayer::GetMapData(class IStorage *pStorage) { if(!m_MapInfo.m_Size) - return 0; + return nullptr; - long CurSeek = io_tell(m_File); - - // get map data - io_seek(m_File, m_MapOffset, IOSEEK_START); + const long CurSeek = io_tell(m_File); + if(CurSeek < 0 || io_seek(m_File, m_MapOffset, IOSEEK_START) != 0) + return nullptr; unsigned char *pMapData = (unsigned char *)malloc(m_MapInfo.m_Size); - io_read(m_File, pMapData, m_MapInfo.m_Size); - io_seek(m_File, CurSeek, IOSEEK_START); + if(io_read(m_File, pMapData, m_MapInfo.m_Size) != m_MapInfo.m_Size || + io_seek(m_File, CurSeek, IOSEEK_START) != 0) + { + free(pMapData); + return nullptr; + } return pMapData; } @@ -970,7 +926,11 @@ int CDemoPlayer::SetPos(int WantedTick) KeyFrame--; // seek to the correct key frame - io_seek(m_File, m_vKeyFrames[KeyFrame].m_Filepos, IOSEEK_START); + if(io_seek(m_File, m_vKeyFrames[KeyFrame].m_Filepos, IOSEEK_START) != 0) + { + Stop("Error seeking keyframe position"); + return -1; + } m_Info.m_NextTick = -1; m_Info.m_Info.m_CurrentTick = -1; @@ -1043,7 +1003,7 @@ int CDemoPlayer::Update(bool RealTime) return 0; } -int CDemoPlayer::Stop() +void CDemoPlayer::Stop(const char *pErrorMessage) { #if defined(CONF_VIDEORECORDER) if(m_UseVideo && IVideo::Current()) @@ -1051,15 +1011,23 @@ int CDemoPlayer::Stop() #endif if(!m_File) - return -1; + return; if(m_pConsole) - m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", "Stopped playback"); + { + char aBuf[256]; + if(pErrorMessage[0] == '\0') + str_copy(aBuf, "Stopped playback"); + else + str_format(aBuf, sizeof(aBuf), "Stopped playback due to error: %s", pErrorMessage); + m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "demo_player", aBuf); + } + io_close(m_File); m_File = 0; m_vKeyFrames.clear(); str_copy(m_aFilename, ""); - return 0; + str_copy(m_aErrorMessage, pErrorMessage); } void CDemoPlayer::GetDemoName(char *pBuffer, size_t BufferSize) const @@ -1067,7 +1035,7 @@ void CDemoPlayer::GetDemoName(char *pBuffer, size_t BufferSize) const IStorage::StripPathAndExtension(m_aFilename, pBuffer, BufferSize); } -bool CDemoPlayer::GetDemoInfo(class IStorage *pStorage, const char *pFilename, int StorageType, CDemoHeader *pDemoHeader, CTimelineMarkers *pTimelineMarkers, CMapInfo *pMapInfo) const +bool CDemoPlayer::GetDemoInfo(IStorage *pStorage, IConsole *pConsole, const char *pFilename, int StorageType, CDemoHeader *pDemoHeader, CTimelineMarkers *pTimelineMarkers, CMapInfo *pMapInfo, IOHANDLE *pFile, char *pErrorMessage, size_t ErrorMessageSize) const { mem_zero(pDemoHeader, sizeof(CDemoHeader)); mem_zero(pTimelineMarkers, sizeof(CTimelineMarkers)); @@ -1075,45 +1043,87 @@ bool CDemoPlayer::GetDemoInfo(class IStorage *pStorage, const char *pFilename, i IOHANDLE File = pStorage->OpenFile(pFilename, IOFLAG_READ, StorageType); if(!File) - return false; - - if(io_read(File, pDemoHeader, sizeof(CDemoHeader)) != sizeof(CDemoHeader) || !pDemoHeader->Valid() || pDemoHeader->m_Version < gs_OldVersion) { + if(pErrorMessage != nullptr) + str_copy(pErrorMessage, "Could not open demo file", ErrorMessageSize); + return false; + } + + if(io_read(File, pDemoHeader, sizeof(CDemoHeader)) != sizeof(CDemoHeader) || !pDemoHeader->Valid()) + { + if(pErrorMessage != nullptr) + str_copy(pErrorMessage, "Error reading demo header", ErrorMessageSize); mem_zero(pDemoHeader, sizeof(CDemoHeader)); io_close(File); return false; } - if(pDemoHeader->m_Version > gs_OldVersion) + if(pDemoHeader->m_Version < gs_OldVersion) { - io_read(File, pTimelineMarkers, sizeof(CTimelineMarkers)); + if(pErrorMessage != nullptr) + str_format(pErrorMessage, ErrorMessageSize, "Demo version '%d' is not supported", pDemoHeader->m_Version); + mem_zero(pDemoHeader, sizeof(CDemoHeader)); + io_close(File); + return false; + } + else if(pDemoHeader->m_Version > gs_OldVersion) + { + if(io_read(File, pTimelineMarkers, sizeof(CTimelineMarkers)) != sizeof(CTimelineMarkers)) + { + if(pErrorMessage != nullptr) + str_copy(pErrorMessage, "Error reading timeline markers", ErrorMessageSize); + mem_zero(pDemoHeader, sizeof(CDemoHeader)); + io_close(File); + return false; + } } - - str_copy(pMapInfo->m_aName, pDemoHeader->m_aMapName); - pMapInfo->m_Crc = bytes_be_to_uint(pDemoHeader->m_aMapCrc); SHA256_DIGEST Sha256 = SHA256_ZEROED; if(pDemoHeader->m_Version >= gs_Sha256Version) { CUuid ExtensionUuid = {}; - io_read(File, &ExtensionUuid.m_aData, sizeof(ExtensionUuid.m_aData)); - - if(ExtensionUuid == SHA256_EXTENSION) + const unsigned ExtensionUuidSize = io_read(File, &ExtensionUuid.m_aData, sizeof(ExtensionUuid.m_aData)); + if(ExtensionUuidSize == sizeof(ExtensionUuid.m_aData) && ExtensionUuid == SHA256_EXTENSION) { - io_read(File, &Sha256, sizeof(SHA256_DIGEST)); // need a safe read + if(io_read(File, &Sha256, sizeof(SHA256_DIGEST)) != sizeof(SHA256_DIGEST)) + { + if(pErrorMessage != nullptr) + str_copy(pErrorMessage, "Error reading SHA256", ErrorMessageSize); + mem_zero(pDemoHeader, sizeof(CDemoHeader)); + mem_zero(pTimelineMarkers, sizeof(CTimelineMarkers)); + io_close(File); + return false; + } } else { // This hopes whatever happened during the version increment didn't add something here - dbg_msg("demo", "demo version incremented, but not by ddnet"); - io_seek(File, -(int)sizeof(ExtensionUuid.m_aData), IOSEEK_CUR); + if(pConsole) + { + pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "demo_player", "Demo version incremented, but not by DDNet"); + } + if(io_seek(File, -(int)ExtensionUuidSize, IOSEEK_CUR) != 0) + { + if(pErrorMessage != nullptr) + str_copy(pErrorMessage, "Error rewinding SHA256 extension UUID", ErrorMessageSize); + mem_zero(pDemoHeader, sizeof(CDemoHeader)); + mem_zero(pTimelineMarkers, sizeof(CTimelineMarkers)); + io_close(File); + return false; + } } } - pMapInfo->m_Sha256 = Sha256; + str_copy(pMapInfo->m_aName, pDemoHeader->m_aMapName); + pMapInfo->m_Sha256 = Sha256; + pMapInfo->m_Crc = bytes_be_to_uint(pDemoHeader->m_aMapCrc); pMapInfo->m_Size = bytes_be_to_uint(pDemoHeader->m_aMapSize); - io_close(File); + if(pFile == nullptr) + io_close(File); + else + *pFile = File; + return true; } diff --git a/src/engine/shared/demo.h b/src/engine/shared/demo.h index 14b127d0e..4cb18304f 100644 --- a/src/engine/shared/demo.h +++ b/src/engine/shared/demo.h @@ -107,6 +107,7 @@ private: IOHANDLE m_File; long m_MapOffset; char m_aFilename[IO_MAX_PATH_LENGTH]; + char m_aErrorMessage[256]; std::vector m_vKeyFrames; CMapInfo m_MapInfo; int m_SpeedIndex; @@ -150,7 +151,7 @@ public: int Play(); void Pause() override; void Unpause() override; - int Stop(); + void Stop(const char *pErrorMessage = ""); void SetSpeed(float Speed) override; void SetSpeedIndex(int SpeedIndex) override; void AdjustSpeedIndex(int Offset) override; @@ -160,8 +161,9 @@ public: int SetPos(int WantedTick) override; const CInfo *BaseInfo() const override { return &m_Info.m_Info; } void GetDemoName(char *pBuffer, size_t BufferSize) const override; - bool GetDemoInfo(class IStorage *pStorage, const char *pFilename, int StorageType, CDemoHeader *pDemoHeader, CTimelineMarkers *pTimelineMarkers, CMapInfo *pMapInfo) const override; - const char *GetDemoFileName() { return m_aFilename; } + bool GetDemoInfo(class IStorage *pStorage, class IConsole *pConsole, const char *pFilename, int StorageType, CDemoHeader *pDemoHeader, CTimelineMarkers *pTimelineMarkers, CMapInfo *pMapInfo, IOHANDLE *pFile = nullptr, char *pErrorMessage = nullptr, size_t ErrorMessageSize = 0) const override; + const char *Filename() { return m_aFilename; } + const char *ErrorMessage() { return m_aErrorMessage; } int Update(bool RealTime = true); diff --git a/src/game/client/components/menus.cpp b/src/game/client/components/menus.cpp index 1d9aeb4bf..a227ea3fc 100644 --- a/src/game/client/components/menus.cpp +++ b/src/game/client/components/menus.cpp @@ -1839,7 +1839,7 @@ void CMenus::PopupConfirmDemoReplaceVideo() if(pError) { m_DemoRenderInput.Clear(); - PopupMessage(Localize("Error"), str_comp(pError, "error loading demo") ? pError : Localize("Error loading demo"), Localize("Ok")); + PopupMessage(Localize("Error loading demo"), pError, Localize("Ok")); } } #endif diff --git a/src/game/client/components/menus_demo.cpp b/src/game/client/components/menus_demo.cpp index e579af129..0e70cb3f9 100644 --- a/src/game/client/components/menus_demo.cpp +++ b/src/game/client/components/menus_demo.cpp @@ -1041,7 +1041,7 @@ bool CMenus::FetchHeader(CDemoItem &Item) { char aBuffer[IO_MAX_PATH_LENGTH]; str_format(aBuffer, sizeof(aBuffer), "%s/%s", m_aCurrentDemoFolder, Item.m_aFilename); - Item.m_Valid = DemoPlayer()->GetDemoInfo(Storage(), aBuffer, Item.m_StorageType, &Item.m_Info, &Item.m_TimelineMarkers, &Item.m_MapInfo); + Item.m_Valid = DemoPlayer()->GetDemoInfo(Storage(), nullptr, aBuffer, Item.m_StorageType, &Item.m_Info, &Item.m_TimelineMarkers, &Item.m_MapInfo); Item.m_InfosLoaded = true; } return Item.m_Valid; @@ -1505,7 +1505,9 @@ void CMenus::RenderDemoBrowserButtons(CUIRect ButtonsView, bool WasListboxItemAc m_LastPauseChange = -1.0f; m_LastSpeedChange = -1.0f; if(pError) - PopupMessage(Localize("Error"), str_comp(pError, "error loading demo") ? pError : Localize("Error loading demo"), Localize("Ok")); + { + PopupMessage(Localize("Error loading demo"), pError, Localize("Ok")); + } else { UI()->SetActiveItem(nullptr);