Merge pull request ddnet#9574 from heinrich5991/pr_ddnet_http_easier_file_download

Make it easier to download a file and have `If-Modified-Since` support

Author: Robyt3

src/engine/client/client.cpp

@@ -2393,51 +2393,6 @@ void CClient::ResetDDNetInfoTask()
 	}
 }
 
-void CClient::FinishDDNetInfo()
-{
-	if(m_ServerBrowser.DDNetInfoSha256() == m_pDDNetInfoTask->ResultSha256())
-	{
-		log_debug("client/info", "DDNet info already up-to-date");
-		return;
-	}
-
-	char aTempFilename[IO_MAX_PATH_LENGTH];
-	IStorage::FormatTmpPath(aTempFilename, sizeof(aTempFilename), DDNET_INFO_FILE);
-	IOHANDLE File = Storage()->OpenFile(aTempFilename, IOFLAG_WRITE, IStorage::TYPE_SAVE);
-	if(!File)
-	{
-		log_error("client/info", "Failed to open temporary DDNet info '%s' for writing", aTempFilename);
-		return;
-	}
-
-	unsigned char *pResult;
-	size_t ResultLength;
-	m_pDDNetInfoTask->Result(&pResult, &ResultLength);
-	bool Error = io_write(File, pResult, ResultLength) != ResultLength;
-	Error |= io_close(File) != 0;
-	if(Error)
-	{
-		log_error("client/info", "Error writing temporary DDNet info to file '%s'", aTempFilename);
-		return;
-	}
-
-	if(Storage()->FileExists(DDNET_INFO_FILE, IStorage::TYPE_SAVE) && !Storage()->RemoveFile(DDNET_INFO_FILE, IStorage::TYPE_SAVE))
-	{
-		log_error("client/info", "Failed to remove old DDNet info '%s'", DDNET_INFO_FILE);
-		Storage()->RemoveFile(aTempFilename, IStorage::TYPE_SAVE);
-		return;
-	}
-	if(!Storage()->RenameFile(aTempFilename, DDNET_INFO_FILE, IStorage::TYPE_SAVE))
-	{
-		log_error("client/info", "Failed to rename temporary DDNet info '%s' to '%s'", aTempFilename, DDNET_INFO_FILE);
-		Storage()->RemoveFile(aTempFilename, IStorage::TYPE_SAVE);
-		return;
-	}
-
-	log_debug("client/info", "Loading new DDNet info");
-	LoadDDNetInfo();
-}
-
 typedef std::tuple<int, int, int> TVersion;
 static const TVersion gs_InvalidVersion = std::make_tuple(-1, -1, -1);
 
@@ -2927,7 +2882,16 @@ void CClient::Update()
 	{
 		if(m_pDDNetInfoTask->State() == EHttpState::DONE)
 		{
-			FinishDDNetInfo();
+			if(m_ServerBrowser.DDNetInfoSha256() == m_pDDNetInfoTask->ResultSha256())
+			{
+				log_debug("client/info", "DDNet info already up-to-date");
+			}
+			else
+			{
+				log_debug("client/info", "Loading new DDNet info");
+				LoadDDNetInfo();
+			}
+
 			ResetDDNetInfoTask();
 		}
 		else if(m_pDDNetInfoTask->State() == EHttpState::ERROR || m_pDDNetInfoTask->State() == EHttpState::ABORTED)
@@ -5084,9 +5048,10 @@ void CClient::RequestDDNetInfo()
 		str_append(aUrl, aEscaped);
 	}
 
-	// Use ipv4 so we can know the ingame ip addresses of players before they join game servers
-	m_pDDNetInfoTask = HttpGet(aUrl);
+	m_pDDNetInfoTask = HttpGetFile(aUrl, Storage(), DDNET_INFO_FILE, IStorage::TYPE_SAVE);
 	m_pDDNetInfoTask->Timeout(CTimeout{10000, 0, 500, 10});
+	m_pDDNetInfoTask->SkipByFileTime(false); // Always re-download.
+	// Use ipv4 so we can know the ingame ip addresses of players before they join game servers
 	m_pDDNetInfoTask->IpResolve(IPRESOLVE::V4);
 	Http()->Run(m_pDDNetInfoTask);
 }

src/engine/client/client.h

@@ -377,7 +377,6 @@ class CClient : public IClient, public CDemoPlayer::IListener
 
 	void RequestDDNetInfo() override;
 	void ResetDDNetInfoTask();
-	void FinishDDNetInfo();
 	void LoadDDNetInfo();
 
 	bool IsSixup() const override { return m_Sixup; }

src/engine/shared/http.cpp

