WinHTTP stops downloading after 8 KiB

Question

I'm downloading .jar files from libraries.minecraft.net and repo1.maven.org using WinHTTP (over HTTPS). Here's the function:

// Namespace alias, all stdfs mentions in the function refer to std::filesystem
namespace stdfs = std::filesystem;

bool downloadFile(DLElement element) {
    HINTERNET session = WinHttpOpen(
        // Identify as Firefox
        L"Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:10.0) Gecko/20100101 Firefox/10.0",
        // Don't care about proxies
        WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, 0);
    if (session == nullptr) return false;
    HINTERNET connection;

    connection = WinHttpConnect(session, element.hostname.c_str(),
        element.https ? INTERNET_DEFAULT_HTTPS_PORT : INTERNET_DEFAULT_HTTP_PORT, 0);

    // Accept `.jar` files
    const wchar_t** acceptTypes = new const wchar_t* [2];
    acceptTypes[0] = L"application/java-archive";
    acceptTypes[1] = nullptr;
    HINTERNET request = WinHttpOpenRequest(connection, L"GET", element.object.c_str(),
        nullptr, WINHTTP_NO_REFERER, acceptTypes, element.https ? WINHTTP_FLAG_SECURE : 0);

    bool result = WinHttpSendRequest(request, WINHTTP_NO_ADDITIONAL_HEADERS, 0, WINHTTP_NO_REQUEST_DATA, 0, 0, 0);
    if (!result) {
        DWORD hr = GetLastError();
        checkHresult(hr, window, false);
    }
    if (!result) {
    failRet:
        WinHttpCloseHandle(request);
        WinHttpCloseHandle(connection);
        WinHttpCloseHandle(session);
        return false;
    }
    result = WinHttpReceiveResponse(request, nullptr);
    DWORD size = 0;
    DWORD downloaded = 0;
    std::vector<uint8_t> file;
    if (!result) goto failRet;
    do {
        if (!WinHttpQueryDataAvailable(request, &size)) {
            goto failRet;
        }
        uint8_t* buffer;
    alloc:
        try {
            buffer = new uint8_t[size];
        }
        catch (std::bad_alloc&) {
            MessageBox(window, lstr(VCLS_OUT_OF_MEMORY_DESC), lstr(VCLS_OUT_OF_MEMORY_TITLE), MB_ICONERROR);
            goto alloc;
        }
        // Dunno why, do I even need this memset?
        memset(buffer, 0, size);
        if (!WinHttpReadData(request, buffer, size, &downloaded)) {
            delete[] buffer;
            checkHresult(GetLastError(), window, false);
            goto failRet;
        }
        size_t vectSize = file.size();
        file.reserve(vectSize + size);
        for (size_t i = vectSize; i < size; i++) {
            // Might as well rewrite this and make this more efficient
            file.push_back(buffer[i]);
        }
        delete[] buffer;

    } while (size > 0);
    WinHttpCloseHandle(request);
    WinHttpCloseHandle(connection);
    WinHttpCloseHandle(session);

    element.path.make_preferred();
    stdfs::path dir = stdfs::path(element.path).remove_filename();
    std::wstring fdirStr = dir.wstring();
    fdirStr.pop_back();
    dir = fdirStr;

tryCreateDir:
    try {
        stdfs::create_directories(mcFolderPath/dir);
    }
    catch (const std::bad_alloc&) {
        MessageBox(window, lstr(VCLS_OUT_OF_MEMORY_DESC), lstr(VCLS_OUT_OF_MEMORY_TITLE), MB_ICONERROR);
        goto tryCreateDir;
    }
    catch (const stdfs::filesystem_error& e) {
        // Error handling removed for brevity

        return false;
    }

    std::basic_ofstream<uint8_t> ofs(mcFolderPath/element.path, std::ios::binary | std::ios::trunc);
    ofs.write(file.data(), file.size());
    ofs.close();

    return true;
}

DLElement is defined as follows:

struct DLElement {
    std::wstring hostname; std::wstring object; stdfs::path path; std::string sha1hex;
    bool hasSha = true; bool https = false;
};

The problem is, for some reason this function only downloads exactly 8 KiB of the actual file, which happens to be the buffer size for WinHTTP. This code is a modified Microsoft WinHTTP example, so I assume its correctness and ability to fetch more than 8 KiB of data. What am I doing wrong and why does WinHttpQueryDataAvailable return 0 to size once it hits 8 KiB?

---

OS: Windows 10 Professional, update 1903
Architecture: x86-64 CPU, x86-64 OS
CPU: Intel Core i5-2300 @ 2.8 GHz
RAM: 8 GiB
Swapfile: 16000 MB

Answer 1

Barmak Shemirani's answer solved the problem for me, though there's one more way to solve this issue, while still using the buffer. (The answer that I mentioned is still faster and more elegant though.) The below single line replaces the for loop:

file.insert(file.end(), &buffer[0], &buffer[size]);

It's "the C++ way" to append an iterable to a std::vector. Still, writing to the vector in the first place is obviously faster. If you can specify where to write, that is.

Answer 2

buffer = new uint8_t[size];
size_t vectSize = file.size();
file.reserve(vectSize + size);
...
for (size_t i = vectSize; i < size; i++) {
    // Might as well rewrite this and make this more efficient
    file.push_back(buffer[i]);
}

This code is supposed to append buffer on to file, so the for loop should start at zero, not vectSize. Change it to:

for (size_t i = 0; i < size; i++) 
    file.push_back(buffer[i]);

It appears you are using C++11 or newer, so you can use std::vector instead of new. Consider replacing your do loop with the following:

while(true)
{
    if(!WinHttpQueryDataAvailable(request, &size))
        break;
    if(!size) 
        break;
    std::vector<uint8_t> buffer(size); 
    if(!WinHttpReadData(request, buffer.data(), size, &downloaded))
        break;
    if(!downloaded)
        break;
    buffer.resize(downloaded);
    file.insert(file.end(), buffer.begin(), buffer.end());
}

Or just write directly to the main buffer:

while(true)
{
    if(!WinHttpQueryDataAvailable(request, &size))
        break;
    if(!size)
        break;

    size_t current_size = file.size();
    file.resize(current_size + size);
    downloaded = 0;
    result = WinHttpReadData(request, file.data() + current_size, size, &downloaded);
    file.resize(current_size + downloaded);

    if(!result || !downloaded)
        break;
}

Unrelated to the bug:

Initializing the buffer to zero is not needed. It is done in the Microsoft sample, but that sample is using a null-terminated string, it only needs to set the last byte to zero.

Consider using WinINet instead of WinHTTP. WinINet is easier, if you don't need a server.