Commit f8c1c492 authored by Dominik Charousset's avatar Dominik Charousset

Integrate review feedback

parent d4175657
......@@ -45,6 +45,7 @@ namespace caf {
namespace net {
namespace basp {
/// An implementation of BASP as an application layer protocol.
class application {
public:
// -- member types -----------------------------------------------------------
......@@ -184,10 +185,10 @@ private:
/// Stores a pointer to the parent actor system.
actor_system* system_ = nullptr;
/// Stores what we are expecting to receive next.
/// Stores the expected type of the next incoming message.
connection_state state_ = connection_state::await_handshake_header;
/// Caches the last header;we need to store it when waiting for the payload.
/// Caches the last header while waiting for the matching payload.
header hdr_;
/// Re-usable buffer for storing payloads.
......@@ -199,7 +200,7 @@ private:
/// Stores the ID of our peer.
node_id peer_id_;
/// Keeps track of which local actors our peer monitors.
/// Tracks which local actors our peer monitors.
std::unordered_set<actor_addr> monitored_actors_;
/// Caches actor handles obtained via `resolve`.
......
......@@ -28,11 +28,10 @@ namespace basp {
/// Stores the state of a connection in a `basp::application`.
enum class connection_state {
/// Indicates that we successfully checked the magic number and now wait for
/// the handshake header.
/// Initial state for any connection to wait for the peer's handshake.
await_handshake_header,
/// Indicates that we received the header for the handshake and now wait for
/// the payload.
/// Indicates that the header for the peer's handshake arrived and BASP
/// requires the payload next.
await_handshake_payload,
/// Indicates that a connection is established and this node is waiting for
/// the next BASP header.
......@@ -40,7 +39,7 @@ enum class connection_state {
/// Indicates that this node has received a header with non-zero payload and
/// is waiting for the data.
await_payload,
/// Indicates that we are about to close this connection.
/// Indicates that the connection is about to shut down.
shutdown,
};
......
......@@ -18,6 +18,7 @@
#pragma once
#include <cstddef>
#include <cstdint>
namespace caf {
......@@ -30,6 +31,9 @@ namespace basp {
/// @note BASP is not backwards compatible.
constexpr uint64_t version = 1;
/// Size of a BASP header in serialized form.
constexpr size_t header_size = 13;
/// @}
} // namespace basp
......
......@@ -21,12 +21,12 @@
#include <array>
#include <cstdint>
#include "caf/byte.hpp"
#include "caf/detail/comparable.hpp"
#include "caf/fwd.hpp"
#include "caf/meta/hex_formatted.hpp"
#include "caf/meta/type_name.hpp"
#include "caf/net/basp/constants.hpp"
#include "caf/net/basp/message_type.hpp"
#include "caf/span.hpp"
namespace caf {
namespace net {
......@@ -34,10 +34,9 @@ namespace basp {
/// @addtogroup BASP
/// The header of a Binary Actor System Protocol (BASP) message.
struct header : detail::comparable<header> {
message_type type;
uint32_t payload_len;
uint64_t operation_data;
// -- constructors, destructors, and assignment operators --------------------
constexpr header() noexcept
: type(message_type::handshake), payload_len(0), operation_data(0) {
......@@ -54,15 +53,28 @@ struct header : detail::comparable<header> {
header& operator=(const header&) noexcept = default;
int compare(header other) const noexcept;
// -- factory functions ------------------------------------------------------
/// @pre `bytes.size() == header_size`
static header from_bytes(span<const byte> bytes);
};
/// Size of a BASP header in serialized form
constexpr size_t header_size = 13;
// -- comparison -------------------------------------------------------------
int compare(header other) const noexcept;
// -- member variables -------------------------------------------------------
/// Denotes the BASP operation and how `operation_data` gets interpreted.
message_type type;
/// Stores the size in bytes for the payload that follows this header.
uint32_t payload_len;
/// Stores type-specific information such as the BASP version in handshakes.
uint64_t operation_data;
};
/// Serializes a header to a byte representation.
/// @relates header
std::array<byte, header_size> to_bytes(header x);
......
......@@ -33,41 +33,41 @@ enum class message_type : uint8_t {
/// Sends supported BASP version and node information to the server.
///
/// ![](client_handshake.png)
handshake = 0x00,
handshake = 0,
/// Transmits an actor-to-actor messages.
///
/// ![](direct_message.png)
actor_message = 0x01,
actor_message = 1,
/// Tries to resolve a path on the receiving node.
///
/// ![](resolve_request.png)
resolve_request = 0x02,
resolve_request = 2,
/// Transmits the result of a path lookup.
///
/// ![](resolve_response.png)
resolve_response = 0x03,
resolve_response = 3,
/// Informs the receiving node that the sending node has created a proxy
/// instance for one of its actors. Causes the receiving node to attach a
/// functor to the actor that triggers a down_message on termination.
///
/// ![](monitor_message.png)
monitor_message = 0x04,
monitor_message = 4,
/// Informs the receiving node that it has a proxy for an actor that has been
/// terminated.
///
/// ![](down_message.png)
down_message = 0x05,
down_message = 5,
/// Used to generate periodic traffic between two nodes in order to detect
/// disconnects.
///
/// ![](heartbeat.png)
heartbeat = 0x06,
heartbeat = 6,
};
/// @relates message_type
......
......@@ -89,7 +89,7 @@ void application::resolve_remote_path(write_packet_callback& write_packet,
auto hdr = to_bytes(header{message_type::resolve_request,
static_cast<uint32_t>(buf_.size()), req_id});
if (auto err = write_packet(hdr, buf_)) {
CAF_LOG_ERROR("unable to serialize path");
CAF_LOG_ERROR("unable to write resolve_request header");
return;
}
response_promise rp{nullptr, actor_cast<strong_actor_ptr>(listener),
......@@ -116,20 +116,8 @@ error application::handle(size_t& next_read_size,
return none;
}
case connection_state::await_handshake_payload: {
node_id nid;
std::vector<std::string> app_ids;
binary_deserializer source{system(), bytes};
if (auto err = source(nid, app_ids))
if (auto err = handle_handshake(write_packet, hdr_, bytes))
return err;
if (!nid || app_ids.empty())
return ec::invalid_handshake;
auto ids = get_or(system().config(), "middleman.app-identifiers",
defaults::middleman::app_identifiers);
auto predicate = [=](const std::string& x) {
return std::find(ids.begin(), ids.end(), x) != ids.end();
};
if (std::none_of(app_ids.begin(), app_ids.end(), predicate))
return ec::app_identifiers_mismatch;
state_ = connection_state::await_header;
return none;
}
......@@ -178,12 +166,20 @@ error application::handle_handshake(write_packet_callback&, header hdr,
return ec::missing_handshake;
if (hdr.operation_data != version)
return ec::version_mismatch;
binary_deserializer source{nullptr, payload};
node_id peer_id;
std::vector<std::string> app_ids;
binary_deserializer source{system(), payload};
if (auto err = source(peer_id, app_ids))
return err;
// TODO: verify peer_id and app_ids
if (!peer_id || app_ids.empty())
return ec::invalid_handshake;
auto ids = get_or(system().config(), "middleman.app-identifiers",
defaults::middleman::app_identifiers);
auto predicate = [=](const std::string& x) {
return std::find(ids.begin(), ids.end(), x) != ids.end();
};
if (std::none_of(app_ids.begin(), app_ids.end(), predicate))
return ec::app_identifiers_mismatch;
peer_id_ = std::move(peer_id);
state_ = connection_state::await_header;
return none;
......@@ -276,6 +272,7 @@ error application::handle_resolve_response(write_packet_callback&, header hdr,
}
error application::generate_handshake() {
buf_.clear();
serializer_impl<buffer_type> sink{system(), buf_};
return sink(system().node(),
get_or(system().config(), "middleman.app-identifiers",
......
......@@ -20,6 +20,7 @@
#include "caf/atom.hpp"
#include "caf/error.hpp"
#include "caf/string_view.hpp"
namespace caf {
namespace net {
......@@ -27,7 +28,7 @@ namespace basp {
namespace {
const char* ec_names[] = {
string_view ec_names[] = {
"none",
"invalid_magic_number",
"unexpected_number_of_bytes",
......@@ -36,8 +37,9 @@ const char* ec_names[] = {
"illegal_state",
"invalid_handshake",
"missing_handshake",
"unexpected_handshake",
"version_mismatch",
"unimplemented",
"unimplemented = 10",
"app_identifiers_mismatch",
"invalid_payload",
};
......@@ -45,7 +47,8 @@ const char* ec_names[] = {
} // namespace
std::string to_string(ec x) {
return ec_names[static_cast<uint8_t>(x)];
auto result = ec_names[static_cast<uint8_t>(x)];
return std::string{result.begin(), result.end()};
}
error make_error(ec x) {
......
......@@ -20,7 +20,9 @@
#include <cstring>
#include "caf/byte.hpp"
#include "caf/detail/network_order.hpp"
#include "caf/span.hpp"
namespace caf {
namespace net {
......
......@@ -34,8 +34,8 @@ string_view message_type_names[] = {
} // namespace
std::string to_string(message_type x) {
auto sv = message_type_names[static_cast<uint8_t>(x)];
return std::string{sv.begin(), sv.end()};
auto result = message_type_names[static_cast<uint8_t>(x)];
return std::string{result.begin(), result.end()};
}
} // namespace basp
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment