Date of latest GitHub project release

Question

I needed a clean, simple and robust piece of code to retrieve the date of publication of the latest release of a given GitHub Project.

My constraints were that it needed to be written in the C++11 subset supported by Visual Studio 2012, and that it would require as few dependencies as possible.

Here is what I came up with. The code uses the Windows Internet API to query the GitHub REST API, and RapidJSON to parse the JSON response.

Here are a few specific questions:

• Am I using the Windows Internet API correctly?
• Will my code work in the presence of proxies or other exotic settings?
• Is there a simpler way to parse the ISO date returned by GitHub and pretty-print it?
• Is my usage of TCHAR and _T() correct? Or should I use wchar_t and L prefixes?

I would appreciate any other comment or suggestion on how to improve (or fix) this code.

#include "rapidjson/document.h"

#include <Windows.h>
#include <wininet.h>
#include <tchar.h>

#include <cassert>
#include <ctime>
#include <iostream>
#include <string>

DWORD query_release_information(std::string& response)
{
    response.clear();

    HINTERNET session =
        InternetOpen(
            _T("appleseed"),
            INTERNET_OPEN_TYPE_DIRECT,
            nullptr,
            nullptr,
            0);
    if (session == nullptr)
        return GetLastError();

    HINTERNET connection =
        InternetConnect(
            session,
            _T("api.github.com"),
            INTERNET_DEFAULT_HTTPS_PORT,
            nullptr,
            nullptr,
            INTERNET_SERVICE_HTTP,
            0,
            0);
    if (connection == nullptr)
    {
        const DWORD result = GetLastError();
        InternetCloseHandle(session);
        return result;
    }

    static const TCHAR* AcceptTypes[] = { _T("application/json"), nullptr };
    HINTERNET request =
        HttpOpenRequest(
            connection,
            _T("GET"),
            _T("/repos/appleseedhq/appleseed-max/releases"),
            nullptr,
            nullptr,
            AcceptTypes,
            INTERNET_FLAG_SECURE | INTERNET_FLAG_RELOAD,
            0);
    if (request == nullptr)
    {
        const DWORD result = GetLastError();
        InternetCloseHandle(connection);
        InternetCloseHandle(session);
        return result;
    }

    if (!HttpSendRequest(request, nullptr, 0, nullptr, 0))
    {
        const DWORD result = GetLastError();
        InternetCloseHandle(request);
        InternetCloseHandle(connection);
        InternetCloseHandle(session);
        return result;
    }

    char buffer[4096];
    DWORD bytes_read = -1;
    BOOL success = TRUE;
    while (success && bytes_read != 0)
    {
        success = InternetReadFile(request, buffer, sizeof(buffer), &bytes_read);
        response.append(buffer, bytes_read);
    }

    const DWORD result = GetLastError();
    InternetCloseHandle(request);
    InternetCloseHandle(connection);
    InternetCloseHandle(session);

    return result;
}

int main()
{
    std::string json;
    const DWORD result = query_release_information(json);

    rapidjson::Document doc;
    doc.Parse(json.c_str());

    assert(doc.IsArray());

    const auto& release = doc[0];
    const auto& tag_name = release["tag_name"];
    const auto& published_date = release["published_at"];
    const auto& asset = release["assets"][0];
    const auto& asset_download_url = asset["browser_download_url"];

    int year, month, day, hours, minutes, seconds;
    sscanf(
        published_date.GetString(),
        "%d-%d-%dT%d:%d:%d:%dZ",
        &year, &month, &day, &hours, &minutes, &seconds);

    std::tm time;
    time.tm_year = year - 1900;
    time.tm_mon = month - 1;
    time.tm_mday = day;
    time.tm_hour = hours;
    time.tm_min = minutes;
    time.tm_sec = seconds;

    const std::time_t t = std::mktime(&time);
    const std::tm* local_time = std::localtime(&t);

    char buffer[1024];
    std::strftime(buffer, sizeof(buffer), "%b %d %Y", local_time);

    std::cout << buffer << std::endl;

    return 0;
}

Answer 1

You're actually writing in C but using some C++ features, I'd drop C style (and functions) in favor of their C++ counterparts:

