7
\$\begingroup\$

I have written a small HTTP downloader using

It's nowhere near completion but I would like to get some feedback from you guys. I wanted to use as much of async interface as I could.

The idea is that it gets a std::vector<std::string> with URLs to fetch and then it goes callback after callback to finally print the response data on screen.

This review's purpose is that I would like to e.g. add support for HTTPS and while doing that I would like to write unit tests mocking somehow (and preferably splitting this class) network (asio) dependencies.

Downloader.hpp

#include <string>
#include <vector>

#include <boost/asio/io_service.hpp>
#include <boost/asio/ip/tcp.hpp>

#include <beast/core/streambuf.hpp>
#include <beast/http/string_body.hpp>

#include <network/uri/uri.hpp>

class Downloader {
public:
  Downloader(const std::vector<std::string> &urls);
  void go();

private:
  void read_handler(const boost::system::error_code &ec,
                    std::shared_ptr<beast::http::response<beast::http::string_body>> response,
                    std::shared_ptr<beast::streambuf>);

  void connect_handler(const boost::system::error_code &ec, const network::uri &uri);

  void queue_read(const boost::system::error_code &ec);

  void resolve_handler(const boost::system::error_code &ec,
                       boost::asio::ip::tcp::resolver::iterator it,
                       const network::uri &uri);

  std::vector<network::uri> uris;

  boost::asio::io_service ioservice;
  boost::asio::ip::tcp::socket tcp_socket;
  boost::asio::ip::tcp::resolver resolv;
};

Downloader.cpp

#include "Downloader.hpp"

#include <iostream>

#include <boost/asio/ip/tcp.hpp>

#include <beast/http/empty_body.hpp>
#include <beast/http/read.hpp>
#include <beast/http/write.hpp>

#include <network/uri/uri.hpp>

using namespace std::placeholders;  // for _1, _2 etc.
using namespace boost::asio;
using namespace boost::asio::ip;

Downloader::Downloader(const std::vector<std::string> &urls) : tcp_socket(ioservice), resolv(ioservice) {
  std::transform(urls.begin(), urls.end(), std::back_inserter(uris), [this](const std::string &url) {
    std::error_code ec;
    network::uri u(url, ec);
    if(ec) {
      ioservice.post([=] {
        std::cout << "Error parsing URL : " << url << '\n' << "Error code : " << ec.message() << '\n';
      });
    }
    return u;
  });
}

void Downloader::go() {
  for(const auto &uri : uris) {
    std::cout << "*******************************" << '\n'
              << "Resolving " << uri.host() << '\n'
              << "*******************************\n\n";

    resolv.async_resolve(
        tcp::resolver::query{uri.host().to_string(), (uri.scheme().to_string() == "https" ? "443" : "80")},
        std::bind(&Downloader::resolve_handler, this, _1, _2, uri));
  }

  ioservice.run();
}

void Downloader::read_handler(const boost::system::error_code &ec,
                              std::shared_ptr<beast::http::response<beast::http::string_body>> response,
                              std::shared_ptr<beast::streambuf>) {
  if(ec) {
    ioservice.post([=] {
      std::cerr << "Problem reading the response: \n"
                << "error : " << ec << '\n'
                << "error_core.value() : " << ec.value() << '\n'
                << "error_core.message() : " << ec.message() << '\n';
    });
    return;
  }

  std::cout << "\n*******************************" << '\n'
            << "Headers\n\n";
  for(auto i : response->fields) std::cout << i.first << " : " << i.second << '\n';
  std::cout << "*******************************"
            << "\n\n";

  std::cout << "Received status code: " << response->status << '\n';

  if((response->status == 301 || response->status == 302) && response->fields.exists("Location")) {
    network::uri u(response->fields["Location"].to_string());
    std::cout << "Added a new request for redirection to " << u.string() << '\n';
    resolv.async_resolve(
        tcp::resolver::query{u.host().to_string(), (u.scheme().to_string() == "https" ? "443" : "80")},
        std::bind(&Downloader::resolve_handler, this, _1, _2, u));
    return;
  }

  std::cout << "*******************************" << '\n'
            << "Response body\n\n" << response->body
            << "\n\n";
}

void Downloader::connect_handler(const boost::system::error_code &ec, const network::uri& uri) {
  if(ec) {
    ioservice.post([=] {
      std::cout << "error connecting : " << ec << '\n'
                << "error_core.value() : " << ec.value() << '\n'
                << "error_core.message() : " << ec.message() << '\n';
    });
    return;
  }

  // Send HTTP(S) request using beast
  beast::http::request<beast::http::empty_body> req;
  req.method = "GET";
  req.url = (uri.path().empty() ? "/" : uri.path().to_string());
  req.version = 11;
  req.fields.insert("Host", uri.host().to_string());
  req.fields.insert("User-Agent", "Beast");
  beast::http::prepare(req);

  const bool HTTPS = (uri.scheme().to_string() == "https");
  std::cout << "*******************************" << '\n'
            << "Sending a HTTP" << (HTTPS ? "S" : "") << " request\n\n" << req
            << "*******************************" << '\n';

  beast::http::async_write(tcp_socket, std::move(req), std::bind(&Downloader::queue_read, this, _1));
}

