From 2be54a30ad311c22b38642c3477e6fa934f08578 Mon Sep 17 00:00:00 2001
From: def <dennis@felsin9.de>
Date: Thu, 24 Sep 2020 09:52:11 +0200
Subject: [PATCH] HTTP: Set final m_State after running completion function

Ensures that we don't delete a file that is being used by ingame map
downloader already.

As reported by Jupeyy in #2901
---
 src/engine/client/http.cpp    | 38 +++++++++++++++++------------------
 src/engine/client/http.h      | 12 ++++++-----
 src/engine/client/updater.cpp | 20 +++++++++++-------
 3 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/src/engine/client/http.cpp b/src/engine/client/http.cpp
index 56f501394..2418ecdd7 100644
--- a/src/engine/client/http.cpp
+++ b/src/engine/client/http.cpp
@@ -88,18 +88,19 @@ CRequest::CRequest(const char *pUrl, CTimeout Timeout, bool LogProgress)
 
 void CRequest::Run()
 {
+	int FinalState;
 	if(!BeforeInit())
 	{
-		m_State = HTTP_ERROR;
-		OnCompletion();
-		return;
+		FinalState = HTTP_ERROR;
+	}
+	else
+	{
+		CURL *pHandle = curl_easy_init();
+		FinalState = RunImpl(pHandle);
+		curl_easy_cleanup(pHandle);
 	}
 
-	CURL *pHandle = curl_easy_init();
-	m_State = RunImpl(pHandle);
-	curl_easy_cleanup(pHandle);
-
-	OnCompletion();
+	m_State = OnCompletion(FinalState);
 }
 
 int CRequest::RunImpl(CURL *pHandle)
@@ -144,10 +145,6 @@ int CRequest::RunImpl(CURL *pHandle)
 		dbg_msg("http", "http %s", m_aUrl);
 	m_State = HTTP_RUNNING;
 	int Ret = curl_easy_perform(pHandle);
-	if(!BeforeCompletion())
-	{
-		return HTTP_ERROR;
-	}
 	if(Ret != CURLE_OK)
 	{
 		if(g_Config.m_DbgCurl || m_LogProgress)
@@ -253,7 +250,8 @@ size_t CGet::OnData(char *pData, size_t DataSize)
 CGetFile::CGetFile(IStorage *pStorage, const char *pUrl, const char *pDest, int StorageType, CTimeout Timeout, bool LogProgress)
 	: CRequest(pUrl, Timeout, LogProgress),
 	  m_pStorage(pStorage),
-	  m_StorageType(StorageType)
+	  m_StorageType(StorageType),
+	  m_File(0)
 {
 	str_copy(m_aDest, pDest, sizeof(m_aDest));
 
@@ -285,17 +283,19 @@ size_t CGetFile::OnData(char *pData, size_t DataSize)
 	return io_write(m_File, pData, DataSize);
 }
 
-bool CGetFile::BeforeCompletion()
+int CGetFile::OnCompletion(int State)
 {
-	return io_close(m_File) == 0;
-}
+	if(m_File && io_close(m_File) != 0)
+	{
+		State = HTTP_ERROR;
+	}
 
-void CGetFile::OnCompletion()
-{
-	if(State() == HTTP_ERROR || State() == HTTP_ABORTED)
+	if(State == HTTP_ERROR || State == HTTP_ABORTED)
 	{
 		m_pStorage->RemoveFile(m_aDestFull, IStorage::TYPE_ABSOLUTE);
 	}
+
+	return State;
 }
 
 CPostJson::CPostJson(const char *pUrl, CTimeout Timeout, const char *pJson)
diff --git a/src/engine/client/http.h b/src/engine/client/http.h
index 005aa4a65..20345b5a7 100644
--- a/src/engine/client/http.h
+++ b/src/engine/client/http.h
@@ -33,9 +33,7 @@ class CRequest : public IJob
 	virtual bool AfterInit(void *pCurl) { return true; }
 	virtual size_t OnData(char *pData, size_t DataSize) = 0;
 
-	virtual void OnProgress() { }
-	virtual bool BeforeCompletion() { return true; }
-	virtual void OnCompletion() { }
+	virtual void OnProgress() {}
 
 	char m_aUrl[256];
 
@@ -55,6 +53,9 @@ class CRequest : public IJob
 	void Run();
 	int RunImpl(CURL *pHandle);
 
+protected:
+	virtual int OnCompletion(int State) { return State; }
+
 public:
 	CRequest(const char *pUrl, CTimeout Timeout, bool LogProgress = true);
 
@@ -87,8 +88,6 @@ class CGetFile : public CRequest
 {
 	virtual size_t OnData(char *pData, size_t DataSize);
 	virtual bool BeforeInit();
-	virtual bool BeforeCompletion();
-	virtual void OnCompletion();
 
 	IStorage *m_pStorage;
 
@@ -97,6 +96,9 @@ class CGetFile : public CRequest
 	int m_StorageType;
 	IOHANDLE m_File;
 
+protected:
+	virtual int OnCompletion(int State);
+
 public:
 	CGetFile(IStorage *pStorage, const char *pUrl, const char *pDest, int StorageType = -2, CTimeout Timeout = CTimeout{4000, 500, 5}, bool LogProgress = true);
 
diff --git a/src/engine/client/updater.cpp b/src/engine/client/updater.cpp
index 72a346b97..625fca9d3 100644
--- a/src/engine/client/updater.cpp
+++ b/src/engine/client/updater.cpp
@@ -18,8 +18,10 @@ class CUpdaterFetchTask : public CGetFile
 	char m_aBuf2[256];
 	CUpdater *m_pUpdater;
 
-	void OnCompletion();
-	void OnProgress();
+	virtual void OnProgress();
+
+protected:
+	virtual int OnCompletion(int State);
 
 public:
 	CUpdaterFetchTask(CUpdater *pUpdater, const char *pFile, const char *pDestPath);
@@ -55,8 +57,10 @@ void CUpdaterFetchTask::OnProgress()
 	lock_unlock(m_pUpdater->m_Lock);
 }
 
-void CUpdaterFetchTask::OnCompletion()
+int CUpdaterFetchTask::OnCompletion(int State)
 {
+	State = CGetFile::OnCompletion(State);
+
 	const char *b = 0;
 	for(const char *a = Dest(); *a; a++)
 		if(*a == '/')
@@ -64,18 +68,20 @@ void CUpdaterFetchTask::OnCompletion()
 	b = b ? b : Dest();
 	if(!str_comp(b, "update.json"))
 	{
-		if(State() == HTTP_DONE)
+		if(State == HTTP_DONE)
 			m_pUpdater->SetCurrentState(IUpdater::GOT_MANIFEST);
-		else if(State() == HTTP_ERROR)
+		else if(State == HTTP_ERROR)
 			m_pUpdater->SetCurrentState(IUpdater::FAIL);
 	}
 	else if(!str_comp(b, m_pUpdater->m_aLastFile))
 	{
-		if(State() == HTTP_DONE)
+		if(State == HTTP_DONE)
 			m_pUpdater->SetCurrentState(IUpdater::MOVE_FILES);
-		else if(State() == HTTP_ERROR)
+		else if(State == HTTP_ERROR)
 			m_pUpdater->SetCurrentState(IUpdater::FAIL);
 	}
+
+	return State;
 }
 
 CUpdater::CUpdater()