@@ -75,19 +75,68 @@ CHttpRequest::~CHttpRequest()
 	free(m_pBuffer);
 	curl_slist_free_all((curl_slist *)m_pHeaders);
 	free(m_pBody);
+	if(m_State == EHttpState::DONE && m_ValidateBeforeOverwrite)
+	{
+		OnValidation(false);
+	}
+}
+
+static bool CalculateSha256(const char *pAbsoluteFilename, SHA256_DIGEST *pSha256)
+{
+	IOHANDLE File = io_open(pAbsoluteFilename, IOFLAG_READ);
+	if(!File)
+	{
+		return false;
+	}
+	SHA256_CTX Sha256Ctxt;
+	sha256_init(&Sha256Ctxt);
+	unsigned char aBuffer[64 * 1024];
+	while(true)
+	{
+		unsigned Bytes = io_read(File, aBuffer, sizeof(aBuffer));
+		if(Bytes == 0)
+			break;
+		sha256_update(&Sha256Ctxt, aBuffer, Bytes);
+	}
+	io_close(File);
+	*pSha256 = sha256_finish(&Sha256Ctxt);
+	return true;
+}
+
+bool CHttpRequest::ShouldSkipRequest()
+{
+	if(m_WriteToFile && m_ExpectedSha256 != SHA256_ZEROED)
+	{
+		SHA256_DIGEST Sha256;
+		if(CalculateSha256(m_aDestAbsolute, &Sha256) && Sha256 == m_ExpectedSha256)
+		{
+			log_debug("http", "skipping download because expected file already exists: %s", m_aDest);
+			return true;
+		}
+	}
+	return false;
 }
 
 bool CHttpRequest::BeforeInit()
 {
 	if(m_WriteToFile)
 	{
-		if(fs_makedir_rec_for(m_aDestAbsolute) < 0)
+		if(m_SkipByFileTime)
+		{
+			time_t FileCreatedTime, FileModifiedTime;
+			if(fs_file_time(m_aDestAbsolute, &FileCreatedTime, &FileModifiedTime) == 0)
+			{
+				m_IfModifiedSince = FileModifiedTime;
+			}
+		}
+
+		if(fs_makedir_rec_for(m_aDestAbsoluteTmp) < 0)
 		{
 			log_error("http", "i/o error, cannot create folder for: %s", m_aDest);
 			return false;
 		}
 
-		m_File = io_open(m_aDestAbsolute, IOFLAG_WRITE);
+		m_File = io_open(m_aDestAbsoluteTmp, IOFLAG_WRITE);
 		if(!m_File)
 		{
 			log_error("http", "i/o error, cannot open file: %s", m_aDest);
@@ -268,14 +317,17 @@ size_t CHttpRequest::OnData(char *pData, size_t DataSize)
 		return 0;
 	}
 
+	if(DataSize == 0)
+	{
+		return DataSize;
+	}
+
 	sha256_update(&m_ActualSha256Ctx, pData, DataSize);
 
-	if(!m_WriteToFile)
+	size_t Result = DataSize;
+
+	if(m_WriteToMemory)
 	{
-		if(DataSize == 0)
-		{
-			return DataSize;
-		}
 		size_t NewBufferSize = maximum((size_t)1024, m_BufferSize);
 		while(m_ResponseLength + DataSize > NewBufferSize)
 		{
@@ -287,14 +339,13 @@ size_t CHttpRequest::OnData(char *pData, size_t DataSize)
 			m_BufferSize = NewBufferSize;
 		}
 		mem_copy(m_pBuffer + m_ResponseLength, pData, DataSize);
-		m_ResponseLength += DataSize;
-		return DataSize;
 	}
-	else
+	if(m_WriteToFile)
 	{
-		m_ResponseLength += DataSize;
-		return io_write(m_File, pData, DataSize);
+		Result = io_write(m_File, pData, DataSize);
 	}
+	m_ResponseLength += DataSize;
+	return Result;
 }
 
 size_t CHttpRequest::HeaderCallback(char *pData, size_t Size, size_t Number, void *pUser)
@@ -375,7 +426,44 @@ void CHttpRequest::OnCompletionInternal(void *pHandle, unsigned int Result)
 
 		if(State == EHttpState::ERROR || State == EHttpState::ABORTED)
 		{
-			fs_remove(m_aDestAbsolute);
+			fs_remove(m_aDestAbsoluteTmp);
+		}
+		else if(m_IfModifiedSince >= 0 && m_StatusCode == 304) // 304 Not Modified
+		{
+			fs_remove(m_aDestAbsoluteTmp);
+			if(m_WriteToMemory)
+			{
+				free(m_pBuffer);
+				m_pBuffer = nullptr;
+				m_ResponseLength = 0;
+				void *pBuffer;
+				unsigned Length;
+				IOHANDLE File = io_open(m_aDestAbsolute, IOFLAG_READ);
+				bool Success = File && io_read_all(File, &pBuffer, &Length);
+				if(File)
+				{
+					io_close(File);
+				}
+				if(Success)
+				{
+					m_pBuffer = (unsigned char *)pBuffer;
+					m_ResponseLength = Length;
+				}
+				else
+				{
+					log_error("http", "i/o error, cannot read existing file: %s", m_aDest);
+					State = EHttpState::ERROR;
+				}
+			}
+		}
+		else if(!m_ValidateBeforeOverwrite)
+		{
+			if(fs_rename(m_aDestAbsoluteTmp, m_aDestAbsolute))
+			{
+				log_error("http", "i/o error, cannot move file: %s", m_aDest);
+				State = EHttpState::ERROR;
+				fs_remove(m_aDestAbsoluteTmp);
+			}
 		}
 	}
 
