HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

HTTP downloader using Beast

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
downloaderhttpusingbeast

Problem

I have written a small HTTP downloader using

  • boost::asio



  • Beast library (proposed to be included in Boost)



  • network::uri library for handling URIs



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 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 
#include 

#include 
#include 

#include 
#include 

#include 

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

private:
  void read_handler(const boost::system::error_code &ec,
                    std::shared_ptr> response,
                    std::shared_ptr);

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

#include

#include
#include
#include

#include

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

Downloader::Downloader(const std::vector &urls) : tcp_socket(ioservice), resolv(ioservice) {
std::transform(urls.begin(), urls.end(), std::back_inserter(uris), this {
std::error_code ec;
network::uri u(url, ec);
if(ec) {
ioservice.post([=] {
std::cout > response

Solution

Missing Headers

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

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

#include
#include
#include

#include
#include
#include
#include
#include

#include
#include
#include
#include
#include

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

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

private:

struct State
{
std::promise promise;
network::uri uri;
boost::asio::ip::tcp::socket socket;
std::unique_ptr> ssl_stream;
std::unique_ptr> response;
std::unique_ptr streambuf;

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

void download_async(const std::string& url, std::promise&& 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 promise;
auto result = promise.get_future();

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

return result;
}

void HttpDownloader::download_async(const std::string& url, std::promise&& promise)
{
auto state = std::make_shared(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 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>(state->socket, ctx);
state->ssl_stream->set_verify_mode(boost::asio::ssl::verify_fail_if_no_peer_

Code Snippets

#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,
                                

Context

StackExchange Code Review Q#148596, answer score: 6

Revisions (0)

No revisions yet.