Simplify serialization/parsing of network packets

.gitignore

@@ -14,3 +14,4 @@ cmake-build*/
 
 # Visual Studio project files
 .vs/
+out/

CMakeSettings.json

@@ -0,0 +1,15 @@
+{
+  "configurations": [
+    {
+      "name": "x64-Debug",
+      "generator": "Ninja",
+      "configurationType": "Debug",
+      "inheritEnvironments": [ "msvc_x64_x64" ],
+      "buildRoot": "${projectDir}\\out\\build\\${name}",
+      "installRoot": "${projectDir}\\out\\install\\${name}",
+      "cmakeCommandArgs": "",
+      "buildCommandArgs": "",
+      "ctestCommandArgs": ""
+    }
+  ]
+}

client/lib/client.cpp

@@ -29,7 +29,7 @@ struct MumbleClient::Impl final {
 
 	asio::steady_timer m_pingTimer;
 
-	std::array<std::byte, common::maxPacketLength> m_receiveBuffer;
+	std::array<std::byte, common::kMaxPacketLength> m_receiveBuffer;
 
 	Impl(std::string_view serverName, uint16_t port, std::string_view userName, bool validateServerCertificate)
 		: m_tlsContext(asio::ssl::context_base::tlsv13_client), m_tlsSocket(m_ioContext, m_tlsContext),
@@ -58,7 +58,7 @@ struct MumbleClient::Impl final {
 			pingTimerCompletionHandler();
 		});
 