@@ -390,10 +478,37 @@ void CHttpRequest::OnCompletionInternal(void *pHandle, unsigned int Result)
 	m_WaitCondition.notify_all();
 }
 
+void CHttpRequest::OnValidation(bool Success)
+{
+	dbg_assert(m_ValidateBeforeOverwrite, "this function is illegal to call without having set ValidateBeforeOverwrite");
+	m_ValidateBeforeOverwrite = false;
+	if(Success)
+	{
+		if(m_IfModifiedSince >= 0 && m_StatusCode == 304) // 304 Not Modified
+		{
+			fs_remove(m_aDestAbsoluteTmp);
+			return;
+		}
+		if(fs_rename(m_aDestAbsoluteTmp, m_aDestAbsolute))
+		{
+			log_error("http", "i/o error, cannot move file: %s", m_aDest);
+			m_State = EHttpState::ERROR;
+			fs_remove(m_aDestAbsoluteTmp);
+		}
+	}
+	else
+	{
+		m_State = EHttpState::ERROR;
+		fs_remove(m_aDestAbsoluteTmp);
+	}
+}
+
 void CHttpRequest::WriteToFile(IStorage *pStorage, const char *pDest, int StorageType)
 {
+	m_WriteToMemory = false;
 	m_WriteToFile = true;
 	str_copy(m_aDest, pDest);
+	m_StorageType = StorageType;
 	if(StorageType == -2)
 	{
 		pStorage->GetBinaryPath(m_aDest, m_aDestAbsolute, sizeof(m_aDestAbsolute));
@@ -402,6 +517,13 @@ void CHttpRequest::WriteToFile(IStorage *pStorage, const char *pDest, int Storag
 	{
 		pStorage->GetCompletePath(StorageType, m_aDest, m_aDestAbsolute, sizeof(m_aDestAbsolute));
 	}
+	IStorage::FormatTmpPath(m_aDestAbsoluteTmp, sizeof(m_aDestAbsoluteTmp), m_aDestAbsolute);
+}
+
+void CHttpRequest::WriteToFileAndMemory(IStorage *pStorage, const char *pDest, int StorageType)
+{
+	WriteToFile(pStorage, pDest, StorageType);
+	m_WriteToMemory = true;
 }
 
 void CHttpRequest::Header(const char *pNameColonValue)
@@ -421,7 +543,7 @@ void CHttpRequest::Wait()
 void CHttpRequest::Result(unsigned char **ppResult, size_t *pResultLength) const
 {
 	dbg_assert(State() == EHttpState::DONE, "Request not done");
-	dbg_assert(!m_WriteToFile, "Result not usable together with WriteToFile");
+	dbg_assert(m_WriteToMemory, "Result only usable when written to memory");
 	*ppResult = m_pBuffer;
 	*pResultLength = m_ResponseLength;
 }
@@ -584,6 +706,18 @@ void CHttp::RunLoop()
 			if(g_Config.m_DbgCurl)
 				log_debug("http", "task: %s %s", CHttpRequest::GetRequestType(pRequest->m_Type), pRequest->m_aUrl);
 
+			if(pRequest->ShouldSkipRequest())
+			{
+				pRequest->OnCompletion(EHttpState::DONE);
+				{
+					std::unique_lock WaitLock(pRequest->m_WaitMutex);
+					pRequest->m_State = EHttpState::DONE;
+				}
+				pRequest->m_WaitCondition.notify_all();
+				NewRequests.pop_front();
+				continue;
+			}
+
 			CURL *pEH = curl_easy_init();
 			if(!pEH)
 			{