c++boostboost-asioboost-beastbeast-websockets

Should I clean up beast::flat_buffer when I see errors on on_read?


http_client_async_ssl

class session : public std::enable_shared_from_this<session>
{
    ...
    beast::flat_buffer buffer_; // (Must persist between reads)
    http::response<http::string_body> res_;
    ...
}

void on_write(beast::error_code ec, std::size_t bytes_transferred) {

  if (ec)
  {
    fail(ec, "write");
    return try_again();    
  }    

  // Receive the HTTP response
  http::async_read(
      stream_, buffer_, res_,
      beast::bind_front_handler(&session::on_read, shared_from_this()));
}

void on_read(beast::error_code ec, std::size_t bytes_transferred) {

  if (ec)
  {
    fail(ec, "read");
    return try_again();  
  }    

  // Step 1: process response
  //
  const auto &body_data = res_.body().data();
  user_parse_data(net::buffers_begin(body_data), net::buffers_end(body_data));
  
  // Step 2: clean up buffer_
  //
  buffer_.consume(buffer_.size()); // clean up buffer_ after finishing reading it

  // Step 3: continue to write
  ...
}

In the above implementation, I ONLY clean up the buffer_ when I finish parsing the data successfully.

Question> Should I clean up the buffer_ when I experience an error on the on_read too?

void on_read(beast::error_code ec, std::size_t bytes_transferred) {

  if (ec)
  {
    // clean up buffer_
    buffer_.consume(buffer_.size()); // Should we do the cleanup here too?
    
    fail(ec, "read");
    return try_again();    
  }    

  // Step 1: process response
  //
  const auto &body_data = res_.body().data();
  user_parse_data(net::buffers_begin(body_data), net::buffers_end(body_data));
  
  // Step 2: clean up buffer_
  //
  buffer_.consume(buffer_.size());

  // Step 3: continue to write
  ...
}

Solution

  • // Should we do the cleanup here too?
    

    That's asking the wrong question entirely.

    One obvious question that comes first is "should we cleanup the read buffer at all".

    And the more important question is: what do you do with the connection?

    The buffer belongs to the connection, as it represents stream data.

    The example you link always closes the connection. So the buffer is irrelevant after receiving the response - since the connection becomes irrelevant. Note that the linked example doesn't consume on the buffer either.

    Should You Cleanup At All?

    You should not cleanup after http::read!

    The reason is that http::read already consumes any data that was parsed as part of the response message.

    Even if you expect to read more messages from the same connection (e.g. HTTP pipelining), you need to start the next http::read with the same buffer since it might already contain (partial) data for the subsequent message.

    What About Errors?

    If you have an IO/parse error during HTTP transmissions, I expect in most circumstances the HTTP protocol specification will require you to shut down the connection.

    There is no "try_again" in HTTP/1. Once you've lost the thread on stream contents, there is no way you can recover to a "known state".

    Regardless, I'd always recommend shutting down failed HTTP sessions, because not doing so opens up to corrupted messages and security vulnerabilities.