1) Even if you're using a C API you should write C++ classes to hide them. Ratchet already addressed this issue in his answer then I won't repeat again here.

2) You're using sscanf to parse a string, you should use std::stringstream instead or - better - directly use std::get_time to parse it.

3) You're formatting your date using std::strftime but starting with C++ 11 (but supported from VC2K10) you have a better option: std::put_time (better because, among other things, it will directly return a std::string). Note that this will also simplify code around that.

4) You have many magic strings, move them to const fields in your class (see also point 1).

Moreover:

5) InternetReadFile reads line by line and it returns ERROR_INSUFFICIENT_BUFFER if lpBuffer is not big enough. It means that if one single line exceeds 4096 bytes then your code will fail. You should handle this case (checking for ERROR_INSUFFICIENT_BUFFER error code) and reiterating reading.

6) You may get extended error information (with useful server's messages) using InternetGetLastResponseInfo(). Win32 error alone won't help you to exactly understand what went wrong server-side (which parameter is wrong, access errors, bad request and so on).

7) I'd also use INTERNET_FLAG_NO_UI for HttpOpenRequest in case your application will run unattended. To be honest I'd drop WinINet functions in favor of WinHTTP...

8) You're not checking rapidjson::Document::Parse() return value, errors may go unnoticed.

9) Your main() function is pretty big, split code into more specific functions: download_and_parse_release_information(), extract_publishing_date() and print_publishing_date(). Code itself (especially after moving to C++) is pretty simple but you'll start building a toolbox of reusable functions.

10) Next step is to refactor them into a proper reusable class github::project with proper methods (github::project::find_releases()) and structs (github::release).

Answer 2

Your code can benefit from RAII. For example by creating a small HInternet struct with a destructor to hold a single HINTERNET. Then you don't need to worry about calling InternetCloseHandle in the correct places as the destructor will do it for you.

struct HInternet{

    HInternet(HINTERNET handle) : handle(handle){}
    HInternet() : handle(nullptr){}
    ~HInternet(){
        if(handle)
            InternetCloseHandle(handle);
    }

    //disable copy
    HInternet(const HInternet&) = delete;
    HInternet operator=(const HInternet&) = delete;
    //optionally add move capabilities

    HINTERNET get() const{
        return handle;
    }

    operator bool() { return handle != nullptr; }

 private:
    HINTERNET handle;
}

If HINTERNET is actually a pointer to some struct then you can use a std::unique_ptr<INTERNET, InternetClose> where InternetClose is a empty struct with a single operator() function:

struct InternetClose{
    void operator() (HINTERNET handle){
        InternetCloseHandle(handle);
    }
}

typedef std::unique_ptr<INTERNET, InternetClose> HInternet;

Answer 3

I integrated a number of suggestions by @ratchet and @Adriano:

• I switched to automatic closing of handles, RAII-style (in a slightly simpler manner than suggested by @ratchet: I added implicit conversions to HINTERNET, one of the few legitimate use of conversion operators IMHO).
• I added logic to handle ERROR_INSUFFICIENT_BUFFER errors returned by InternetReadFile() (see note below).
• I added proper error handling when navigating the JSON response.
• I added the INTERNET_FLAG_NO_UI flag to the HttpOpenRequest call.
• I moved a couple of magic strings to constants.

Regarding the use of std::put_time() and std::get_time() (which I considered when I wrote the initial code), what put me off is that, as far as I can tell, they don't actually work with strings, but with streams. I find that this introduces boilerplate without bringing much tangible benefits.

Regarding InternetGetLastResponseInfo() and the (good) suggestions to introduce a github class: I'm not willing to do too much error handling or formalization as this code is simply meant to retrieve whether a newer update of the product is available (in which case it should print a few information about the new release like the version, the publication date and maybe a download link). That's why I'm trying to keep this code as focused and straightforward as possible, until further needs arise. (But of course it should not crash the application, hence the proper error handling.)

Regarding the handling of ERROR_INSUFFICIENT_BUFFER errors: while they don't seem to actually occur (for this particular URL maybe?), I've followed @Adriano's advice and handled them properly using an exponentially growing buffer.

Here is the revised code:

// Code in public domain, do whatever you want with it.

