Louis Bernard
Louis Bernard

Reputation: 279

Downloading an executable (.exe) in C using WinHTTP

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!

Upvotes: 0

Views: 562

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 596256

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);

Upvotes: 1

tsc_chazz
tsc_chazz

Reputation: 206

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.

Upvotes: 2

Related Questions