Downloading an executable (.exe) in C using WinHTTP

Question

I'm trying to download an executable from a HTTP web server in C using WinHTTP. The code below works perfectly with HTML files, but when I try to download an executable (.exe) it does only download a part of the file (and the amount of downloaded bytes is different every time I run the program).

Code (inspired by this example):

...    
LPSTR retVal = (LPSTR) GlobalAlloc(GMEM_FIXED, 20000);
retVal[0] = 0;
LPSTR tempVal = 0;
DWORD dwSize = 0;
DWORD dwDownloaded = 0;
LPCWSTR accept[2] = { L"application/octet-stream", NULL };

HINTERNET hSession = WinHttpOpen(userAgent, WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, 0);

hConnect = WinHttpConnect(hSession, domain, INTERNET_DEFAULT_HTTP_PORT, 0);
hRequest = WinHttpOpenRequest(hConnect, L "GET", pathL, NULL, WINHTTP_NO_REFERER, accept, NULL);

BOOL work = WinHttpSendRequest(hRequest, WINHTTP_NO_ADDITIONAL_HEADERS, 0, WINHTTP_NO_REQUEST_DATA, 0, 0, 0);
work = WinHttpReceiveResponse(hRequest, NULL);

if (work) {
  do {
    dwSize = 0;
    WinHttpQueryDataAvailable(hRequest, & dwSize);
    tempVal = (LPSTR) GlobalAlloc(GMEM_FIXED, dwSize + 1);
    ZeroMemory(tempVal, dwSize + 1);
    WinHttpReadData(hRequest, (LPVOID) tempVal, dwSize, & dwDownloaded);
    StringCchCatA(retVal, 20000, tempVal);
    GlobalFree(tempVal);
  } while (dwSize > 0);
}

WinHttpCloseHandle(hRequest);
WinHttpCloseHandle(hConnect);
WinHttpCloseHandle(hSession);

return retVal;

What could be the reason for this and how could I try fix it? I appreciate every comment!

I want to know how much you know so the answer makes sense to you. So, do you know how strings work in C? — user253751, Dec 17 '20 at 21:11
My main focus lies on high level languages like Java or PHP, so my knowledge is limited. From my understanding, strings in C are char arrays terminated by a null value. — Louis Bernard, Dec 17 '20 at 21:14
Correct. HTML files don't usually have null values in them. But EXE files do. — user253751, Dec 17 '20 at 21:16
Well, I assume StringCchCatA will only concatente until the first null value is reached? As another user already pointed out, I will have to use memcpy instead of StringCchCatA, which does make sense to me. — Louis Bernard, Dec 17 '20 at 21:20

score 2 · Answer 1 · answered Dec 17 '20 at 20:57

Do not use StringCchCatA with binary strings like executable files. You'll want to use memcpy or something similar, with the destination pointing at the current end of the buffer you're building each iteration.

A C string uses a null value '\0', binary 0, to mark the end of the string. Executable files are loaded with these, at approximately random places throughout. So your executable will be concatenated up to the first 0 byte in the buffer, and then with the next buffer the 0 byte will be overwritten, and so on.

So you need to do your own pointer arithmetic to figure out where in your target buffer each buffer piece needs to go, and use a memory copy, not a string copy, to put it there.

Remy Lebeau · Accepted Answer · 2020-12-18T18:31:17.813

StringCchCatA() operates on null-terminated strings, but an EXE file is not textual data, it is binary data, and will have 0x00 bytes in it, which would cause StringCchCatA to truncate data.

If the file you are downloading is more than 20000 bytes, you are going to have to expand your buffer once you have filled it up to its max capacity. Unless you are downloading small files, you should generally use a fixed-sized buffer (the WinHttpReadData() documentation suggests 8KB), and just reuse and append that buffer to a temp file on each loop iteration. WinHttpReadData() tells you how many bytes are in the buffer after each read.

You are also not checking the return values of WinHttpQueryDataAvailable() or WinHttpReadData() for failure to break the download loop if needed.

Try something more like this instead:

...    
DWORD dwFileCap = 20000;
DWORD dwFileSize = 0;
DWORD dwReadSize = 8192;
DWORD dwAvailableSize = 0;
DWORD dwDownloaded = 0;

// TODO: create a file instead...
LPSTR fileData = (LPSTR) GlobalAlloc(GMEM_FIXED, dwFileCap);
if (!fileData)
{
    // error handling ...
    return NULL;
}

LPSTR readBuffer = (LPSTR) GlobalAlloc(GMEM_FIXED, dwReadSize);
if (!readBuffer)
{
    // error handling and cleanup ...
    return NULL;
}

LPCWSTR accept[2] = { L"application/octet-stream", NULL };

HINTERNET hSession = WinHttpOpen(userAgent, WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, 0);
if (!hSession)
{
    // error handling and cleanup ...
    return NULL;
}

hConnect = WinHttpConnect(hSession, domain, INTERNET_DEFAULT_HTTP_PORT, 0);
if (!hConnect)
{
    // error handling and cleanup ...
    return NULL;
}

hRequest = WinHttpOpenRequest(hConnect, L"GET", pathL, NULL, WINHTTP_NO_REFERER, accept, NULL);
if (!hRequest)
{
    // error handling and cleanup ...
    return NULL;
}

if (!WinHttpSendRequest(hRequest, WINHTTP_NO_ADDITIONAL_HEADERS, 0, WINHTTP_NO_REQUEST_DATA, 0, 0, 0))
{
    // error handling and cleanup ...
    return NULL;
}

if (!WinHttpReceiveResponse(hRequest, NULL))
{
    // error handling and cleanup ...
    return NULL;
}

bool done = false;

do
{
    dwAvailableSize = 0;
    if (!WinHttpQueryDataAvailable(hRequest, &dwAvailableSize))
    {
        // error handling and cleanup ...
        return NULL;
    }

    do
    {
        if (!WinHttpReadData(hRequest, readBuffer, min(dwAvailableSize, dwReadSize), &dwDownloaded))
        {
            // error handling and cleanup...
            return NULL;
        }

        if (dwDownloaded == 0)
        {
            done = true;
            break;
        }

        // TODO: if using a file instead, ignore this part ...
        if ((dwFileSize + dwDownloaded) > dwFileCap)
        {
            DWORD newCapacity = double(dwFileCap) * 1.5;
            LPSTR newFileData = (LPSTR) GlobalReAlloc((HGLOBAL)fileData, newCapacity, 0);
            if (!newFileData)
            {
                // error handling and cleanup ...
                return NULL;
            }
            fileData = newFileData;
            dwFileCap = newCapacity;
        }
        //

        // TODO: if using a file instead, write the bytes to the file here...
        memcpy(fileData + dwFileSize, readBuffer, dwDownloaded);

        dwFileSize += dwDownloaded;
        dwAvailableSize -= dwDownloaded;
    }
    while (dwAvailableSize > 0);
}
while (!done);

WinHttpCloseHandle(hRequest);
WinHttpCloseHandle(hConnect);
WinHttpCloseHandle(hSession);

GlobalFree((HGLOBAL)readBuffer);

// TODO: if using a file instead, close the file here...

// use file data up to dwFileSize bytes as needed ...

// TODO: if using a file instead, ignore this part ...
GlobalFree((HGLOBAL)fileData);

Thank you very much! I will study your changes later today. However, I have one question: what do you mean with "local file"? Writing the downloaded bytes to the new file in the same function? — Louis Bernard, Dec 18 '20 at 11:36
@smokeSH yes. Using `CreateFile()`+`WriteFile()`, or `fopen()`+`fwrite()`, instead of `Global(Re)Alloc()`+`memcpy()` — Remy Lebeau, Dec 18 '20 at 15:31
Thanks again. I implemented your changes in my program and it works fine. I only think the `break` needed to be at another location because otherwise the `while(true)` loop would continue forever. — Louis Bernard, Dec 18 '20 at 18:23

Downloading an executable (.exe) in C using WinHTTP

2 Answers2