#include "rapidjson/document.h"

#include <Windows.h>
#include <wininet.h>
#include <tchar.h>

#include <cstddef>
#include <ctime>
#include <exception>
#include <iostream>
#include <string>
#include <vector>

namespace json = rapidjson;

class HInternet
{
  public:
    HInternet(const HINTERNET handle)
      : m_handle(handle)
    {
    }

    ~HInternet()
    {
        if (m_handle != nullptr)
            InternetCloseHandle(m_handle);
    }

    operator HINTERNET() const
    {
        return m_handle;
    }

  private:
    const HINTERNET m_handle;
};

DWORD query_release_information(std::string& response)
{
    static const TCHAR* Host = _T("api.github.com");
    static const TCHAR* Path = _T("/repos/appleseedhq/appleseed-max/releases");

    response.clear();

    const HInternet session =
        InternetOpen(
            _T("appleseed"),
            INTERNET_OPEN_TYPE_DIRECT,
            nullptr,
            nullptr,
            0);
    if (!session)
        return GetLastError();

    const HInternet connection =
        InternetConnect(
            session,
            Host,
            INTERNET_DEFAULT_HTTPS_PORT,
            nullptr,
            nullptr,
            INTERNET_SERVICE_HTTP,
            0,
            0);
    if (!connection)
        return GetLastError();

    static const TCHAR* AcceptTypes[] = { _T("application/json"), nullptr };
    const HInternet request =
        HttpOpenRequest(
            connection,
            _T("GET"),
            Path,
            nullptr,
            nullptr,
            AcceptTypes,
            INTERNET_FLAG_SECURE | INTERNET_FLAG_NO_UI | INTERNET_FLAG_RELOAD,
            0);
    if (!request)
        return GetLastError();

    if (!HttpSendRequest(request, nullptr, 0, nullptr, 0))
        return GetLastError();

    std::vector<char> buffer(4096);

    while (true)
    {
        DWORD bytes_read;
        if (!InternetReadFile(request, &buffer[0], buffer.size(), &bytes_read))
        {
            const DWORD last_error = GetLastError();
            if (last_error == ERROR_INSUFFICIENT_BUFFER)
                buffer.resize(buffer.size() * 2);
            else return last_error;
        }

        if (bytes_read > 0)
            response.append(&buffer[0], bytes_read);
        else return ERROR_SUCCESS;
    }
}

struct JSONMemberNotFound : public std::exception {};

const json::Value& get_member(const json::Value& parent, const json::Value::Ch* member)
{
    if (!parent.HasMember(member))
        throw JSONMemberNotFound();
    return parent[member];
}

const json::Value& get_index(const json::Value& parent, const std::size_t index)
{
    if (!parent.IsArray() || index >= parent.Size())
        throw JSONMemberNotFound();
    return parent[index];
}

int main()
{
    std::string json;
    if (query_release_information(json) != ERROR_SUCCESS)
        return -1;

    json::Document doc;
    if (doc.Parse(json.c_str()).HasParseError())
        return -1;

    try
    {
        const auto& release = get_index(doc, 0);
        const auto& tag_name = get_member(release, "tag_name");
        const auto& published_date = get_member(release, "published_at");
        const auto& assets = get_member(release, "assets");
        const auto& asset = get_index(assets, 0);
        const auto& asset_download_url = get_member(asset, "browser_download_url");

        int year, month, day, hours, minutes, seconds;
        std::sscanf(
            published_date.GetString(),
            "%d-%d-%dT%d:%d:%d:%dZ",
            &year, &month, &day, &hours, &minutes, &seconds);

        std::tm time;
        time.tm_year = year - 1900;
        time.tm_mon = month - 1;
        time.tm_mday = day;
        time.tm_hour = hours;
        time.tm_min = minutes;
        time.tm_sec = seconds;

        const std::time_t t = std::mktime(&time);
        const std::tm* local_time = std::localtime(&t);

        char buffer[1024];
        std::strftime(buffer, sizeof(buffer), "%b %d %Y", local_time);

        std::cout << buffer << std::endl;

        return 0;
    }
    catch (const JSONMemberNotFound&)
    {
        return -1;
    }
}