void Downloader::queue_read(const boost::system::error_code &ec) {
  if(ec) {
    ioservice.post([=] {
      std::cerr << "error : " << ec << '\n'
                << "error_core.value() : " << ec.value() << '\n'
                << "error_core.message() : " << ec.message() << '\n';
    });
    return;
  }

  auto response = std::make_shared<beast::http::response<beast::http::string_body>>();
  auto response_streambuf = std::make_shared<beast::streambuf>();

  beast::http::async_read(tcp_socket,
                          *response_streambuf,
                          *response,
                          std::bind(&Downloader::read_handler, this, _1, response, response_streambuf));
}

void Downloader::resolve_handler(const boost::system::error_code &ec,
                                 tcp::resolver::iterator it,
                                 const network::uri& uri) {
  if(ec) {
    ioservice.post([=] {
      std::cerr << "Problem resolving URL: \"" << uri.host() << "\"\n" << ec.message() << '\n';
    });
    return;
  }

  auto ep = it->endpoint();
  std::cout << "*******************************" << '\n'
            << "Resolved to " << ep.address() << ':' << ep.port() << '\n'
            << "*******************************"
            << "\n\n";

  tcp_socket.async_connect(*it, std::bind(&Downloader::connect_handler, this, _1, uri));
}
\$\endgroup\$
6
\$\begingroup\$

Missing Headers

Before going any further, the current code is missing a few headers that it needs, namely: <functional> and <memory>.

Navigating to Location doesn't work

Next, I've ran the code against a site that redirects me to a different location and the second call resulted in an error:

error_core.message(): A connect request was made on an already connected socket

HTTPS doesn't currently work

Using beast for HTTPS requires a bit more code, as can be seen in the official example.

API

Your Downloader class is difficult to use

  • It creates its own io_service while the user might want to use an already existing one
  • It only prints the output using cout

C++ provides std::future<> to ease working with asynchronous APIs. Futures can also pass exceptions.

I would refactor the code like the following, with the main changes being:

  • Pass an already existing io_service
  • Once created, be able to pass in requests at any time
  • Return futures of the beast response class for each individual download. Pass any exception to the future and leave it to the user to handle the result

Suggestion:

Warning! The code assumes that the HttpDownloader instance will not be destroyed while any async operation it initiated is pending. Correctly dealing with cancellation and the lifetime of the downloader instance requires extra code.

#include <network/uri/uri.hpp>

#include <boost/asio/io_service.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/ssl.hpp>

#include <beast/core/streambuf.hpp>
#include <beast/http/string_body.hpp>
#include <beast/http/empty_body.hpp>
#include <beast/http/read.hpp>
#include <beast/http/write.hpp>

#include <iostream>
#include <string>
#include <memory>
#include <functional>
#include <future>

class HttpDownloader
{
public:
    using response_type = beast::http::response<beast::http::string_body>;
    using future_type = std::future<response_type>;

    HttpDownloader(boost::asio::io_service& service);
    future_type download_async(const std::string& url);

private:

    struct State
    {
        std::promise<response_type> promise;
        network::uri uri;
        boost::asio::ip::tcp::socket socket;
        std::unique_ptr<boost::asio::ssl::stream<boost::asio::ip::tcp::socket&>> ssl_stream;
        std::unique_ptr<beast::http::response<beast::http::string_body>> response;
        std::unique_ptr<beast::streambuf> streambuf;

        State(std::promise<response_type>&& promise, boost::asio::ip::tcp::socket&& socket) : 
            promise{std::move(promise)}, socket(std::move(socket))
        {
        }
    };
    using state_ptr = std::shared_ptr<State>;

    void download_async(const std::string& url, std::promise<response_type>&& promise);
    void download_async(state_ptr state);
    void on_resolve(state_ptr state, const boost::system::error_code& ec, boost::asio::ip::tcp::resolver::iterator iterator);
    void on_connect(state_ptr state, const boost::system::error_code& ec);
    void on_request_sent(state_ptr state, const boost::system::error_code& ec);
    void on_read(state_ptr state, const boost::system::error_code& ec);

    boost::asio::io_service& service_;
    boost::asio::ip::tcp::resolver resolver_;
};

HttpDownloader::HttpDownloader(boost::asio::io_service& service) : service_{ service }, resolver_{ service }
{    
}

HttpDownloader::future_type HttpDownloader::download_async(const std::string& url)
{
    std::promise<response_type> promise;
    auto result = promise.get_future();

    download_async(url, std::move(promise));

    return result;
}

