Commit cc1374ff authored by Samir Halilčević's avatar Samir Halilčević Committed by Samir Halilcevic

Integrate review feedback

Co-authored-by: default avatarDominik Charousset <dominik@charousset.de>
parent e60222ff
...@@ -58,8 +58,9 @@ struct CAF_NET_EXPORT rfc6455 { ...@@ -58,8 +58,9 @@ struct CAF_NET_EXPORT rfc6455 {
static ptrdiff_t decode_header(const_byte_span data, header& hdr); static ptrdiff_t decode_header(const_byte_span data, header& hdr);
static bool is_control_frame(uint8_t opcode); static constexpr bool is_control_frame(uint8_t opcode) noexcept {
return opcode != text_frame && opcode != binary_frame;
}
}; };
} // namespace caf::detail } // namespace caf::detail
...@@ -85,9 +85,8 @@ ptrdiff_t rfc6455::decode_header(const_byte_span data, header& hdr) { ...@@ -85,9 +85,8 @@ ptrdiff_t rfc6455::decode_header(const_byte_span data, header& hdr) {
auto len_field = byte2 & 0x7F; auto len_field = byte2 & 0x7F;
size_t header_length; size_t header_length;
// control frames can only have payload up to 125 bytes // control frames can only have payload up to 125 bytes
if (rfc6455::is_control_frame(hdr.opcode) && len_field > 125) { if (rfc6455::is_control_frame(hdr.opcode) && len_field > 125)
return -1; return -1;
}
if (len_field < 126) { if (len_field < 126) {
header_length = 2 + (masked ? 4 : 0); header_length = 2 + (masked ? 4 : 0);
hdr.payload_len = len_field; hdr.payload_len = len_field;
...@@ -139,15 +138,4 @@ ptrdiff_t rfc6455::decode_header(const_byte_span data, header& hdr) { ...@@ -139,15 +138,4 @@ ptrdiff_t rfc6455::decode_header(const_byte_span data, header& hdr) {
} }
} }
bool rfc6455::is_control_frame(uint8_t opcode) {
switch (opcode) {
case ping:
case pong:
case connection_close:
case continuation_frame:
return true;
}
return false;
}
} // namespace caf::detail } // namespace caf::detail
...@@ -27,7 +27,7 @@ public: ...@@ -27,7 +27,7 @@ public:
bool aborted = false; bool aborted = false;
net::web_socket::lower_layer* down; net::web_socket::lower_layer* down = nullptr;
error start(net::web_socket::lower_layer* ll) override { error start(net::web_socket::lower_layer* ll) override {
down = ll; down = ll;
...@@ -71,13 +71,12 @@ struct fixture { ...@@ -71,13 +71,12 @@ struct fixture {
= net::web_socket::framing::make_server(std::move(app_layer)); = net::web_socket::framing::make_server(std::move(app_layer));
uut = uut_layer.get(); uut = uut_layer.get();
transport = mock_stream_transport::make(std::move(uut_layer)); transport = mock_stream_transport::make(std::move(uut_layer));
CHECK_EQ(transport->start(nullptr), error{});
transport->configure_read(caf::net::receive_policy::up_to(2048u));
} }
static const_byte_span make_test_data(uint64_t requested_size) { byte_buffer make_test_data(size_t requested_size) {
static std::vector<std::byte> bytes{150, std::byte{0xFF}}; return byte_buffer{requested_size, std::byte{0xFF}};
if (requested_size > bytes.size())
bytes.resize(requested_size, std::byte{0xFF});
return const_byte_span{bytes.data(), requested_size};
} }
}; };
...@@ -89,8 +88,6 @@ SCENARIO("the client sends a ping and receives a pong response") { ...@@ -89,8 +88,6 @@ SCENARIO("the client sends a ping and receives a pong response") {
GIVEN("a valid WebSocket connection") { GIVEN("a valid WebSocket connection") {
std::vector<std::byte> ping_frame; std::vector<std::byte> ping_frame;
std::vector<std::byte> pong_frame; std::vector<std::byte> pong_frame;
CHECK_EQ(transport->start(nullptr), error{});
transport->configure_read(caf::net::receive_policy::up_to(2048u));
WHEN("the client sends a ping with no data") { WHEN("the client sends a ping with no data") {
auto data = make_test_data(0); auto data = make_test_data(0);
detail::rfc6455::assemble_frame(detail::rfc6455::ping, 0x0, data, detail::rfc6455::assemble_frame(detail::rfc6455::ping, 0x0, data,
...@@ -98,7 +95,7 @@ SCENARIO("the client sends a ping and receives a pong response") { ...@@ -98,7 +95,7 @@ SCENARIO("the client sends a ping and receives a pong response") {
transport->push(ping_frame); transport->push(ping_frame);
CHECK_EQ(transport->handle_input(), CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(ping_frame.size())); static_cast<ptrdiff_t>(ping_frame.size()));
THEN("the client receives an empty pong") { THEN("the server responds with an empty pong") {
detail::rfc6455::assemble_frame(detail::rfc6455::pong, 0x0, data, detail::rfc6455::assemble_frame(detail::rfc6455::pong, 0x0, data,
pong_frame); pong_frame);
CHECK_EQ(transport->output_buffer(), pong_frame); CHECK_EQ(transport->output_buffer(), pong_frame);
...@@ -112,21 +109,21 @@ SCENARIO("the client sends a ping and receives a pong response") { ...@@ -112,21 +109,21 @@ SCENARIO("the client sends a ping and receives a pong response") {
transport->push(ping_frame); transport->push(ping_frame);
CHECK_EQ(transport->handle_input(), CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(ping_frame.size())); static_cast<ptrdiff_t>(ping_frame.size()));
THEN("the client receives a pong containing the data echoed back") { THEN("the server echoes the data back to the client") {
detail::rfc6455::assemble_frame(detail::rfc6455::pong, 0x0, data, detail::rfc6455::assemble_frame(detail::rfc6455::pong, 0x0, data,
pong_frame); pong_frame);
CHECK_EQ(transport->output_buffer(), pong_frame); CHECK_EQ(transport->output_buffer(), pong_frame);
} }
} }
transport->output_buffer().clear(); transport->output_buffer().clear();
WHEN("the client sends a ping with the maximum amount of data") { WHEN("the client sends a ping with the maximum allowed 125 bytes") {
auto data = make_test_data(125); auto data = make_test_data(125);
detail::rfc6455::assemble_frame(detail::rfc6455::ping, 0x0, data, detail::rfc6455::assemble_frame(detail::rfc6455::ping, 0x0, data,
ping_frame); ping_frame);
transport->push(ping_frame); transport->push(ping_frame);
CHECK_EQ(transport->handle_input(), CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(ping_frame.size())); static_cast<ptrdiff_t>(ping_frame.size()));
THEN("the client receives a pong containing the data echoed back") { THEN("the server echoes the data back to the client") {
detail::rfc6455::assemble_frame(detail::rfc6455::pong, 0x0, data, detail::rfc6455::assemble_frame(detail::rfc6455::pong, 0x0, data,
pong_frame); pong_frame);
CHECK_EQ(transport->output_buffer(), pong_frame); CHECK_EQ(transport->output_buffer(), pong_frame);
...@@ -136,46 +133,40 @@ SCENARIO("the client sends a ping and receives a pong response") { ...@@ -136,46 +133,40 @@ SCENARIO("the client sends a ping and receives a pong response") {
} }
TEST_CASE("calling shutdown with protocol_error sets status in close header") { TEST_CASE("calling shutdown with protocol_error sets status in close header") {
CHECK_EQ(transport->start(nullptr), error{});
transport->configure_read(caf::net::receive_policy::up_to(2048u));
uut->shutdown(make_error(sec::protocol_error)); uut->shutdown(make_error(sec::protocol_error));
detail::rfc6455::header hdr; detail::rfc6455::header hdr;
detail::rfc6455::decode_header(transport->output_buffer(), hdr); detail::rfc6455::decode_header(transport->output_buffer(), hdr);
CHECK_EQ(hdr.opcode, detail::rfc6455::connection_close); CHECK_EQ(hdr.opcode, detail::rfc6455::connection_close);
CHECK(hdr.payload_len >= 2); CHECK(hdr.payload_len >= 2);
auto error_code = (static_cast<uint16_t>(transport->output_buffer()[2]) << 8) auto status = (std::to_integer<int>(transport->output_buffer().at(2)) << 8)
+ static_cast<uint16_t>(transport->output_buffer()[3]); + std::to_integer<int>(transport->output_buffer().at(3));
CHECK_EQ(error_code, CHECK_EQ(status, static_cast<int>(net::web_socket::status::protocol_error));
static_cast<uint16_t>(net::web_socket::status::protocol_error));
} }
SCENARIO("the client sends an invalid ping that closes the connection") { SCENARIO("the client sends an invalid ping that closes the connection") {
GIVEN("a valid WebSocket connection") { GIVEN("a valid WebSocket connection") {
std::vector<std::byte> ping_frame; std::vector<std::byte> ping_frame;
CHECK_EQ(transport->start(nullptr), error{});
transport->configure_read(caf::net::receive_policy::up_to(2048u));
WHEN("the client sends a ping with more data than allowed") { WHEN("the client sends a ping with more data than allowed") {
auto data = make_test_data(126); auto data = make_test_data(126);
detail::rfc6455::assemble_frame(detail::rfc6455::ping, 0x0, data, detail::rfc6455::assemble_frame(detail::rfc6455::ping, 0x0, data,
ping_frame); ping_frame);
transport->push(ping_frame); transport->push(ping_frame);
THEN("the server rejects and closes the connection") { THEN("the server aborts the application") {
CHECK_EQ(transport->handle_input(), 0); CHECK_EQ(transport->handle_input(), 0);
CHECK(app->aborted); CHECK(app->aborted);
CHECK_EQ(app->err, sec::protocol_error); CHECK_EQ(app->err, sec::protocol_error);
MESSAGE("Aborted with: " << app->err); MESSAGE("Aborted with: " << app->err);
} }
AND("shuts down the connection with a close header") { AND("the server closes the connection with a protocol error") {
detail::rfc6455::header hdr; detail::rfc6455::header hdr;
detail::rfc6455::decode_header(transport->output_buffer(), hdr); detail::rfc6455::decode_header(transport->output_buffer(), hdr);
MESSAGE("Buffer: " << transport->output_buffer()); MESSAGE("Buffer: " << transport->output_buffer());
CHECK_EQ(hdr.opcode, detail::rfc6455::connection_close); CHECK_EQ(hdr.opcode, detail::rfc6455::connection_close);
CHECK(hdr.payload_len >= 2); CHECK(hdr.payload_len >= 2);
auto error_code auto status = (std::to_integer<int>(transport->output_buffer()[2]) << 8)
= (static_cast<uint16_t>(transport->output_buffer()[2]) << 8) + std::to_integer<int>(transport->output_buffer()[3]);
+ static_cast<uint16_t>(transport->output_buffer()[3]); CHECK_EQ(status,
CHECK_EQ(error_code, static_cast<uint16_t>( static_cast<int>(net::web_socket::status::protocol_error));
net::web_socket::status::protocol_error));
} }
} }
} }
......
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