Commit b1151251 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 11998e7b
...@@ -57,7 +57,7 @@ ptrdiff_t framing::consume(byte_span buffer, byte_span) { ...@@ -57,7 +57,7 @@ ptrdiff_t framing::consume(byte_span buffer, byte_span) {
detail::rfc6455::mask_data(hdr.mask_key, payload); detail::rfc6455::mask_data(hdr.mask_key, payload);
} }
// Handle control frames first, since these may not me fragmented, // Handle control frames first, since these may not me fragmented,
// and can come up in between regular message fragments // and can arrive between regular message fragments.
if (detail::rfc6455::is_control_frame(hdr.opcode) if (detail::rfc6455::is_control_frame(hdr.opcode)
&& hdr.opcode != detail::rfc6455::continuation_frame) { && hdr.opcode != detail::rfc6455::continuation_frame) {
if (!hdr.fin) { if (!hdr.fin) {
...@@ -67,8 +67,8 @@ ptrdiff_t framing::consume(byte_span buffer, byte_span) { ...@@ -67,8 +67,8 @@ ptrdiff_t framing::consume(byte_span buffer, byte_span) {
} }
return handle(hdr.opcode, payload, frame_size); return handle(hdr.opcode, payload, frame_size);
} }
// Reject any payload that exceed max_frame_size. This covers assembled // Reject any payload that exceeds max_frame_size. This covers assembled
// payloads as well by including payload_buf_ // payloads as well by including payload_buf_.
if (payload_buf_.size() + payload_len > max_frame_size) { if (payload_buf_.size() + payload_len > max_frame_size) {
CAF_LOG_DEBUG("fragmented WebSocket payload exceeds maximum size"); CAF_LOG_DEBUG("fragmented WebSocket payload exceeds maximum size");
abort_and_shutdown(sec::protocol_error, "fragmented WebSocket payload " abort_and_shutdown(sec::protocol_error, "fragmented WebSocket payload "
......
...@@ -15,11 +15,15 @@ using namespace std::literals; ...@@ -15,11 +15,15 @@ using namespace std::literals;
namespace { namespace {
struct fixture { struct fixture {
mock_web_socket_app* app; mock_web_socket_app* app = nullptr;
net::web_socket::framing* uut; net::web_socket::framing* uut = nullptr;
std::unique_ptr<mock_stream_transport> transport; std::unique_ptr<mock_stream_transport> transport;
fixture() { fixture() {
reset();
}
void reset() {
auto app_layer = mock_web_socket_app::make(); auto app_layer = mock_web_socket_app::make();
app = app_layer.get(); app = app_layer.get();
auto uut_layer auto uut_layer
...@@ -163,7 +167,7 @@ SCENARIO("the client closes the connection with a closing handshake") { ...@@ -163,7 +167,7 @@ SCENARIO("the client closes the connection with a closing handshake") {
} }
} }
SCENARIO("the client sends a fragmented ping that fails the connection") { SCENARIO("ping messages may not be fragmented") {
GIVEN("a valid WebSocket connection") { GIVEN("a valid WebSocket connection") {
std::vector<std::byte> ping_frame; std::vector<std::byte> ping_frame;
WHEN("the client sends the first frame of a fragmented ping message") { WHEN("the client sends the first frame of a fragmented ping message") {
...@@ -192,12 +196,10 @@ SCENARIO("the client sends a fragmented ping that fails the connection") { ...@@ -192,12 +196,10 @@ SCENARIO("the client sends a fragmented ping that fails the connection") {
} }
} }
SCENARIO("the client sends a fragmented text message with a ping in-between") { SCENARIO("ping messages may arrive between message fragments") {
GIVEN("a valid WebSocket connection") { GIVEN("a valid WebSocket connection") {
WHEN("the client sends the first text frame, a ping, and the final text " WHEN("the frames arrive all at once") {
"frame at once") {
std::vector<std::byte> input; std::vector<std::byte> input;
auto fragment1 = "Hello"sv; auto fragment1 = "Hello"sv;
auto fragment2 = ", world!"sv; auto fragment2 = ", world!"sv;
auto data = as_bytes(make_span(fragment1)); auto data = as_bytes(make_span(fragment1));
...@@ -224,21 +226,15 @@ SCENARIO("the client sends a fragmented text message with a ping in-between") { ...@@ -224,21 +226,15 @@ SCENARIO("the client sends a fragmented text message with a ping in-between") {
CHECK_EQ(hdr.payload_len, 5u); CHECK_EQ(hdr.payload_len, 5u);
CHECK_EQ(hdr.mask_key, 0u); CHECK_EQ(hdr.mask_key, 0u);
} }
THEN("the server receives the full text message") { AND("the server receives the full text message") {
CHECK_EQ(app->text_input, "Hello, world!"sv); CHECK_EQ(app->text_input, "Hello, world!"sv);
} }
AND("the client did not abort") { AND("the app is still running") {
CHECK(!app->has_aborted()); CHECK(!app->has_aborted());
} }
} }
} WHEN("the frames arrive separately") {
} reset();
SCENARIO("the client sends an fragmented text message with a ping in-between "
"separated by octets") {
GIVEN("a valid WebSocket connection") {
WHEN("the client sends the first text frame, a ping, and then the final "
"text frame separately") {
auto fragment1 = "Hello"sv; auto fragment1 = "Hello"sv;
auto fragment2 = ", world!"sv; auto fragment2 = ", world!"sv;
std::vector<std::byte> input; std::vector<std::byte> input;
...@@ -283,6 +279,8 @@ SCENARIO("the client sends an fragmented text message with a ping in-between " ...@@ -283,6 +279,8 @@ SCENARIO("the client sends an fragmented text message with a ping in-between "
END_FIXTURE_SCOPE() END_FIXTURE_SCOPE()
namespace {
// The following setup is a mock websocket application that rejects everything // The following setup is a mock websocket application that rejects everything
struct rejecting_mock_web_socket_app : public mock_web_socket_app { struct rejecting_mock_web_socket_app : public mock_web_socket_app {
...@@ -299,11 +297,11 @@ struct rejecting_mock_web_socket_app : public mock_web_socket_app { ...@@ -299,11 +297,11 @@ struct rejecting_mock_web_socket_app : public mock_web_socket_app {
}; };
struct rejecting_fixture { struct rejecting_fixture {
rejecting_mock_web_socket_app* app; rejecting_mock_web_socket_app* app = nullptr;
net::web_socket::framing* uut; net::web_socket::framing* uut = nullptr;
std::unique_ptr<mock_stream_transport> transport; std::unique_ptr<mock_stream_transport> transport;
rejecting_fixture() { void reset() {
auto app_layer = std::make_unique<rejecting_mock_web_socket_app>(); auto app_layer = std::make_unique<rejecting_mock_web_socket_app>();
app = app_layer.get(); app = app_layer.get();
auto uut_layer auto uut_layer
...@@ -315,53 +313,70 @@ struct rejecting_fixture { ...@@ -315,53 +313,70 @@ struct rejecting_fixture {
} }
}; };
BEGIN_FIXTURE_SCOPE(rejecting_fixture); } // namespace
TEST_CASE("send a binary message that gets rejected") {
byte_buffer input;
const auto data = make_test_data(4);
detail::rfc6455::assemble_frame(0x0, data, input);
transport->push(input);
CHECK_EQ(transport->handle_input(), 0);
CHECK(app->has_aborted());
}
TEST_CASE("send a framed binary message gets rejected") {
byte_buffer frame1;
byte_buffer frame2;
const auto data = make_test_data(4);
detail::rfc6455::assemble_frame(detail::rfc6455::binary_frame, 0x0, data,
frame1, 0);
transport->push(frame1);
detail::rfc6455::assemble_frame(detail::rfc6455::continuation_frame, 0x0,
data, frame2);
transport->push(frame2);
// We only handle one frame, the second one results in an abort
CHECK_EQ(transport->handle_input(), static_cast<ptrdiff_t>(frame1.size()));
CHECK(app->has_aborted());
}
TEST_CASE("send a text message that gets rejected") { BEGIN_FIXTURE_SCOPE(rejecting_fixture);
byte_buffer input;
const auto msg = "Hello, world!"sv;
detail::rfc6455::assemble_frame(0x0, make_span(msg), input);
transport->push(input);
CHECK_EQ(transport->handle_input(), 0);
CHECK(app->has_aborted());
}
TEST_CASE("send a framed text message gets rejected") { SCENARIO("apps can return errors to shut down the framing layer") {
byte_buffer frame1; GIVEN("an app that returns -1 for any frame it receives") {
byte_buffer frame2; WHEN("receiving a binary message") {
const auto msg = "Hello, world!"sv; THEN("the framing layer closes the connection") {
detail::rfc6455::assemble_frame(detail::rfc6455::text_frame, 0x0, reset();
as_bytes(make_span(msg)), frame1, 0); byte_buffer input;
transport->push(frame1); const auto data = make_test_data(4);
detail::rfc6455::assemble_frame(detail::rfc6455::continuation_frame, 0x0, detail::rfc6455::assemble_frame(0x0, data, input);
as_bytes(make_span(msg)), frame2); transport->push(input);
transport->push(frame2); CHECK_EQ(transport->handle_input(), 0);
CHECK_EQ(transport->handle_input(), static_cast<ptrdiff_t>(frame1.size())); CHECK(app->has_aborted());
CHECK(app->has_aborted()); }
}
WHEN("receiving a fragmented binary message") {
THEN("the framing layer closes the connection") {
reset();
byte_buffer frame1;
byte_buffer frame2;
const auto data = make_test_data(4);
detail::rfc6455::assemble_frame(detail::rfc6455::binary_frame, 0x0,
data, frame1, 0);
transport->push(frame1);
detail::rfc6455::assemble_frame(detail::rfc6455::continuation_frame,
0x0, data, frame2);
transport->push(frame2);
// We only handle one frame, the second one results in an abort
CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(frame1.size()));
CHECK(app->has_aborted());
}
}
WHEN("receiving a text message") {
THEN("the framing layer closes the connection") {
reset();
byte_buffer input;
const auto msg = "Hello, world!"sv;
detail::rfc6455::assemble_frame(0x0, make_span(msg), input);
transport->push(input);
CHECK_EQ(transport->handle_input(), 0);
CHECK(app->has_aborted());
}
}
WHEN("receiving a fragmented text message") {
THEN("the framing layer closes the connection") {
reset();
byte_buffer frame1;
byte_buffer frame2;
const auto msg = "Hello, world!"sv;
detail::rfc6455::assemble_frame(detail::rfc6455::text_frame, 0x0,
as_bytes(make_span(msg)), frame1, 0);
transport->push(frame1);
detail::rfc6455::assemble_frame(detail::rfc6455::continuation_frame,
0x0, as_bytes(make_span(msg)), frame2);
transport->push(frame2);
CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(frame1.size()));
CHECK(app->has_aborted());
}
}
}
} }
END_FIXTURE_SCOPE(); END_FIXTURE_SCOPE();
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