-		asio::async_read(m_tlsSocket, asio::buffer(m_receiveBuffer), asio::transfer_at_least(common::headerLength),
+		asio::async_read(m_tlsSocket, asio::buffer(m_receiveBuffer), asio::transfer_at_least(common::kHeaderLength),
 						 [this](const std::error_code &error, std::size_t bytes_transferred) {
 							 readCompletionHandler(error, bytes_transferred);
 						 });
@@ -120,7 +120,7 @@ struct MumbleClient::Impl final {
 			case common::PacketType::SuggestConfig: not_implemented(); break;
 		}
 
-		asio::async_read(m_tlsSocket, asio::buffer(m_receiveBuffer), asio::transfer_at_least(common::headerLength),
+		asio::async_read(m_tlsSocket, asio::buffer(m_receiveBuffer), asio::transfer_at_least(common::kHeaderLength),
 						 [this](const std::error_code &error, std::size_t bytes_transferred) {
 							 readCompletionHandler(error, bytes_transferred);
 						 });

common/src/packet.cpp

@@ -13,26 +13,28 @@
 namespace libmumble_protocol::common {
 
 std::tuple<PacketType, std::span<const std::byte>>
-parseNetworkBuffer(std::span<const std::byte, maxPacketLength> buffer) {
+parseNetworkBuffer(std::span<const std::byte, kMaxPacketLength> buffer) {
 
-	const auto packetType = static_cast<PacketType>(readIntegerFromNetworkBuffer<std::uint16_t>(buffer.first<2>()));
-	const auto payloadLength = readIntegerFromNetworkBuffer<std::uint32_t>(buffer.subspan<2, 4>());
+	const auto header = std::bit_cast<Header>(buffer.subspan<kHeaderLength>());
 
-	return {packetType, buffer.subspan(headerLength, payloadLength)};
+	const auto packetType = static_cast<PacketType>(swapNetworkBytes(header.packet_type));
+	const auto payloadLength = swapNetworkBytes(header.payload_length);
+
+	return {packetType, buffer.subspan(kHeaderLength, payloadLength)};
 }
 
 std::vector<std::byte> MumbleControlPacket::serialize() const {
 
 	const auto packetType = this->packetType();
 	const auto &message = this->message();
 	const auto payloadBytes = message.ByteSizeLong();
+	const Header header{swapNetworkBytes(std::to_underlying(packetType)),
+						static_cast<uint32_t>(swapNetworkBytes(payloadBytes))};
 
-	std::vector<std::byte> buffer(headerLength + payloadBytes);
-	auto bufferBegin = std::begin(buffer);
-	writeIntegerToNetworkBuffer(std::span(bufferBegin, 2), static_cast<std::uint16_t>(packetType));
-	writeIntegerToNetworkBuffer(std::span(bufferBegin + 2, 4), static_cast<std::uint32_t>(payloadBytes));
-
-	message.SerializeToArray(buffer.data() + headerLength, static_cast<int>(payloadBytes));
+	std::vector<std::byte> buffer(kHeaderLength + payloadBytes);
+	std::byte *data = buffer.data();
+	std::memcpy(data, &header, kHeaderLength);
+	message.SerializeToArray(data + kHeaderLength, static_cast<int>(payloadBytes));
 
 	return buffer;
 }

common/src/packet.hpp

@@ -16,20 +16,25 @@
 
 namespace libmumble_protocol::common {
 
+MUMBLE_PROTOCOL_COMMON_EXPORT struct Header {
+	const std::uint16_t packet_type;
+	const std::uint32_t payload_length;
+};
+
 /**
  * Maximum length of the payload part of the packet according to the specification.
  */
-MUMBLE_PROTOCOL_COMMON_EXPORT constexpr std::size_t maxPayloadLength = 8 * 1024 * 1024 - 1;
+MUMBLE_PROTOCOL_COMMON_EXPORT constexpr std::size_t kMaxPayloadLength = 8 * 1024 * 1024 - 1;
 
 /**
  * Length of the packet header.
  */
-MUMBLE_PROTOCOL_COMMON_EXPORT constexpr std::size_t headerLength = 2 + 4;
+MUMBLE_PROTOCOL_COMMON_EXPORT constexpr std::size_t kHeaderLength = 2 + 4;
 
 /**
  * Maximum length of the entire packet (header + payload).
  */
-MUMBLE_PROTOCOL_COMMON_EXPORT constexpr std::size_t maxPacketLength = headerLength + maxPayloadLength;
+MUMBLE_PROTOCOL_COMMON_EXPORT constexpr std::size_t kMaxPacketLength = kHeaderLength + kMaxPayloadLength;
 
 /**
  * All defined packet types.
@@ -64,7 +69,7 @@ enum struct MUMBLE_PROTOCOL_COMMON_EXPORT PacketType : uint16_t {
 };
 
 MUMBLE_PROTOCOL_COMMON_EXPORT std::tuple<PacketType, std::span<const std::byte>>
-	parseNetworkBuffer(std::span<const std::byte, maxPacketLength>);
+	parseNetworkBuffer(std::span<const std::byte, kMaxPacketLength>);
 
 class MUMBLE_PROTOCOL_COMMON_EXPORT MumbleControlPacket {
    public:
@@ -101,18 +106,26 @@ struct MUMBLE_PROTOCOL_COMMON_EXPORT MumbleVersionPacket : public MumbleControlP
 	MumbleVersionPacket(std::uint16_t majorVersion, std::uint8_t minorVersion, std::uint8_t patchVersion,
 						std::string_view release, std::string_view operatingSystem,
 						std::string_view operatingSystemVersion);
+
 	explicit MumbleVersionPacket(std::span<const std::byte>);
+
 	~MumbleVersionPacket() override = default;
 
 	std::uint16_t majorVersion() const { return m_numericVersion.major; }
+
 	std::uint8_t minorVersion() const { return m_numericVersion.minor; }
+
 	std::uint8_t patchVersion() const { return m_numericVersion.patch; }
+
 	std::string_view release() const { return m_version.release(); }
+
 	std::string_view operatingSystem() const { return m_version.os(); }
+
 	std::string_view operatingSystemVersion() const { return m_version.os_version(); }
 
    protected:
 	PacketType packetType() const override;
+
 	google::protobuf::Message const &message() const override;
 
    private:
@@ -124,27 +137,34 @@ struct MUMBLE_PROTOCOL_COMMON_EXPORT MumbleAuthenticatePacket : public MumbleCon
 	MumbleAuthenticatePacket(std::string_view username, std::string_view password,
 							 const std::vector<std::string_view> &tokens, const std::vector<std::int32_t> &celtVersions,
 							 bool opusSupported);
+
 	explicit MumbleAuthenticatePacket(std::span<const std::byte>);
+
 	~MumbleAuthenticatePacket() override = default;
 
 	std::string_view username() const { return m_authenticate.username(); }
+
 	std::string_view password() const { return m_authenticate.password(); }
+
 	std::vector<std::string_view> tokens() const {
 		std::vector<std::string_view> result;
 		result.reserve(m_authenticate.tokens_size());
 		for (const auto &token : m_authenticate.tokens()) { result.emplace_back(token); }
 		return result;
 	};
+
 	std::vector<std::int32_t> celtVersions() const {
 		std::vector<std::int32_t> result;
 		result.reserve(m_authenticate.celt_versions_size());
 		for (const auto celtVersion : m_authenticate.celt_versions()) { result.push_back(celtVersion); }
 		return result;
 	}
+
 	bool opusSupported() const { return m_authenticate.opus(); }
 
    protected:
 	PacketType packetType() const override;
+
 	google::protobuf::Message const &message() const override;
 
    private:
@@ -153,13 +173,16 @@ struct MUMBLE_PROTOCOL_COMMON_EXPORT MumbleAuthenticatePacket : public MumbleCon
 
 struct MUMBLE_PROTOCOL_COMMON_EXPORT MumblePingPacket : public MumbleControlPacket {
 	explicit MumblePingPacket(std::uint64_t);
+
 	MumblePingPacket(std::uint64_t timestamp, std::uint32_t good, std::uint32_t late, std::uint32_t lost,
 					 std::uint32_t reSync, std::uint32_t udpPackets, std::uint32_t tcpPackets, float udpPingAverage,
 					 float udpPingVariation, float tcpPingAverage, float tcpPingVariation);
+
 	explicit MumblePingPacket(std::span<const std::byte>);
 
    protected:
 	PacketType packetType() const override;
+
 	const google::protobuf::Message &message() const override;
 
    private:
@@ -169,14 +192,18 @@ struct MUMBLE_PROTOCOL_COMMON_EXPORT MumblePingPacket : public MumbleControlPack
 struct MumbleCryptographySetupPacket : public MumbleControlPacket {
 	MumbleCryptographySetupPacket(std::span<const std::byte> &key, std::span<const std::byte> &clientNonce,
 								  std::span<const std::byte> &serverNonce);
+
 	explicit MumbleCryptographySetupPacket(std::span<const std::byte>);
 
 	std::span<const std::byte> key() const { return as_bytes(std::span(m_cryptSetup.key())); }
+
 	std::span<const std::byte> clientNonce() const { return as_bytes(std::span(m_cryptSetup.client_nonce())); }
+
 	std::span<const std::byte> serverNonce() const { return as_bytes(std::span(m_cryptSetup.server_nonce())); }
 
    protected:
 	PacketType packetType() const override;
+
 	const google::protobuf::Message &message() const override;
 
    private:

common/src/util.hpp

@@ -25,28 +25,6 @@ MUMBLE_PROTOCOL_COMMON_EXPORT auto swapNetworkBytes(std::integral auto const i)
 	}
 }
 
-template<typename T>
-	requires(std::integral<T>)
-MUMBLE_PROTOCOL_COMMON_EXPORT T readIntegerFromNetworkBuffer(std::span<const std::byte, sizeof(T)> buffer) {
-	T result;
-	std::memcpy(&result, buffer.data(), sizeof(T));
-
-	return swapNetworkBytes(result);
-}
-
-MUMBLE_PROTOCOL_COMMON_EXPORT std::size_t writeIntegerToNetworkBuffer(std::span<std::byte> buffer,
-																	  std::integral auto const value) {
-	const auto valueSize = sizeof(value);
-	if (std::size(buffer) < valueSize) {
-		throw std::range_error("Buffer too small for value " + std::to_string(value)
-							   + ". Buffer size: " + std::to_string(std::size(buffer)));
-	}
-
-	auto temp = swapNetworkBytes(value);
-	std::memcpy(buffer.data(), &temp, valueSize);
-	return valueSize;
-}
-
 MUMBLE_PROTOCOL_COMMON_EXPORT std::tuple<std::size_t, std::int64_t>
 decodeVariableInteger(std::span<const std::byte> buffer);
 

common/test/util.cpp

@@ -8,108 +8,6 @@
 
 #include <catch2/catch_test_macros.hpp>
 
-TEST_CASE("Test the readIntegerFromNetworkBuffer function", "[common]") {
-
-	SECTION("Read uint8_t") {
-
-		const std::uint8_t expected = 0xff;
-		const auto data = std::array<const std::byte, sizeof(std::uint8_t)>{std::byte{0xff}};
-
-		const auto result = libmumble_protocol::common::readIntegerFromNetworkBuffer<std::uint8_t>(data);
-
-		REQUIRE(sizeof(result) == sizeof(expected));
-		REQUIRE(result == expected);
-	}
-
-	SECTION("Read uint16_t") {
-
-		const std::uint16_t expected = 0xff00;
-		const auto data = std::array<const std::byte, sizeof(std::uint16_t)>{std::byte{0xff}, std::byte{0x00}};
-
-		const auto result = libmumble_protocol::common::readIntegerFromNetworkBuffer<std::uint16_t>(data);
-
-		REQUIRE(sizeof(result) == sizeof(expected));
-		REQUIRE(result == expected);
-	}
-
-	SECTION("Read uint32_t") {
-
-		const std::uint32_t expected = 0xdeadff00;
-		const auto data = std::array<const std::byte, sizeof(std::uint32_t)>{std::byte{0xde}, std::byte{0xad},
-																			 std::byte{0xff}, std::byte{0x00}};
-
-		const auto result = libmumble_protocol::common::readIntegerFromNetworkBuffer<std::uint32_t>(data);
-
-		REQUIRE(sizeof(result) == sizeof(expected));
-		REQUIRE(result == expected);
-	}
-
-	SECTION("Read uint64_t") {
-
-		const std::uint64_t expected = 0xcafebabe'deadf0f0;
-		const auto data = std::array<const std::byte, sizeof(std::uint64_t)>{
-			std::byte{0xca}, std::byte{0xfe}, std::byte{0xba}, std::byte{0xbe},
-			std::byte{0xde}, std::byte{0xad}, std::byte{0xf0}, std::byte{0xf0}};
-
-		const auto result = libmumble_protocol::common::readIntegerFromNetworkBuffer<std::uint64_t>(data);
-
-		REQUIRE(sizeof(result) == sizeof(expected));
-		REQUIRE(result == expected);
-	}
-}
-
-TEST_CASE("Test the writeIntegerToNetworkBuffer function", "[common]") {
-
-	SECTION("Write uint8_t") {
-
-		const std::uint8_t input = 0xff;
-		const auto expected = std::array{std::byte{0xff}};
-		std::array<std::byte, sizeof(std::uint8_t)> buffer{};
-
-		const auto result = libmumble_protocol::common::writeIntegerToNetworkBuffer(buffer, input);
-
-		REQUIRE(result == sizeof(input));
-		REQUIRE(buffer == expected);
-	}
-
-	SECTION("Write uint16_t") {
-
-		const std::uint16_t input = 0xff00;
-		const auto expected = std::array{std::byte{0xff}, std::byte{0x0}};
-		std::array<std::byte, sizeof(std::uint16_t)> buffer{};
-
-		const auto result = libmumble_protocol::common::writeIntegerToNetworkBuffer(buffer, input);
-
-		REQUIRE(result == sizeof(input));
-		REQUIRE(buffer == expected);
-	}
-
-	SECTION("Write uint32_t") {
-
-		const std::uint32_t input = 0xdeadff00;
-		const auto expected = std::array{std::byte{0xde}, std::byte{0xad}, std::byte{0xff}, std::byte{0x00}};
-		std::array<std::byte, sizeof(std::uint32_t)> buffer{};
-
-		const auto result = libmumble_protocol::common::writeIntegerToNetworkBuffer(buffer, input);
-
-		REQUIRE(result == sizeof(input));
-		REQUIRE(buffer == expected);
-	}
-
-	SECTION("Write uint64_t") {
-
-		const std::uint64_t input = 0xcafebabe'deadf0f0;
-		const auto expected = std::array{std::byte{0xca}, std::byte{0xfe}, std::byte{0xba}, std::byte{0xbe},
-										 std::byte{0xde}, std::byte{0xad}, std::byte{0xf0}, std::byte{0xf0}};
-		std::array<std::byte, sizeof(std::uint64_t)> buffer{};
-
-		const auto result = libmumble_protocol::common::writeIntegerToNetworkBuffer(buffer, input);
-
-		REQUIRE(result == sizeof(input));
-		REQUIRE(buffer == expected);
-	}
-}
-
 TEST_CASE("Test the mumble protocol variable integer decode function", "[common]") {
 
 	SECTION("Decode single byte") {