Fix HTTP file mismatch (CRC:00000000) when a resource file is re-downloaded - #5150
Open
QueryOfficial wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Client-side file reads could return an empty buffer while reporting success, which made a
freshly downloaded resource file checksum to
CRC:00000000 MD5:D41D8CD98F00B204E9800998ECF8427E— the checksum of zero bytes. Three changes:
CDownloadableResource::GenerateClientChecksum()now retries briefly (4 × 20 ms) when afile reads as empty and the server-reported download size is non-zero. A file that does not
exist yet checksums to
CChecksum()instead, so files still awaiting download never enter thispath and pay no cost.
SharedUtil::FileLoadWithTimeout()andSharedUtil::FileLoad(std::nothrow, …)took thenumber of bytes to read from
GetFileAttributesExWand returnedtruewith an empty buffer whenthat reported 0. Sizing a read from the directory entry samples the length and the bytes at two
different moments. Both now open the handle first and size the read with
GetFileSizeEx, and ashort read is never reported as success.
FileLoadWithTimeoutmoves the whole open/measure/readsequence onto the single worker thread that already provided the hang timeout — this keeps the
protection from 7bc2315 / 7e69069 end-to-end and drops thread spawns per call from 3 to 2.
FileLoad(std::nothrow, …)is whatCClientDFF/CClientTXD/CClientColModeluse, so the samedefect could load a model from an empty buffer even when the checksum passed.
The POSIX branch switches from
stat64(path)tofstat64(fd)for the same reason.CChecksum's Windows client cache is now validated and populated with size/mtime taken fromthe open handle, never caches a zero-byte result, and gains
InvalidateChecksumCacheEntry(path)— called wherever a file is deleted or replaced(
CPacketHandler::Packet_ResourceStart,CResource::VerifyPendingClientChecksums,CResourceFileDownloadManager::DownloadFinished,CSingularFileDownload).CResource::Load()also re-reads once before reporting a mismatch, so a lost race becomes a silentretry rather than a player-visible error.
GetFileAttributesExWithTimeout()had no callers left after this and was removed, along with theWIN32_FILE_ATTRIBUTE_DATAforward declaration that existed only for it.Motivation
Resolves #5120.
Restarting a resource that contains
.dff/.txd/.hf/.mp3files producedHTTP server file mismatch!withGot CRC:00000000on every restart, and the model was notreplaced. A second restart of the same resource worked.
That MD5 is the MD5 of an empty input, so the client was hashing zero bytes for a file that had
downloaded correctly. Client scripts were unaffected because
CResource::Load()reads those throughFileLoad()(stdio, length from the open stream); only the auto-download files went throughGenerateChecksumFromFileUnsafe()→FileLoadWithTimeout().Test plan
New unit tests —
Tests/client/SharedUtilFile_Tests.cpp, 11 cases covering both loaders: wholefile, genuinely empty file, missing file, offset/maxSize, repeated delete + rewrite, a writer handle
still open, and
GetFileInfoWithTimeout. Full suite: 315/315 pass(
Bin/tests/Tests_Client_d.exe).Measured before/after with a standalone harness running the pre-fix and post-fix
implementations side by side against a writer thread doing delete → create → write → close, counting
calls that returned success with a buffer that was not the complete payload:
A and D show the read is fine once the file is settled; B and C are the failure mode, and the retry
closes it completely.
Checklist