void HttpDownloader::download_async(const std::string& url, std::promise<response_type>&& promise)
{
    auto state = std::make_shared<State>(std::move(promise), boost::asio::ip::tcp::socket{ service_ });
    try
    {
        state->uri = network::uri{ url };

        download_async(state);
    }
    catch (...)
    {
        state->promise.set_exception(std::current_exception());
    }   
}

void HttpDownloader::download_async(state_ptr state)
{
    boost::asio::ip::tcp::resolver::query query(state->uri.host().to_string(), state->uri.scheme().to_string());
    resolver_.async_resolve(query, std::bind(&HttpDownloader::on_resolve, this, state, std::placeholders::_1, std::placeholders::_2));
}

void HttpDownloader::on_resolve(state_ptr state,
                                const boost::system::error_code& ec, boost::asio::ip::tcp::resolver::iterator iterator)
{
    if (ec)
    {
        state->promise.set_exception(std::make_exception_ptr(boost::system::system_error(ec)));
        return;
    }

    state->socket.async_connect(*iterator, std::bind(&HttpDownloader::on_connect, this, state, std::placeholders::_1));
}

void HttpDownloader::on_connect(state_ptr state, const boost::system::error_code& ec)
{
    if (ec)
    {
        state->promise.set_exception(std::make_exception_ptr(boost::system::system_error(ec)));
        return;
    }

    beast::http::request<beast::http::empty_body> req;
    req.method = "GET";
    req.url = state->uri.path().empty() ? "/" : state->uri.path().to_string();
    req.version = 11;
    req.fields.insert("Host", state->uri.host().to_string());
    req.fields.insert("User-Agent", "Beast");
    beast::http::prepare(req);

    if (state->uri.scheme().to_string() == "https")
    {
        boost::asio::ssl::context ctx{ boost::asio::ssl::context::tlsv12 };
        state->ssl_stream = std::make_unique<boost::asio::ssl::stream<boost::asio::ip::tcp::socket&>>(state->socket, ctx);
        state->ssl_stream->set_verify_mode(boost::asio::ssl::verify_fail_if_no_peer_cert);
        try
        {
            state->ssl_stream->handshake(boost::asio::ssl::stream_base::client);
        }
        catch(...)
        {
            state->promise.set_exception(std::current_exception());
            return;
        }

        beast::http::async_write(*state->ssl_stream, std::move(req), 
                                 std::bind(&HttpDownloader::on_request_sent, this, state, std::placeholders::_1));
    }
    else
    {
        beast::http::async_write(state->socket, std::move(req), 
                                 std::bind(&HttpDownloader::on_request_sent, this, state, std::placeholders::_1));
    }

}

void HttpDownloader::on_request_sent(state_ptr state, const boost::system::error_code& ec)
{
    if (ec)
    {
        state->promise.set_exception(std::make_exception_ptr(boost::system::system_error(ec)));
        return;
    }

    state->response = std::make_unique<beast::http::response<beast::http::string_body>>();
    state->streambuf = std::make_unique<beast::streambuf>();

    if (state->ssl_stream)
    {
        beast::http::async_read(*state->ssl_stream, *state->streambuf, *state->response,
                                std::bind(&HttpDownloader::on_read, this, state, std::placeholders::_1));
    }
    else
    {
        beast::http::async_read(state->socket, *state->streambuf, *state->response,
                                std::bind(&HttpDownloader::on_read, this, state, std::placeholders::_1));
    }
}

void HttpDownloader::on_read(state_ptr state, const boost::system::error_code& ec)
{
    if (ec)
    {
        state->promise.set_exception(std::make_exception_ptr(boost::system::system_error(ec)));
        return;
    }

    if ((state->response->status == 301 || state->response->status == 302) && state->response->fields.exists("Location")) 
    {
        download_async(state->response->fields["Location"].to_string(), std::move(state->promise));
        return;
    }

    state->promise.set_value(std::move(*state->response));
}

int main()
{
    boost::asio::io_service io_service;

    HttpDownloader downloader{io_service};

    std::string urls[] =
    {
        "http://site1",
        "https://site2"
    };
    std::tuple<std::string, HttpDownloader::future_type> downloads[std::extent<decltype(urls)>::value];

    std::transform(begin(urls), end(urls), begin(downloads), [&](auto& url)
    {
        return std::make_tuple(url, std::move(downloader.download_async(url)));
    });

    std::thread asio_thread{ [&]() { io_service.run();} };

    for (auto& tuple : downloads)
    {
        auto& url = std::get<0>(tuple);
        auto& download = std::get<1>(tuple);

        std::cout << url << "\n===\n";
        try
        {
            auto response = download.get();

            std::cout << "Received status code: " << response.status << '\n';

            for (auto pair : response.fields)
            {
                std::cout << pair.first << " : " << pair.second << '\n';
            }
        }
        catch (boost::system::system_error& e)
        {
            std::cout << "Error (" << e.code() << "): " << e.what() << "\n";
        }
        catch (std::exception& e)
        {
            std::cout << "Error: " << e.what() << "\n";
        }
        std::cout << "\n";
    }

    asio_thread.join();
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.