I'm super new to C and I'm trying to download an executable file from a URL and write it to disk.

I have the below code which successfully downloads the file and stores it in memory. The issue I am having is then writing that to disk.

I am pretty sure this is down to where I am creating the buffer where the downloaded data will be written to before being going to the new file.

char buffer[4096];

The 4096 is an arbitrary number from the template code I got elsewhere. What I can't figure out or don't know is how to dynamically allocate that buffer size based on the size of the data at &pStream.

I have tried using functions such as sizeof() but these just get me the memory address size rather than the value itself.

Alternatively, is there better way to try and accomplish this download and write?

#include <Windows.h>
#include <Urlmon.h>   // URLOpenBlockingStreamW()
#include <atlbase.h>  // CComPtr
#include <iostream>
#include "download.h"
#include <fstream>
#include <assert.h>
#include <chrono>
#include <thread>
#pragma comment( lib, "Urlmon.lib" )

struct ComInit
{
    HRESULT hr;
    ComInit() : hr(::CoInitialize(nullptr)) {}
    ~ComInit() { if (SUCCEEDED(hr)) ::CoUninitialize(); }
};

int download_file()
{
    ComInit init;
    HRESULT hr;

    // use CComPtr so you don't have to manually call Release()
    CComPtr<IStream> pStream;
    bool success = false;

    while (success == false)
    {
        try {
            // Open the HTTP request.
            hr = URLOpenBlockingStreamW(nullptr, L"https://www.foo.bar/download/somefile.exe", &pStream, 0, nullptr);
            if (FAILED(hr))
            {
                std::cout << "ERROR: Could not connect. HRESULT: 0x" << std::hex << hr << std::dec << "\n";
            }
            else
            {
                success = true;
            }
        }
        catch (const std::exception& ex) {
            std::cout << ex.what();
        }
    }


    // Download the response and write it to stdout.
    char buffer[4096]; // Issue is here I think
    do
    {
        DWORD bytesRead = 0;
        hr = pStream->Read(buffer, sizeof(buffer), &bytesRead);

        if (bytesRead > 0)
        {
            //std::cout.write(buffer, bytesRead);
            std::ofstream file;
            file.open("some_path_dot_exe", std::ios_base::binary);
            assert(file.is_open());
            for (int i = 0; i < sizeof(buffer) / sizeof(buffer[0]);   i)
                file.write((char*)(buffer   i * sizeof(buffer[0])), sizeof(buffer[0]));
            file.close();
        }
    } while (SUCCEEDED(hr) && hr != S_FALSE);

    if (FAILED(hr))
    {
        std::cout << "ERROR: Download failed. HRESULT: 0x" << std::hex << hr << std::dec << "\n";
        return 2;
    }

    std::cout << "\n";

    return 0;
}

CodePudding user response：

The IStream that URLOpenBlockingStreamW() gives you isn't guaranteed to be able to give you the full file size up front, so if you want to hold the entire file in memory, you will have to use std::vector or other dynamically-growing buffer.

Though, you don't actually need to hold the entire file in memory just to save it to disk, you can use a fixed array and write it to disk as it is being downloaded, as you already are doing.

The real problem is, you are opening and closing the file on every Read(), wiping out all previous data written. And you are ignoring the bytesRead value that Read() gives you.

You need to open the file one time, leave it open until you are done with the download, and don't write more than is actually in the buffer on each write().

Try this:

#include <Windows.h>
#include <Urlmon.h>   // URLOpenBlockingStreamW()
#include <atlbase.h>  // CComPtr
#include <iostream>
#include "download.h"
#include <fstream>
#include <assert.h>
#include <chrono>
#include <thread>
#pragma comment( lib, "Urlmon.lib" )

struct ComInit
{
    HRESULT hr;
    ComInit() : hr(::CoInitialize(nullptr)) {}
    ~ComInit() { if (SUCCEEDED(hr)) ::CoUninitialize(); }
};

int download_file()
{
    ComInit init;
    HRESULT hr;

    // use CComPtr so you don't have to manually call Release()
    CComPtr<IStream> pStream;

    do
    {
        try {
            // Open the HTTP request.
            hr = URLOpenBlockingStreamW(nullptr, L"https://www.foo.bar/download/somefile.exe", &pStream, 0, nullptr);
            if (SUCCEEDED(hr)) break;

            std::cout << "ERROR: Could not connect. HRESULT: 0x" << std::hex << hr << std::dec << "\n";
        }
        catch (const std::exception& ex) {
            std::cout << ex.what();
        }
    }
    while (true);

    std::ofstream file("some_path_dot_exe", std::ios_base::binary);
    if (!file.is_open()) {
        std::cout << "ERROR: Download failed. Unable to create output file.\n";
        return 1;
    }

    // Download the response and write it to file.
    char buffer[4096];
    DWORD bytesRead;

    do
    {
        hr = pStream->Read(buffer, sizeof(buffer), &bytesRead);
        if (bytesRead > 0)
            file.write(buffer, bytesRead);

    } while (SUCCEEDED(hr) && hr != S_FALSE);

    file.close();

    if (FAILED(hr))
    {
        std::cout << "ERROR: Download failed. HRESULT: 0x" << std::hex << hr << std::dec << "\n";
        return 2;
    }

    std::cout << "\n";

    return 0;
}