Commit 423bb432 authored by Samir Halilcevic's avatar Samir Halilcevic

Integrate review feedback

Co-authored-by: default avatarDominik Charousset <dominik@charousset.de>
parent 687502fc
...@@ -105,8 +105,8 @@ enum class sec : uint8_t { ...@@ -105,8 +105,8 @@ enum class sec : uint8_t {
incompatible_versions, incompatible_versions,
/// Connection refused because of incompatible application IDs. /// Connection refused because of incompatible application IDs.
incompatible_application_ids, incompatible_application_ids,
/// The middleman received a malformed BASP message from another node. /// Received a malformed message from another node.
malformed_basp_message, malformed_message,
/// The middleman closed a connection because it failed to serialize or /// The middleman closed a connection because it failed to serialize or
/// deserialize a payload. /// deserialize a payload.
serializing_basp_payload_failed = 50, serializing_basp_payload_failed = 50,
......
...@@ -32,27 +32,6 @@ constexpr std::byte head(std::byte value) noexcept { ...@@ -32,27 +32,6 @@ constexpr std::byte head(std::byte value) noexcept {
} }
} }
// Takes the last N bits of `value`.
template <size_t N>
constexpr std::byte tail(std::byte value) noexcept {
if constexpr (N == 1) {
return value & 0b0000'0001_b;
} else if constexpr (N == 2) {
return value & 0b0000'0011_b;
} else if constexpr (N == 3) {
return value & 0b0000'0111_b;
} else if constexpr (N == 4) {
return value & 0b0000'1111_b;
} else if constexpr (N == 5) {
return value & 0b0001'1111_b;
} else if constexpr (N == 6) {
return value & 0b0011'1111_b;
} else {
static_assert(N == 7);
return value & 0b0111'1111_b;
}
}
// Checks whether `value` is an UTF-8 continuation byte. // Checks whether `value` is an UTF-8 continuation byte.
constexpr bool is_continuation_byte(std::byte value) noexcept { constexpr bool is_continuation_byte(std::byte value) noexcept {
return head<2>(value) == 0b1000'0000_b; return head<2>(value) == 0b1000'0000_b;
...@@ -92,23 +71,22 @@ bool validate_rfc3629(const std::byte* first, const std::byte* last) { ...@@ -92,23 +71,22 @@ bool validate_rfc3629(const std::byte* first, const std::byte* last) {
} }
// 1111'0xxx: 4-byte sequence. // 1111'0xxx: 4-byte sequence.
if (head<5>(x) == 0b1111'0000_b) { if (head<5>(x) == 0b1111'0000_b) {
uint64_t code_point = std::to_integer<uint64_t>(tail<3>(x)) << 18; // Check if code point is in the valid UTF range.
if (x > std::byte{0xf4})
return false;
if (first == last || !is_continuation_byte(*first)) if (first == last || !is_continuation_byte(*first))
return false; return false;
// No non-shortest form. // No non-shortest form.
if (x == 0b1111'0000_b && head<4>(*first) == 0b1000'0000_b) if (x == 0b1111'0000_b && head<4>(*first) == 0b1000'0000_b)
return false; return false;
code_point |= std::to_integer<uint64_t>(tail<6>(*first++)) << 12; // Check if code point is in the valid UTF range.
if (first == last || !is_continuation_byte(*first)) if (x == std::byte{0xf4} && *first >= std::byte{0x90})
return false; return false;
code_point |= std::to_integer<uint64_t>(tail<6>(*first++)) << 6; ++first;
if (first == last || !is_continuation_byte(*first)) if (first == last || !is_continuation_byte(*first++))
return false; return false;
code_point |= std::to_integer<uint64_t>(tail<6>(*first++)); if (first == last || !is_continuation_byte(*first++))
// Out of valid UTF range
if (code_point >= 0x110000) {
return false; return false;
}
continue; continue;
} }
return false; return false;
......
...@@ -113,8 +113,8 @@ std::string to_string(sec x) { ...@@ -113,8 +113,8 @@ std::string to_string(sec x) {
return "caf::sec::incompatible_versions"; return "caf::sec::incompatible_versions";
case sec::incompatible_application_ids: case sec::incompatible_application_ids:
return "caf::sec::incompatible_application_ids"; return "caf::sec::incompatible_application_ids";
case sec::malformed_basp_message: case sec::malformed_message:
return "caf::sec::malformed_basp_message"; return "caf::sec::malformed_message";
case sec::serializing_basp_payload_failed: case sec::serializing_basp_payload_failed:
return "caf::sec::serializing_basp_payload_failed"; return "caf::sec::serializing_basp_payload_failed";
case sec::redundant_connection: case sec::redundant_connection:
...@@ -306,8 +306,8 @@ bool from_string(std::string_view in, sec& out) { ...@@ -306,8 +306,8 @@ bool from_string(std::string_view in, sec& out) {
} else if (in == "caf::sec::incompatible_application_ids") { } else if (in == "caf::sec::incompatible_application_ids") {
out = sec::incompatible_application_ids; out = sec::incompatible_application_ids;
return true; return true;
} else if (in == "caf::sec::malformed_basp_message") { } else if (in == "caf::sec::malformed_message") {
out = sec::malformed_basp_message; out = sec::malformed_message;
return true; return true;
} else if (in == "caf::sec::serializing_basp_payload_failed") { } else if (in == "caf::sec::serializing_basp_payload_failed") {
out = sec::serializing_basp_payload_failed; out = sec::serializing_basp_payload_failed;
...@@ -429,7 +429,7 @@ bool from_integer(std::underlying_type_t<sec> in, ...@@ -429,7 +429,7 @@ bool from_integer(std::underlying_type_t<sec> in,
case sec::unavailable_or_would_block: case sec::unavailable_or_would_block:
case sec::incompatible_versions: case sec::incompatible_versions:
case sec::incompatible_application_ids: case sec::incompatible_application_ids:
case sec::malformed_basp_message: case sec::malformed_message:
case sec::serializing_basp_payload_failed: case sec::serializing_basp_payload_failed:
case sec::redundant_connection: case sec::redundant_connection:
case sec::remote_lookup_failed: case sec::remote_lookup_failed:
......
...@@ -27,8 +27,8 @@ enum connection_state { ...@@ -27,8 +27,8 @@ enum connection_state {
incompatible_versions, incompatible_versions,
/// See `sec::incompatible_application_ids`. /// See `sec::incompatible_application_ids`.
incompatible_application_ids, incompatible_application_ids,
/// See `sec::malformed_basp_message`. /// See `sec::malformed_message`.
malformed_basp_message, malformed_message,
/// See `sec::serializing_basp_payload_failed`. /// See `sec::serializing_basp_payload_failed`.
serializing_basp_payload_failed, serializing_basp_payload_failed,
/// See `sec::redundant_connection`. /// See `sec::redundant_connection`.
...@@ -55,8 +55,8 @@ inline sec to_sec(connection_state x) noexcept { ...@@ -55,8 +55,8 @@ inline sec to_sec(connection_state x) noexcept {
return sec::incompatible_versions; return sec::incompatible_versions;
case incompatible_application_ids: case incompatible_application_ids:
return sec::incompatible_application_ids; return sec::incompatible_application_ids;
case malformed_basp_message: case malformed_message:
return sec::malformed_basp_message; return sec::malformed_message;
case serializing_basp_payload_failed: case serializing_basp_payload_failed:
return sec::serializing_basp_payload_failed; return sec::serializing_basp_payload_failed;
case redundant_connection: case redundant_connection:
......
...@@ -55,17 +55,17 @@ connection_state instance::handle(execution_unit* ctx, new_data_msg& dm, ...@@ -55,17 +55,17 @@ connection_state instance::handle(execution_unit* ctx, new_data_msg& dm,
if (payload->size() != hdr.payload_len) { if (payload->size() != hdr.payload_len) {
CAF_LOG_WARNING("received invalid payload, expected" CAF_LOG_WARNING("received invalid payload, expected"
<< hdr.payload_len << "bytes, got" << payload->size()); << hdr.payload_len << "bytes, got" << payload->size());
return err(malformed_basp_message); return err(malformed_message);
} }
} else { } else {
binary_deserializer source{ctx, dm.buf}; binary_deserializer source{ctx, dm.buf};
if (!source.apply(hdr)) { if (!source.apply(hdr)) {
CAF_LOG_WARNING("failed to receive header:" << source.get_error()); CAF_LOG_WARNING("failed to receive header:" << source.get_error());
return err(malformed_basp_message); return err(malformed_message);
} }
if (!valid(hdr)) { if (!valid(hdr)) {
CAF_LOG_WARNING("received invalid header:" << CAF_ARG(hdr)); CAF_LOG_WARNING("received invalid header:" << CAF_ARG(hdr));
return err(malformed_basp_message); return err(malformed_message);
} }
if (hdr.payload_len > 0) { if (hdr.payload_len > 0) {
CAF_LOG_DEBUG("await payload before processing further"); CAF_LOG_DEBUG("await payload before processing further");
...@@ -309,11 +309,11 @@ connection_state instance::handle(execution_unit* ctx, connection_handle hdl, ...@@ -309,11 +309,11 @@ connection_state instance::handle(execution_unit* ctx, connection_handle hdl,
if (payload == nullptr) { if (payload == nullptr) {
if (hdr.payload_len != 0) { if (hdr.payload_len != 0) {
CAF_LOG_WARNING("missing payload"); CAF_LOG_WARNING("missing payload");
return malformed_basp_message; return malformed_message;
} }
} else if (hdr.payload_len != payload->size()) { } else if (hdr.payload_len != payload->size()) {
CAF_LOG_WARNING("actual payload size differs from advertised size"); CAF_LOG_WARNING("actual payload size differs from advertised size");
return malformed_basp_message; return malformed_message;
} }
// Dispatch by message type. // Dispatch by message type.
switch (hdr.operation) { switch (hdr.operation) {
...@@ -506,7 +506,7 @@ connection_state instance::handle(execution_unit* ctx, connection_handle hdl, ...@@ -506,7 +506,7 @@ connection_state instance::handle(execution_unit* ctx, connection_handle hdl,
} }
default: { default: {
CAF_LOG_ERROR("invalid operation"); CAF_LOG_ERROR("invalid operation");
return malformed_basp_message; return malformed_message;
} }
} }
return await_header; return await_header;
......
...@@ -204,7 +204,7 @@ ptrdiff_t framing::handle(uint8_t opcode, byte_span payload, ...@@ -204,7 +204,7 @@ ptrdiff_t framing::handle(uint8_t opcode, byte_span payload,
std::string_view text{reinterpret_cast<const char*>(payload.data()), std::string_view text{reinterpret_cast<const char*>(payload.data()),
payload.size()}; payload.size()};
if (!detail::rfc3629::valid(text)) { if (!detail::rfc3629::valid(text)) {
abort_and_shutdown(sec::runtime_error, "invalid UTF-8 sequence"); abort_and_shutdown(sec::malformed_message, "invalid UTF-8 sequence");
return -1; return -1;
} }
if (up_->consume_text(text) < 0) if (up_->consume_text(text) < 0)
......
...@@ -24,7 +24,7 @@ void lower_layer::shutdown(const error& reason) { ...@@ -24,7 +24,7 @@ void lower_layer::shutdown(const error& reason) {
shutdown(status::normal_close, to_string(reason)); shutdown(status::normal_close, to_string(reason));
} else if (reason.code() == static_cast<uint8_t>(sec::protocol_error)) { } else if (reason.code() == static_cast<uint8_t>(sec::protocol_error)) {
shutdown(status::protocol_error, to_string(reason)); shutdown(status::protocol_error, to_string(reason));
} else if (reason.code() == static_cast<uint8_t>(sec::runtime_error)) { } else if (reason.code() == static_cast<uint8_t>(sec::malformed_message)) {
shutdown(status::inconsistent_data, to_string(reason)); shutdown(status::inconsistent_data, to_string(reason));
} else { } else {
shutdown(status::unexpected_condition, to_string(reason)); shutdown(status::unexpected_condition, to_string(reason));
......
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