From 8b8fb93a14184a6e9a46a9f255eb9320291de2c0 Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:00:47 +0200 Subject: [PATCH 01/12] build: fetch rnnoise with FetchContent_MakeAvailable FetchContent_Populate with declared details is deprecated (CMP0169) and slated for removal. Neither rnnoise nor its model archive has a CMakeLists.txt, so MakeAvailable just downloads them, as before. Also sets DOWNLOAD_EXTRACT_TIMESTAMP on the model archive (CMP0135). --- cmake/FetchVoiceDependencies.cmake | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/cmake/FetchVoiceDependencies.cmake b/cmake/FetchVoiceDependencies.cmake index ceaa0eafc..98bf69edb 100644 --- a/cmake/FetchVoiceDependencies.cmake +++ b/cmake/FetchVoiceDependencies.cmake @@ -45,24 +45,15 @@ FetchContent_Declare( rnnoise_model URL https://media.xiph.org/rnnoise/models/rnnoise_data-${RNNOISE_MODEL_VERSION}.tar.gz URL_HASH SHA256=4ac81c5c0884ec4bd5907026aaae16209b7b76cd9d7f71af582094a2f98f4b43 + DOWNLOAD_EXTRACT_TIMESTAMP TRUE ) # ----------------------------------------------------------------------------- # Fetch dependencies # ----------------------------------------------------------------------------- -FetchContent_MakeAvailable(Opus) - -# Fetch rnnoise separately (needs manual target creation) -FetchContent_GetProperties(rnnoise) -if(NOT rnnoise_POPULATED) - FetchContent_Populate(rnnoise) -endif() - -# Fetch rnnoise model data -FetchContent_GetProperties(rnnoise_model) -if(NOT rnnoise_model_POPULATED) - FetchContent_Populate(rnnoise_model) -endif() +# Neither rnnoise nor its model archive ships a CMakeLists.txt, so +# MakeAvailable only downloads them; the rnnoise target is created below. +FetchContent_MakeAvailable(Opus rnnoise rnnoise_model) # ----------------------------------------------------------------------------- # RNNoise - manual target creation (no upstream CMakeLists.txt) From fffc4c035f248d5955687b20deabf7b00c7a8e46 Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:21:45 +0200 Subject: [PATCH 02/12] fix(security): bound split-packet reassembly and BitStream length checks Found by the new libFuzzer harnesses (Tests/Fuzz) and by reading the code around what they hit. Every issue is reachable by any connected peer. ReliabilityLayer split packets (every header field comes from the remote): - The first fragment of a split message preallocated one pointer per claimed fragment: a single datagram claiming 2^32 fragments made the receiver allocate 32 GB (remote denial of service). - Later fragments were not checked against the first fragment's count, so a fragment claiming a larger count and index was written past the fragment table (heap overflow in Release; only a RakAssert in Debug). - Reassembly sized its buffer from the total bit length but copied each fragment at a byte offset, so non-final fragments that were not whole bytes overran it. - The summed message length was 32-bit and could wrap. - A fragment rejected (or a duplicate) was freed by InsertIntoSplitPacketList and then read by the caller to look up its message: use-after-free. SplitPacketSort now stores fragments as they arrive (memory follows what was received, not what was claimed), rejects fragments that disagree with the first one or that are non-final and not whole bytes, and reassembly sums lengths in 64 bits. Datagram parsing no longer RakAsserts on malformed input, so a Debug build cannot be aborted remotely; it drops the packet, including one claiming more data than a datagram can hold. Reassembly is ~8% slower in the in-memory microbenchmark (std::map vs a flat array). BitStream: - Read(char*, n) and ReadAlignedBytes compared against n << 3 in 32 bits; lengths of 512 MB and up wrapped, passed the check and memcpy'd past the stream. - ReadAlignedBytesSafeAlloc returned its buffer allocated but unfilled when the read failed; callers that free only on success leaked it. - ReadDelta / ReadCompressedDelta branched on an uninitialized flag when the read hit the end of the stream. --- Source/include/mafianet/bit_stream.h | 10 +- Source/include/mafianet/reliability_layer.h | 32 ++- Source/src/bit_stream.cpp | 18 +- Source/src/reliability_layer.cpp | 152 ++++++------ Tests/Unit/BitStreamBoundsTests.cpp | 93 ++++++++ .../Unit/ReliabilityLayerSplitPacketTests.cpp | 220 ++++++++++++++++++ 6 files changed, 421 insertions(+), 104 deletions(-) create mode 100644 Tests/Unit/BitStreamBoundsTests.cpp create mode 100644 Tests/Unit/ReliabilityLayerSplitPacketTests.cpp diff --git a/Source/include/mafianet/bit_stream.h b/Source/include/mafianet/bit_stream.h index 9edc3549f..096521567 100644 --- a/Source/include/mafianet/bit_stream.h +++ b/Source/include/mafianet/bit_stream.h @@ -1475,10 +1475,11 @@ namespace MafiaNet template inline bool BitStream::ReadDelta(templateType &outTemplateVar) { - bool dataWritten; + // dataWritten is unset when the read fails at the end of the stream. + bool dataWritten=false; bool success; success=Read(dataWritten); - if (dataWritten) + if (success && dataWritten) success=Read(outTemplateVar); return success; } @@ -1616,10 +1617,11 @@ namespace MafiaNet template inline bool BitStream::ReadCompressedDelta(templateType &outTemplateVar) { - bool dataWritten; + // dataWritten is unset when the read fails at the end of the stream. + bool dataWritten=false; bool success; success=Read(dataWritten); - if (dataWritten) + if (success && dataWritten) success=ReadCompressed(outTemplateVar); return success; } diff --git a/Source/include/mafianet/reliability_layer.h b/Source/include/mafianet/reliability_layer.h index 6ba63f15c..6145730bd 100644 --- a/Source/include/mafianet/reliability_layer.h +++ b/Source/include/mafianet/reliability_layer.h @@ -44,6 +44,7 @@ #include "mafianet/plugin_interface2.h" #include "mafianet/rand.h" #include "mafianet/socket2.h" +#include #if USE_SLIDING_WINDOW_CONGESTION_CONTROL!=1 #include "mafianet/cc_udt.h" @@ -67,37 +68,34 @@ class RNS2SendBatch; // mmsg_batch.h typedef uint64_t reliabilityHeapWeightType; // #med - consider a more suitable name for the class / maybe even make an internal class to SplitPacketChannel? +// The fragments of one split message, keyed by fragment index. +// +// Every field here comes from the remote. Storage grows with the fragments that +// actually arrive, never with the count a fragment claims, so a peer cannot make +// us allocate more than it sends. Add() rejects any fragment that disagrees with +// the first one about the fragment count, and any non-final fragment that is not +// a whole number of bytes: reassembly copies fragments at byte offsets. class SplitPacketSort { - // member variables private: - InternalPacket **m_data; - size_t m_allocationSize; - unsigned int m_addedPacketsCount; + std::map m_fragments; + SplitPacketIndexType m_count; SplitPacketIdType m_packetId; - // construction/destruction public: SplitPacketSort(); - ~SplitPacketSort(); - // initialization -public: - void Preallocate(InternalPacket *internalPacket, const char *file, unsigned int line); + // Records the message's id and fragment count from its first fragment. + void Init(const InternalPacket *firstFragment); - // accessors -public: bool AllPacketsAdded() const; - size_t GetAllocSize() const; unsigned int GetNumAddedPackets() const; SplitPacketIdType GetPacketId() const; - // operators -public: - InternalPacket*& operator[](size_t index); + // In index order. Once AllPacketsAdded(), indices are exactly 0..count-1. + const std::map &Fragments() const; - // container operations -public: + // false (packet not taken) for a duplicate or an inconsistent fragment. bool Add(InternalPacket *internalPacket); }; diff --git a/Source/src/bit_stream.cpp b/Source/src/bit_stream.cpp index 62e3855ae..ab5f600b7 100644 --- a/Source/src/bit_stream.cpp +++ b/Source/src/bit_stream.cpp @@ -263,11 +263,14 @@ bool BitStream::Read( BitStream &bitStream ) // Read an array or casted stream bool BitStream::Read( char* outByteArray, const unsigned int numberOfBytes ) { + // Compare wide: numberOfBytes << 3 wraps in 32 bits for lengths of 512 MB + // and up, and a wrapped check let memcpy read past the stream. + if ((uint64_t) numberOfBytes * 8 > GetNumberOfUnreadBits()) + return false; + // Optimization: if ((readOffset & 7) == 0) { - if (GetNumberOfUnreadBits() < (numberOfBytes << 3)) - return false; // Write the data memcpy( outByteArray, data + ( readOffset >> 3 ), (size_t) numberOfBytes ); @@ -372,7 +375,8 @@ bool BitStream::ReadAlignedBytes( unsigned char* inOutByteArray, const unsigned // Byte align AlignReadToByteBoundary(); - if (GetNumberOfUnreadBits() < (numberOfBytesToRead << 3)) + // Wide, as in Read(char*, unsigned int): a 32-bit shift wraps. + if ((uint64_t) numberOfBytesToRead * 8 > GetNumberOfUnreadBits()) return false; // Write the data @@ -411,7 +415,13 @@ bool BitStream::ReadAlignedBytesSafeAlloc( char ** outByteArray, unsigned int &i if (inputLength==0) return true; *outByteArray = (char*) rakMalloc_Ex( (size_t) inputLength, _FILE_AND_LINE_ ); - return ReadAlignedBytes((unsigned char*) *outByteArray, inputLength); + if (ReadAlignedBytes((unsigned char*) *outByteArray, inputLength)) + return true; + // Callers commonly free only on success; do not hand back a buffer with + // nothing read into it. + rakFree_Ex(*outByteArray, _FILE_AND_LINE_ ); + *outByteArray=0; + return false; } // Write numberToWrite bits from the input source diff --git a/Source/src/reliability_layer.cpp b/Source/src/reliability_layer.cpp index 79343711e..17a09b80d 100644 --- a/Source/src/reliability_layer.cpp +++ b/Source/src/reliability_layer.cpp @@ -242,72 +242,50 @@ static int waitFlag = -1; using namespace MafiaNet; SplitPacketSort::SplitPacketSort() : - m_data(nullptr), - m_allocationSize(0), - m_addedPacketsCount(0) + m_count(0), + m_packetId(0) { } -SplitPacketSort::~SplitPacketSort() +void SplitPacketSort::Init(const InternalPacket *firstFragment) { - if (m_allocationSize) { - OP_DELETE_ARRAY(m_data, _FILE_AND_LINE_); - } -} - -void SplitPacketSort::Preallocate(InternalPacket *internalPacket, const char *file, unsigned int line) -{ - RakAssert(m_data == nullptr); - m_allocationSize = internalPacket->splitPacketCount; - m_data = OP_NEW_ARRAY((int)m_allocationSize, file, line); - m_packetId = internalPacket->splitPacketId; - - for (size_t i = 0; i < m_allocationSize; ++i) { - m_data[i] = nullptr; - } + RakAssert(m_fragments.empty()); + m_count = firstFragment->splitPacketCount; + m_packetId = firstFragment->splitPacketId; } bool SplitPacketSort::AllPacketsAdded() const { - return GetAllocSize() == GetNumAddedPackets(); -} - -size_t SplitPacketSort::GetAllocSize() const -{ - return m_allocationSize; + return m_count > 0 && m_fragments.size() == m_count; } unsigned int SplitPacketSort::GetNumAddedPackets() const { - return m_addedPacketsCount; + return (unsigned int) m_fragments.size(); } SplitPacketIdType SplitPacketSort::GetPacketId() const { - RakAssert(m_data != nullptr); return m_packetId; } -InternalPacket*& SplitPacketSort::operator[](size_t index) +const std::map &SplitPacketSort::Fragments() const { - RakAssert(m_data != nullptr); - RakAssert(index < m_allocationSize); - return m_data[index]; + return m_fragments; } bool SplitPacketSort::Add(InternalPacket *internalPacket) { - RakAssert(m_data != nullptr); - RakAssert(internalPacket->splitPacketIndex < m_allocationSize); - RakAssert(m_packetId == internalPacket->splitPacketId); - RakAssert(m_data[internalPacket->splitPacketIndex] == nullptr); - - if (m_data[internalPacket->splitPacketIndex] == nullptr) { - m_data[internalPacket->splitPacketIndex] = internalPacket; - ++m_addedPacketsCount; - return true; - } - return false; + const SplitPacketIndexType index = internalPacket->splitPacketIndex; + if (internalPacket->splitPacketId != m_packetId || + internalPacket->splitPacketCount != m_count || + index >= m_count) + return false; + // An honest sender splits on byte boundaries; only the last fragment may + // end mid-byte. Anything else would overrun the reassembly buffer. + if (index + 1 < m_count && (internalPacket->dataBitLength & 7) != 0) + return false; + return m_fragments.emplace(index, internalPacket).second; } int MafiaNet::SplitPacketChannelComp( SplitPacketIdType const &key, SplitPacketChannel* const &data ) @@ -547,13 +525,11 @@ void ReliabilityLayer::FreeThreadSafeMemory( void ) for (i=0; i < splitPacketChannelList.Size(); i++) { - for (j=0; j < splitPacketChannelList[i]->splitPacketList.GetAllocSize(); j++) + for (const auto &fragment : splitPacketChannelList[i]->splitPacketList.Fragments()) { - internalPacket = splitPacketChannelList[i]->splitPacketList[j]; - if (internalPacket != nullptr) { - FreeInternalPacketData(internalPacket, _FILE_AND_LINE_); - ReleaseToInternalPacketPool(internalPacket); - } + internalPacket = fragment.second; + FreeInternalPacketData(internalPacket, _FILE_AND_LINE_); + ReleaseToInternalPacketPool(internalPacket); } #if PREALLOCATE_LARGE_MESSAGES==1 if (splitPacketChannelList[i]->returnedPacket) @@ -1190,9 +1166,11 @@ bool ReliabilityLayer::HandleSocketReceiveFromConnectedPlayer( if ( internalPacket->reliability != MafiaNet::Reliability::ReliableOrdered ) internalPacket->orderingChannel = 255; // Use 255 to designate not sequenced and not ordered + // Insert may free the fragment (duplicate or malformed): read its id first. + const SplitPacketIdType splitPacketId = internalPacket->splitPacketId; InsertIntoSplitPacketList( internalPacket, timeRead ); - internalPacket = BuildPacketFromSplitPacketList( internalPacket->splitPacketId, timeRead, + internalPacket = BuildPacketFromSplitPacketList( splitPacketId, timeRead, s, systemAddress, rnr, extraSocketOptions); if ( internalPacket == 0 ) @@ -1285,9 +1263,11 @@ bool ReliabilityLayer::HandleSocketReceiveFromConnectedPlayer( if ( internalPacket->reliability != MafiaNet::Reliability::ReliableOrdered && internalPacket->reliability!=MafiaNet::Reliability::ReliableSequenced && internalPacket->reliability!=MafiaNet::Reliability::UnreliableSequenced) internalPacket->orderingChannel = 255; // Use 255 to designate not sequenced and not ordered + // Insert may free the fragment (duplicate or malformed): read its id first. + const SplitPacketIdType splitPacketId = internalPacket->splitPacketId; InsertIntoSplitPacketList( internalPacket, timeRead ); - internalPacket = BuildPacketFromSplitPacketList( internalPacket->splitPacketId, timeRead, + internalPacket = BuildPacketFromSplitPacketList( splitPacketId, timeRead, s, systemAddress, rnr, updateBitStream); if ( internalPacket == 0 ) @@ -2872,7 +2852,6 @@ InternalPacket* ReliabilityLayer::CreateInternalPacketFromBitStream(MafiaNet::Bi bitStream->ReadAlignedVar32((char*)& internalPacket->splitPacketCount); // Only needed if splitPacketCount>0. 4 bytes bitStream->ReadAlignedVar16((char*)& internalPacket->splitPacketId); // Only needed if splitPacketCount>0. readSuccess=bitStream->ReadAlignedVar32((char*)& internalPacket->splitPacketIndex); // Only needed if splitPacketCount>0. 4 bytes - RakAssert(readSuccess); // printf("Read after\n"); // bitStream->PrintBits(); @@ -2886,17 +2865,17 @@ InternalPacket* ReliabilityLayer::CreateInternalPacketFromBitStream(MafiaNet::Bi internalPacket->dataBitLength==0 || (unsigned int)internalPacket->reliability>=MafiaNet::NUMBER_OF_RELIABILITIES || internalPacket->orderingChannel>=32 || - (hasSplitPacket && (internalPacket->splitPacketIndex >= internalPacket->splitPacketCount))) + (hasSplitPacket && (internalPacket->splitPacketIndex >= internalPacket->splitPacketCount)) || + BITS_TO_BYTES( internalPacket->dataBitLength ) >= MAXIMUM_MTU_SIZE) { - // If this assert hits, encoding is garbage - RakAssert("Encoding is garbage" && 0); + // Malformed or hostile input from the remote: drop it. Not a RakAssert -- + // a Debug build must not abort on what a peer chooses to send. ReleaseToInternalPacketPool( internalPacket ); return 0; } // Allocate memory to hold our data AllocInternalPacketData(internalPacket, BITS_TO_BYTES( internalPacket->dataBitLength ), false, _FILE_AND_LINE_ ); - RakAssert(BITS_TO_BYTES( internalPacket->dataBitLength )data == 0) { @@ -3384,8 +3363,7 @@ void ReliabilityLayer::InsertIntoSplitPacketList( InternalPacket * internalPacke #else newChannel->firstPacket=0; index=splitPacketChannelList.Insert(internalPacket->splitPacketId, newChannel, true, __FILE__,__LINE__); - // Preallocate to the final size, to avoid runtime copies - newChannel->splitPacketList.Preallocate(internalPacket, __FILE__,__LINE__); + newChannel->splitPacketList.Init(internalPacket); #endif } @@ -3471,6 +3449,13 @@ void ReliabilityLayer::InsertIntoSplitPacketList( InternalPacket * internalPacke if (!splitPacketChannelList[index]->splitPacketList.Add(internalPacket)) { FreeInternalPacketData(internalPacket, _FILE_AND_LINE_); ReleaseToInternalPacketPool(internalPacket); + // A channel whose first fragment was rejected holds nothing; drop it so + // a malformed fragment cannot pin a channel slot. + if (splitPacketChannelList[index]->splitPacketList.GetNumAddedPackets() == 0) + { + MafiaNet::OP_DELETE(splitPacketChannelList[index], _FILE_AND_LINE_); + splitPacketChannelList.RemoveAtIndex(index); + } return; } @@ -3524,32 +3509,38 @@ InternalPacket * ReliabilityLayer::BuildPacketFromSplitPacketList( SplitPacketCh (void) time; return returnedPacket; #else - size_t j; - InternalPacket * internalPacket, *splitPacket; - // int splitPacketPartLength; - - // Reconstruct - internalPacket = CreateInternalPacketCopy( splitPacketChannel->splitPacketList[0], 0, 0, time ); - internalPacket->dataBitLength=0; - for (j=0; j < splitPacketChannel->splitPacketList.GetAllocSize(); j++) - internalPacket->dataBitLength+=splitPacketChannel->splitPacketList[j]->dataBitLength; - // splitPacketPartLength=BITS_TO_BYTES(splitPacketChannel->firstPacket->dataBitLength); - - internalPacket->data = (unsigned char*) rakMalloc_Ex( (size_t) BITS_TO_BYTES( internalPacket->dataBitLength ), _FILE_AND_LINE_ ); - internalPacket->allocationScheme=InternalPacket::NORMAL; + const std::map &fragments = splitPacketChannel->splitPacketList.Fragments(); - BitSize_t offset = 0; - for (j=0; j < splitPacketChannel->splitPacketList.GetAllocSize(); j++) + // The fragment lengths come from the remote: sum them wide and refuse a + // message whose length does not fit BitSize_t rather than wrap around and + // allocate too little. + uint64_t totalBits = 0; + for (const auto &fragment : fragments) + totalBits += fragment.second->dataBitLength; + + InternalPacket *internalPacket = nullptr; + if (totalBits <= (uint64_t) (BitSize_t) -1) { - splitPacket = splitPacketChannel->splitPacketList[j]; - memcpy(internalPacket->data + BITS_TO_BYTES(offset), splitPacket->data, (size_t)BITS_TO_BYTES(splitPacket->dataBitLength)); - offset += splitPacket->dataBitLength; + internalPacket = CreateInternalPacketCopy( fragments.begin()->second, 0, 0, time ); + internalPacket->dataBitLength = (BitSize_t) totalBits; + internalPacket->data = (unsigned char*) rakMalloc_Ex( (size_t) BITS_TO_BYTES( internalPacket->dataBitLength ), _FILE_AND_LINE_ ); + internalPacket->allocationScheme=InternalPacket::NORMAL; + + // Add() guarantees every fragment but the last is whole bytes, so the + // byte offsets below are exact and the copies end inside the buffer. + BitSize_t offset = 0; + for (const auto &fragment : fragments) + { + InternalPacket *splitPacket = fragment.second; + memcpy(internalPacket->data + BITS_TO_BYTES(offset), splitPacket->data, (size_t)BITS_TO_BYTES(splitPacket->dataBitLength)); + offset += splitPacket->dataBitLength; + } } - for (j=0; j < splitPacketChannel->splitPacketList.GetAllocSize(); j++) + for (const auto &fragment : fragments) { - FreeInternalPacketData(splitPacketChannel->splitPacketList[j], _FILE_AND_LINE_ ); - ReleaseToInternalPacketPool(splitPacketChannel->splitPacketList[j]); + FreeInternalPacketData(fragment.second, _FILE_AND_LINE_ ); + ReleaseToInternalPacketPool(fragment.second); } MafiaNet::OP_DELETE(splitPacketChannel, __FILE__, __LINE__); @@ -3566,8 +3557,11 @@ InternalPacket * ReliabilityLayer::BuildPacketFromSplitPacketList( SplitPacketId SplitPacketChannel *splitPacketChannel; InternalPacket * internalPacket; - // Find in splitPacketChannelList the SplitPacketChannel with this splitPacketId + // Find in splitPacketChannelList the SplitPacketChannel with this splitPacketId. + // It is gone if the fragment just inserted was rejected and was its only one. i=splitPacketChannelList.GetIndexFromKey(inSplitPacketId, &objectExists); + if (!objectExists) + return 0; splitPacketChannel=splitPacketChannelList[i]; #if PREALLOCATE_LARGE_MESSAGES==1 diff --git a/Tests/Unit/BitStreamBoundsTests.cpp b/Tests/Unit/BitStreamBoundsTests.cpp new file mode 100644 index 000000000..90bc81a3f --- /dev/null +++ b/Tests/Unit/BitStreamBoundsTests.cpp @@ -0,0 +1,93 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * Reads whose length comes from the data being read. Found by the + * bit_stream fuzz harness (Tests/Fuzz). + */ + +#include + +#include + +#include "mafianet/bit_stream.h" + +using namespace MafiaNet; + +namespace +{ + // 0x20000001 << 3 wraps to 8 in 32 bits: the old bounds checks compared + // against 8 bits, passed, and copied 512 MB. + const unsigned int kWrappingLength = 0x20000001u; + + void FillSixteenBytes(BitStream &bs) + { + for (int i = 0; i < 16; ++i) + bs.Write((unsigned char) i); + } +} + +TEST(BitStreamBounds, AlignedReadLongerThanTheStreamWhoseBitCountWrapsIsRejected) +{ + BitStream bs; + FillSixteenBytes(bs); + char out[16] = {}; + EXPECT_FALSE(bs.Read(out, kWrappingLength)); + EXPECT_EQ(0u, bs.GetReadOffset()); +} + +TEST(BitStreamBounds, UnalignedReadLongerThanTheStreamWhoseBitCountWrapsIsRejected) +{ + BitStream bs; + FillSixteenBytes(bs); + bs.IgnoreBits(1); + char out[16] = {}; + EXPECT_FALSE(bs.Read(out, kWrappingLength)); + EXPECT_EQ(1u, bs.GetReadOffset()); +} + +TEST(BitStreamBounds, ReadAlignedBytesWhoseBitCountWrapsIsRejected) +{ + BitStream bs; + FillSixteenBytes(bs); + unsigned char out[16] = {}; + EXPECT_FALSE(bs.ReadAlignedBytes(out, kWrappingLength)); +} + +TEST(BitStreamBounds, ReadExactlyTheRemainingBytesSucceeds) +{ + BitStream bs; + FillSixteenBytes(bs); + char out[16] = {}; + ASSERT_TRUE(bs.Read(out, 16)); + EXPECT_EQ(15, out[15]); + EXPECT_FALSE(bs.Read(out, 1)); +} + +TEST(BitStreamBounds, SafeAllocReadThatRunsShortReturnsNoBuffer) +{ + // Claims 100 bytes, carries 4: the buffer used to be returned allocated + // with nothing read into it, leaking for callers that free on success. + BitStream bs; + bs.WriteCompressed((unsigned int) 100); + bs.Write((uint32_t) 0xDEADBEEF); + + char *buffer = nullptr; + unsigned int length = 0; + EXPECT_FALSE(bs.ReadAlignedBytesSafeAlloc(&buffer, length, 4096)); + EXPECT_EQ(nullptr, buffer); +} + +TEST(BitStreamBounds, ReadDeltaAtTheEndOfTheStreamFailsAndLeavesTheValue) +{ + // The "was written" flag was read into an uninitialized bool and then + // branched on even when that read had failed. + BitStream bs; + uint32_t value = 1234; + EXPECT_FALSE(bs.ReadDelta(value)); + EXPECT_EQ(1234u, value); + EXPECT_FALSE(bs.ReadCompressedDelta(value)); + EXPECT_EQ(1234u, value); +} diff --git a/Tests/Unit/ReliabilityLayerSplitPacketTests.cpp b/Tests/Unit/ReliabilityLayerSplitPacketTests.cpp new file mode 100644 index 000000000..41bcf793f --- /dev/null +++ b/Tests/Unit/ReliabilityLayerSplitPacketTests.cpp @@ -0,0 +1,220 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * Hostile split-packet fragments against a connected ReliabilityLayer. + * + * Every field of a fragment header (fragment count, index, id, bit length) + * comes from the remote. These tests hand-build datagrams in the wire format + * so they can send what an honest sender never would. The overflow cases are + * only guaranteed to fail loudly under the asan preset; in a plain build they + * check that the message is still reassembled exactly and the layer keeps + * working. + */ + +#include + +#include +#include + +#include "mafianet/bit_stream.h" +#include "mafianet/ds_list.h" +#include "mafianet/get_time.h" +#include "mafianet/mtu_size.h" +#include "mafianet/packet_priority.h" +#include "mafianet/plugin_interface2.h" +#include "mafianet/rand.h" +#include "mafianet/reliability_layer.h" +#include "mafianet/socket2.h" + +using namespace MafiaNet; + +namespace +{ + struct NullSocket : public RakNetSocket2 + { + RNS2SendResult Send(RNS2_SendParameters *sendParameters, const char *, unsigned int) override + { + return sendParameters->length; + } + }; + + struct Fragment + { + uint32_t count; + uint16_t id; + uint32_t index; + std::vector data; + uint16_t bitLength; // 0 means data.size() * 8 + }; + + class SplitPacketFixture : public ::testing::Test + { + protected: + ReliabilityLayer layer; + NullSocket socket; + SystemAddress remote{"127.0.0.1", 40001}; + DataStructures::List handlers; + RakNetRandom rnr; + BitStream updateBitStream{MAXIMUM_MTU_SIZE}; + CCTimeType now = 0; + uint32_t nextDatagramNumber = 0; + + void SetUp() override + { + // ReliabilityLayer stamps arrivals with the real clock; anchor to it + // (see ReliabilityLayerBlackHoleTests). + now = GetTimeUS(); + layer.Reset(true, MAXIMUM_MTU_SIZE, false); + layer.SetTimeoutTime(1000000); + } + + // One unreliable split fragment in one datagram, in the layer's wire + // format (DatagramHeaderFormat::Serialize, WriteToBitStreamFromInternalPacket). + void Deliver(const Fragment &f) + { + BitStream b; + b.Write(true); // isValid + b.Write(false); // isACK + b.Write(false); // isNAK + b.Write(false); // isPacketPair + b.Write(false); // isContinuousSend + b.Write(false); // needsBAndAs + b.AlignWriteToByteBoundary(); +#if INCLUDE_TIMESTAMP_WITH_DATAGRAMS == 1 + b.Write((MafiaNet::TimeMS) 0); +#endif + b.Write(DatagramSequenceNumberType(nextDatagramNumber++)); + + b.AlignWriteToByteBoundary(); + const unsigned char reliability = (unsigned char) MafiaNet::Reliability::Unreliable; + b.WriteBits(&reliability, 3, true); + b.Write(true); // hasSplitPacket + b.AlignWriteToByteBoundary(); + const uint16_t bits = f.bitLength ? f.bitLength : (uint16_t) (f.data.size() * 8); + b.WriteAlignedVar16((const char *) &bits); + b.AlignWriteToByteBoundary(); + b.WriteAlignedVar32((const char *) &f.count); + b.WriteAlignedVar16((const char *) &f.id); + b.WriteAlignedVar32((const char *) &f.index); + b.WriteAlignedBytes(f.data.data(), (unsigned int) f.data.size()); + + std::vector datagram(b.GetData(), b.GetData() + b.GetNumberOfBytesUsed()); + layer.HandleSocketReceiveFromConnectedPlayer(datagram.data(), (unsigned int) datagram.size(), + remote, handlers, MAXIMUM_MTU_SIZE, &socket, &rnr, now, updateBitStream); + now += 10000; + layer.Update(&socket, remote, MAXIMUM_MTU_SIZE, now, 0, handlers, &rnr, updateBitStream); + } + + std::vector > Drain() + { + std::vector > messages; + unsigned char *data; + BitSize_t bits; + while ((bits = layer.Receive(&data)) > 0) + { + messages.emplace_back(data, data + BITS_TO_BYTES(bits)); + rakFree_Ex(data, _FILE_AND_LINE_); + } + return messages; + } + + static std::vector Bytes(unsigned char seed, size_t n) + { + std::vector v(n); + v[0] = 200; // clear of internal message ids + for (size_t i = 1; i < n; ++i) + v[i] = (unsigned char) (seed + i); + return v; + } + + static std::vector Concat(const std::vector &a, const std::vector &b) + { + std::vector out(a); + out.insert(out.end(), b.begin(), b.end()); + return out; + } + + // An honest two-fragment message; proves the layer still works. + void ExpectHonestMessageDelivered(uint16_t id) + { + const std::vector first = Bytes(1, 8), second = Bytes(50, 8); + Deliver({2, id, 0, first, 0}); + Deliver({2, id, 1, second, 0}); + const auto messages = Drain(); + ASSERT_EQ(1u, messages.size()); + EXPECT_EQ(Concat(first, second), messages[0]); + } + }; +} + +TEST_F(SplitPacketFixture, HonestFragmentsReassembleInAnyOrder) +{ + // Harness check: the hand-built format is what the layer parses. + const std::vector a = Bytes(1, 8), b = Bytes(2, 8), c = Bytes(3, 5); + Deliver({3, 4, 2, c, 0}); + Deliver({3, 4, 0, a, 0}); + EXPECT_TRUE(Drain().empty()); + Deliver({3, 4, 1, b, 0}); + const auto messages = Drain(); + ASSERT_EQ(1u, messages.size()); + EXPECT_EQ(Concat(Concat(a, b), c), messages[0]); +} + +TEST_F(SplitPacketFixture, HugeClaimedFragmentCountAllocatesNothingUpFront) +{ + // Used to preallocate one pointer per claimed fragment: 32 GB here. + Deliver({0xFFFFFFFFu, 1, 0, Bytes(9, 8), 0}); + EXPECT_TRUE(Drain().empty()); + ExpectHonestMessageDelivered(2); +} + +TEST_F(SplitPacketFixture, FragmentDisagreeingWithTheFirstOnCountIsRejected) +{ + // Index 999 is valid for the count this fragment claims, but not for the + // message's real count of 2: it used to be written past the fragment table. + const std::vector first = Bytes(1, 8), second = Bytes(50, 8); + Deliver({2, 7, 0, first, 0}); + Deliver({1000, 7, 999, Bytes(99, 8), 0}); + EXPECT_TRUE(Drain().empty()); + Deliver({2, 7, 1, second, 0}); + const auto messages = Drain(); + ASSERT_EQ(1u, messages.size()); + EXPECT_EQ(Concat(first, second), messages[0]); +} + +TEST_F(SplitPacketFixture, NonFinalFragmentThatIsNotWholeBytesIsRejected) +{ + // Reassembly copies fragments at byte offsets; a 12-bit first fragment + // made the copies run a byte past the buffer sized from the bit total. + Deliver({2, 9, 0, Bytes(1, 2), 12}); + Deliver({2, 9, 1, Bytes(2, 1), 0}); + EXPECT_TRUE(Drain().empty()); + ExpectHonestMessageDelivered(10); +} + +TEST_F(SplitPacketFixture, FinalFragmentMayEndMidByte) +{ + const std::vector first = Bytes(1, 8); + const std::vector last = {0xA0}; + Deliver({2, 11, 0, first, 0}); + Deliver({2, 11, 1, last, 3}); // 3 bits + const auto messages = Drain(); + ASSERT_EQ(1u, messages.size()); + ASSERT_EQ(9u, messages[0].size()); + EXPECT_TRUE(std::equal(first.begin(), first.end(), messages[0].begin())); + EXPECT_EQ(0xA0 & 0xE0, messages[0][8] & 0xE0); +} + +TEST_F(SplitPacketFixture, DuplicateFragmentIsIgnored) +{ + const std::vector first = Bytes(1, 8), second = Bytes(50, 8); + Deliver({2, 12, 0, first, 0}); + Deliver({2, 12, 0, Bytes(77, 8), 0}); + Deliver({2, 12, 1, second, 0}); + const auto messages = Drain(); + ASSERT_EQ(1u, messages.size()); + EXPECT_EQ(Concat(first, second), messages[0]); +} From 7e58710ff02169f30bf9d3059af60511164b086c Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:22:40 +0200 Subject: [PATCH 03/12] fix: build cleanly with -Wall -Wextra, and fix the bugs the warnings exposed mafianet_enable_warnings() applies -Wall -Wextra (/W4 on MSVC) to the MafiaNet libraries and test targets only, never to fetched dependencies. MAFIANET_WARNINGS_AS_ERRORS makes them errors. Apple Clang and GCC 13 now report zero warnings in Debug and Release. Real bugs behind the warnings, each fixed with a test where testable: - TCPInterface ConnectionAttemptLoop read s->bindAddress after deleting s (use-after-free on every outgoing TCP connect). - Table::Cell's copy constructor freed its own uninitialized pointer and left it uninitialized when the source held no data, so the copy's destructor freed garbage. - RelayPlugin relayed every group message as High / ReliableOrdered / channel 0, ignoring what the sender asked for. - The vsnprintf_s shim (Linux, macOS) wrote one byte past the buffer when _TRUNCATE truncated, which FormatString hit on any long string. - FileList::PopulateDataFromDisk(..., false, false, ...) never advanced to the next file and leaked the open handle: an infinite loop. - The test plugins' OnNewConnection / OnClosedConnection no longer matched PluginInterface2 and were never called. - RakVoice::SetOrderingChannels could not reject negative channels where char is unsigned (ARM). Also: fread/fgets results are checked (a short read skips the file, as an unopenable one did); sprintf -> snprintf where the bound is known; SystemAddress and RakNetGUID get explicit defaulted copy operations (their assignment operators were plain member-wise copies; the out-of-line SystemAddress::operator= is kept for the exported symbol); MSVC-only C4996 pragmas removed (the build already defines _CRT_SECURE_NO_DEPRECATE); dead code behind unused-but-set variables removed; mixed-indentation sites re-indented. --- CMakeLists.txt | 3 + Source/CMakeLists.txt | 3 + Source/include/mafianet/bit_stream.h | 3 +- Source/include/mafianet/dr_sha1.h | 4 +- Source/include/mafianet/ds_linked_list.h | 42 ++--- Source/include/mafianet/ds_map.h | 6 +- Source/include/mafianet/ds_ordered_list.h | 6 +- Source/include/mafianet/relay_plugin.h | 2 +- Source/include/mafianet/replica_manager3.h | 4 +- Source/include/mafianet/types.h | 12 +- Source/src/bit_stream.cpp | 10 +- Source/src/cc_sliding_window.cpp | 4 +- Source/src/data_compressor.cpp | 2 +- Source/src/ds_table.cpp | 11 +- Source/src/email_sender.cpp | 10 +- Source/src/file_list.cpp | 73 +++++--- Source/src/fully_connected_mesh2.cpp | 4 +- Source/src/get_time.cpp | 4 +- Source/src/gets.cpp | 8 +- Source/src/http_connection.cpp | 4 +- Source/src/incremental_read_interface.cpp | 2 +- Source/src/linux_adapter.cpp | 13 +- Source/src/log_command_parser.cpp | 3 +- Source/src/network_id_manager.cpp | 4 +- Source/src/osx_adapter.cpp | 13 +- Source/src/packet_logger.cpp | 13 +- Source/src/packet_output_window_logger.cpp | 2 +- Source/src/peer.cpp | 45 ++--- Source/src/relay_plugin.cpp | 9 +- Source/src/reliability_layer.cpp | 44 +++-- Source/src/replica_manager3.cpp | 2 - Source/src/signaled_event.cpp | 3 +- Source/src/sleep.cpp | 4 +- Source/src/socket2.cpp | 2 +- Source/src/socket2_berkley.cpp | 4 +- Source/src/socket2_windows_linux.cpp | 2 +- Source/src/socket_layer.cpp | 2 +- Source/src/statistics.cpp | 35 ++-- Source/src/statistics_history.cpp | 6 +- Source/src/string.cpp | 4 +- Source/src/super_fast_hash.cpp | 7 +- Source/src/table_serializer.cpp | 4 +- Source/src/tcp_interface.cpp | 14 +- Source/src/types.cpp | 34 +--- Source/src/voice.cpp | 6 +- Source/src/wstring.cpp | 17 +- Tests/CMakeLists.txt | 3 + .../Integration/ComprehensiveConvertTests.cpp | 6 +- .../CrossConnectionConvertTests.cpp | 2 +- Tests/Integration/PingTestsTests.cpp | 1 - .../RelayPluginGroupMessageLiveTests.cpp | 167 ++++++++++++++++++ Tests/Integration/SessionConfigLiveTests.cpp | 17 +- Tests/Support/CommonFunctions.cpp | 6 +- Tests/Support/CommonFunctions.h | 2 +- Tests/Support/DebugTools.cpp | 3 +- Tests/Support/PacketChangerPlugin.h | 26 +-- Tests/Support/PacketDropPlugin.cpp | 2 +- Tests/Support/PacketDropPlugin.h | 24 +-- Tests/Support/RakTimer.cpp | 2 +- Tests/Unit/FileListDirectoryTests.cpp | 67 +++++++ Tests/Unit/GuidUtilTests.cpp | 6 +- Tests/Unit/SafeStringShimTests.cpp | 83 +++++++++ Tests/Unit/TableCellTests.cpp | 67 +++++++ cmake/MafiaNetHelpers.cmake | 17 ++ 64 files changed, 701 insertions(+), 309 deletions(-) create mode 100644 Tests/Integration/RelayPluginGroupMessageLiveTests.cpp create mode 100644 Tests/Unit/SafeStringShimTests.cpp create mode 100644 Tests/Unit/TableCellTests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 26a79392e..80c8234bc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -63,6 +63,9 @@ set(MAFIANET_SANITIZER "" CACHE STRING "Build with sanitizers (e.g. address+unde # unless the caller already chose a launcher (CI passes one explicitly). option(MAFIANET_USE_COMPILER_CACHE "Use sccache/ccache when available" ON) +# Compiler warnings on MafiaNet's own targets (see mafianet_enable_warnings). +option(MAFIANET_WARNINGS_AS_ERRORS "Treat compiler warnings in MafiaNet code as errors" OFF) + # Ensure at least one library type is built if(NOT MAFIANET_BUILD_SHARED AND NOT MAFIANET_BUILD_STATIC) message(FATAL_ERROR "At least one of MAFIANET_BUILD_SHARED or MAFIANET_BUILD_STATIC must be ON") diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 5ddd97d52..e8ceb0967 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -428,6 +428,8 @@ function(mafianet_configure_target target_name) source_group("Source Files\\crypto" FILES ${MAFIANET_CRYPTO_SOURCES}) source_group("Header Files" FILES ${MAFIANET_HEADERS}) source_group("Header Files\\crypto" FILES ${MAFIANET_CRYPTO_HEADERS}) + + mafianet_enable_warnings(${target_name}) endfunction() # Static library @@ -490,6 +492,7 @@ if(MAFIANET_BUILD_EXTRAS) $<$:NDEBUG> ) set_target_properties(MafiaNetExtras PROPERTIES FOLDER "Libraries") + mafianet_enable_warnings(MafiaNetExtras) source_group("Source Files" FILES ${MAFIANET_EXTRAS_SOURCES}) endif() diff --git a/Source/include/mafianet/bit_stream.h b/Source/include/mafianet/bit_stream.h index 096521567..62340867c 100644 --- a/Source/include/mafianet/bit_stream.h +++ b/Source/include/mafianet/bit_stream.h @@ -1770,8 +1770,9 @@ namespace MafiaNet return ReadBitsFromIntegerRange(value,minimum,maximum,requiredBits,allowOutsideRange); } template - bool BitStream::ReadBitsFromIntegerRange( templateType &value, const templateType minimum, const templateType maximum, const int requiredBits, bool allowOutsideRange ) + bool BitStream::ReadBitsFromIntegerRange( templateType &value, const templateType minimum, [[maybe_unused]] const templateType maximum, const int requiredBits, bool allowOutsideRange ) { + // maximum is only checked by the (debug-only) assert; requiredBits already encodes the range. RakAssert(maximum>=minimum); if (allowOutsideRange) { diff --git a/Source/include/mafianet/dr_sha1.h b/Source/include/mafianet/dr_sha1.h index 64a4daece..4f1c0c598 100644 --- a/Source/include/mafianet/dr_sha1.h +++ b/Source/include/mafianet/dr_sha1.h @@ -299,10 +299,10 @@ unsigned char * GetHash( void ) const; // Member variables UINT_32 m_state[5]; UINT_32 m_count[2]; - UINT_32 m_reserved0[1]; // Memory alignment padding + [[maybe_unused]] UINT_32 m_reserved0[1]; // Memory alignment padding (never read; keeps the layout) UINT_8 m_buffer[64]; UINT_8 m_digest[20]; - UINT_32 m_reserved1[3]; // Memory alignment padding + [[maybe_unused]] UINT_32 m_reserved1[3]; // Memory alignment padding (never read; keeps the layout) UINT_8 m_workspace[64]; SHA1_WORKSPACE_BLOCK* m_block; // SHA1 pointer to the byte array above diff --git a/Source/include/mafianet/ds_linked_list.h b/Source/include/mafianet/ds_linked_list.h index 72cfabf82..4919fd193 100644 --- a/Source/include/mafianet/ds_linked_list.h +++ b/Source/include/mafianet/ds_linked_list.h @@ -954,33 +954,33 @@ namespace DataStructures { if ( this->list_size == 0 ) return ; - else - if ( this->list_size == 1 ) // {MafiaNet::OP_DELETE(root->item); MafiaNet::OP_DELETE(root, _FILE_AND_LINE_);} - { - MafiaNet::OP_DELETE(this->root, _FILE_AND_LINE_); - } - else - { - node* current; - node* temp; + if ( this->list_size == 1 ) // {MafiaNet::OP_DELETE(root->item); MafiaNet::OP_DELETE(root, _FILE_AND_LINE_);} + { + MafiaNet::OP_DELETE(this->root, _FILE_AND_LINE_); + } - current = this->root; + else + { + node* current; + node* temp; - do - { - temp = current; - current = current->next; - // MafiaNet::OP_DELETE(temp->item, _FILE_AND_LINE_); - MafiaNet::OP_DELETE(temp, _FILE_AND_LINE_); - } + current = this->root; - while ( current != this->root ); + do + { + temp = current; + current = current->next; + // MafiaNet::OP_DELETE(temp->item, _FILE_AND_LINE_); + MafiaNet::OP_DELETE(temp, _FILE_AND_LINE_); } - this->list_size = 0; - this->root = 0; - this->position = 0; + while ( current != this->root ); + } + + this->list_size = 0; + this->root = 0; + this->position = 0; } template diff --git a/Source/include/mafianet/ds_map.h b/Source/include/mafianet/ds_map.h index bc241791a..4f2c701c2 100644 --- a/Source/include/mafianet/ds_map.h +++ b/Source/include/mafianet/ds_map.h @@ -39,7 +39,11 @@ namespace DataStructures template int defaultMapKeyComparison(const key_type &a, const key_type &b) { - if (a int defaultOrderedListComparison(const key_type &a, const data_type &b) { - if (a b? - const DatagramSequenceNumberType halfSpan =(DatagramSequenceNumberType) (((DatagramSequenceNumberType)(const uint32_t)-1)/(DatagramSequenceNumberType)2); + const DatagramSequenceNumberType halfSpan =(DatagramSequenceNumberType) (((DatagramSequenceNumberType)(uint32_t)-1)/(DatagramSequenceNumberType)2); return b!=a && b-a>halfSpan; } // ---------------------------------------------------------------------------------------------------------------------------- bool CCRakNetSlidingWindow::LessThan(DatagramSequenceNumberType a, DatagramSequenceNumberType b) { // a < b? - const DatagramSequenceNumberType halfSpan = ((DatagramSequenceNumberType)(const uint32_t)-1)/(DatagramSequenceNumberType)2; + const DatagramSequenceNumberType halfSpan = ((DatagramSequenceNumberType)(uint32_t)-1)/(DatagramSequenceNumberType)2; return b!=a && b-anext; } + _columnIndices=0; return; } @@ -954,6 +953,8 @@ void Table::SortTable(Table::SortQuery *sortQueries, unsigned numSortQueries, Ta outLength=0; for (i=0; i < orderedList.Size(); i++) out[(outLength)++]=orderedList[i]; + // RowSort reads the indices only while this function runs. + _columnIndices=0; } void Table::PrintColumnHeaders(char *out, int outLength, char columnDelineator) const { diff --git a/Source/src/email_sender.cpp b/Source/src/email_sender.cpp index e14308ad7..27d806d30 100644 --- a/Source/src/email_sender.cpp +++ b/Source/src/email_sender.cpp @@ -107,16 +107,16 @@ const char *EmailSender::Send(const char *hostAddress, unsigned short hostPort, return response; if (password==0) return "Password needed"; - char *outputData = MafiaNet::OP_NEW_ARRAY((const int) (strlen(sender)+strlen(password)+2)*3, _FILE_AND_LINE_ ); + char *outputData = MafiaNet::OP_NEW_ARRAY((int) (strlen(sender)+strlen(password)+2)*3, _FILE_AND_LINE_ ); MafiaNet::BitStream bs; char zero=0; bs.Write(&zero,1); - bs.Write(sender,(const unsigned int)strlen(sender)); - //bs.Write("jms1@jms1.net",(const unsigned int)strlen("jms1@jms1.net")); + bs.Write(sender,(unsigned int)strlen(sender)); + //bs.Write("jms1@jms1.net",(unsigned int)strlen("jms1@jms1.net")); bs.Write(&zero,1); - bs.Write(password,(const unsigned int)strlen(password)); + bs.Write(password,(unsigned int)strlen(password)); bs.Write(&zero,1); - //bs.Write("not.my.real.password",(const unsigned int)strlen("not.my.real.password")); + //bs.Write("not.my.real.password",(unsigned int)strlen("not.my.real.password")); Base64Encoding((const unsigned char*)bs.GetData(), bs.GetNumberOfBytesUsed(), outputData); sprintf_s(query, "AUTH PLAIN %s", outputData); tcpInterface.Send(query, (unsigned int)strlen(query), emailServer,false); diff --git a/Source/src/file_list.cpp b/Source/src/file_list.cpp index 9a6a6098c..6111c95d5 100644 --- a/Source/src/file_list.cpp +++ b/Source/src/file_list.cpp @@ -155,8 +155,10 @@ void FileList::AddFile(const char *filepath, const char *filename, FileListNodeC RakAssert(data); } - fread(data, 1, length, fp); - AddFile(filename, filepath, data, length, length, context); + // A short read means the file shrank or failed under us; skip it like an unopenable one. + const bool readOk = fread(data, 1, length, fp) == (size_t) length; + if (readOk) + AddFile(filename, filepath, data, length, length, context); fclose(fp); #if USE_ALLOCA==1 @@ -313,20 +315,23 @@ void FileList::AddFilesFromDirectory(const char *applicationDirectory, const cha { fileData= (char*) rakMalloc_Ex( fileInfo.size+HASH_LENGTH, _FILE_AND_LINE_ ); RakAssert(fileData); - fread(fileData+HASH_LENGTH, fileInfo.size, 1, fp); + const bool readOk = fileInfo.size == 0 || fread(fileData+HASH_LENGTH, fileInfo.size, 1, fp) == 1; fclose(fp); - unsigned int hash = SuperFastHash(fileData+HASH_LENGTH, fileInfo.size); - if (MafiaNet::BitStream::DoEndianSwap()) - MafiaNet::BitStream::ReverseBytesInPlace((unsigned char*) &hash, sizeof(hash)); - memcpy(fileData, &hash, HASH_LENGTH); - - // sha1.Reset(); - // sha1.Update( ( unsigned char* ) fileData+HASH_LENGTH, fileInfo.size ); - // sha1.Final(); - // memcpy(fileData, sha1.GetHash(), HASH_LENGTH); - // File data and hash - AddFile((const char*)fullPath+rootLen, fullPath, fileData, fileInfo.size+HASH_LENGTH, fileInfo.size, context); + if (readOk) + { + unsigned int hash = SuperFastHash(fileData+HASH_LENGTH, fileInfo.size); + if (MafiaNet::BitStream::DoEndianSwap()) + MafiaNet::BitStream::ReverseBytesInPlace((unsigned char*) &hash, sizeof(hash)); + memcpy(fileData, &hash, HASH_LENGTH); + + // sha1.Reset(); + // sha1.Update( ( unsigned char* ) fileData+HASH_LENGTH, fileInfo.size ); + // sha1.Final(); + // memcpy(fileData, sha1.GetHash(), HASH_LENGTH); + // File data and hash + AddFile((const char*)fullPath+rootLen, fullPath, fileData, fileInfo.size+HASH_LENGTH, fileInfo.size, context); + } } } else if (writeHash) @@ -347,12 +352,17 @@ void FileList::AddFilesFromDirectory(const char *applicationDirectory, const cha { fileData= (char*) rakMalloc_Ex( fileInfo.size, _FILE_AND_LINE_ ); RakAssert(fileData); - fopen_s(&fp, fullPath, "rb"); - fread(fileData, fileInfo.size, 1, fp); - fclose(fp); + // Skip a file that cannot be opened or read in full, as the hash+data branch + // above does; fileData is freed below either way. + if (fopen_s(&fp, fullPath, "rb") == 0) + { + const bool readOk = fileInfo.size == 0 || fread(fileData, fileInfo.size, 1, fp) == 1; + fclose(fp); - // File data only - AddFile(fullPath+rootLen, fullPath, fileData, fileInfo.size, fileInfo.size, context); + // File data only + if (readOk) + AddFile(fullPath+rootLen, fullPath, fileData, fileInfo.size, fileInfo.size, context); + } } else { @@ -621,6 +631,7 @@ void FileList::PopulateDataFromDisk(const char *applicationDirectory, bool write { if (writeFileHash || writeFileData) { + bool readOk = true; fseek(fp, 0, SEEK_END); fileList[i].fileLengthBytes = ftell(fp); fseek(fp, 0, SEEK_SET); @@ -631,7 +642,7 @@ void FileList::PopulateDataFromDisk(const char *applicationDirectory, bool write // Hash + data so offset the data by HASH_LENGTH fileList[i].data=(char*) rakMalloc_Ex( fileList[i].fileLengthBytes+HASH_LENGTH, _FILE_AND_LINE_ ); RakAssert(fileList[i].data); - fread(fileList[i].data+HASH_LENGTH, fileList[i].fileLengthBytes, 1, fp); + readOk = fileList[i].fileLengthBytes == 0 || fread(fileList[i].data+HASH_LENGTH, fileList[i].fileLengthBytes, 1, fp) == 1; // sha1.Reset(); // sha1.Update((unsigned char*)fileList[i].data+HASH_LENGTH, fileList[i].fileLength); // sha1.Final(); @@ -650,7 +661,7 @@ void FileList::PopulateDataFromDisk(const char *applicationDirectory, bool write else fileList[i].data=(char*) rakMalloc_Ex( fileList[i].fileLengthBytes, _FILE_AND_LINE_ ); RakAssert(fileList[i].data); - fread(fileList[i].data, fileList[i].fileLengthBytes, 1, fp); + readOk = fileList[i].fileLengthBytes == 0 || fread(fileList[i].data, fileList[i].fileLengthBytes, 1, fp) == 1; // sha1.Reset(); // sha1.Update((unsigned char*)fileList[i].data, fileList[i].fileLength); // sha1.Final(); @@ -667,17 +678,28 @@ void FileList::PopulateDataFromDisk(const char *applicationDirectory, bool write fileList[i].dataLengthBytes=fileList[i].fileLengthBytes; fileList[i].data=(char*) rakMalloc_Ex( fileList[i].fileLengthBytes, _FILE_AND_LINE_ ); RakAssert(fileList[i].data); - fread(fileList[i].data, fileList[i].fileLengthBytes, 1, fp); + readOk = fileList[i].fileLengthBytes == 0 || fread(fileList[i].data, fileList[i].fileLengthBytes, 1, fp) == 1; } - fclose(fp); - i++; + if (!readOk) + { + // Short read: drop the partial contents rather than keep uninitialized bytes + // (and a hash computed over them). + rakFree_Ex(fileList[i].data, _FILE_AND_LINE_ ); + fileList[i].data=0; + fileList[i].dataLengthBytes=0; + } } else { fileList[i].data=0; fileList[i].dataLengthBytes=0; } + + // Both branches are done with this entry. The no-data/no-hash branch used to skip + // these, leaking fp and re-processing entry i forever. + fclose(fp); + i++; } else { @@ -795,10 +817,7 @@ bool FileList::FixEndingSlash(char *str) #ifdef _WIN32 if (str[strlen(str) - 1] != '/' && str[strlen(str) - 1] != '\\') { -#pragma warning(push) -#pragma warning(disable:4996) strcat(str, "\\"); // Only \ works with system commands, used by AutopatcherClient -#pragma warning(pop) return true; } #else diff --git a/Source/src/fully_connected_mesh2.cpp b/Source/src/fully_connected_mesh2.cpp index 0f923480c..05356b173 100644 --- a/Source/src/fully_connected_mesh2.cpp +++ b/Source/src/fully_connected_mesh2.cpp @@ -833,7 +833,7 @@ void FullyConnectedMesh2::RespondOnVerifiedJoinCapable(Packet *packet, bool acce bsOut.Write(participatingMembersNotOnClient[i]); bsOut.Write(rakPeerInterface->GetSystemAddressFromGuid(participatingMembersNotOnClient[i])); - bool written=false; + [[maybe_unused]] bool written=false; // only checked by RakAssert (debug builds) for (unsigned int j=0; j < fcm2ParticipantList.Size(); j++) { if (fcm2ParticipantList[j]->rakNetGuid == participatingMembersNotOnClient[i]) @@ -1148,7 +1148,7 @@ PluginReceiveResult FullyConnectedMesh2::OnVerifiedJoinCapable(Packet *packet) bsOut.Write(participatingMembersNotOnClient[i]); bsOut.Write(rakPeerInterface->GetSystemAddressFromGuid(participatingMembersNotOnClient[i])); - bool written=false; + [[maybe_unused]] bool written=false; // only checked by RakAssert (debug builds) for (unsigned int j=0; j < fcm2ParticipantList.Size(); j++) { if (fcm2ParticipantList[j]->rakNetGuid == participatingMembersNotOnClient[i]) diff --git a/Source/src/get_time.cpp b/Source/src/get_time.cpp index a48eca7e7..120d31910 100644 --- a/Source/src/get_time.cpp +++ b/Source/src/get_time.cpp @@ -222,12 +222,12 @@ MafiaNet::TimeUS MafiaNet::GetTimeUS( void ) bool MafiaNet::GreaterThan(MafiaNet::Time a, MafiaNet::Time b) { // a > b? - const MafiaNet::Time halfSpan =(MafiaNet::Time) (((MafiaNet::Time)(const MafiaNet::Time)-1)/(MafiaNet::Time)2); + const MafiaNet::Time halfSpan =(MafiaNet::Time) (((MafiaNet::Time)(MafiaNet::Time)-1)/(MafiaNet::Time)2); return b!=a && b-a>halfSpan; } bool MafiaNet::LessThan(MafiaNet::Time a, MafiaNet::Time b) { // a < b? - const MafiaNet::Time halfSpan = ((MafiaNet::Time)(const MafiaNet::Time)-1)/(MafiaNet::Time)2; + const MafiaNet::Time halfSpan = ((MafiaNet::Time)(MafiaNet::Time)-1)/(MafiaNet::Time)2; return b!=a && b-a 0) + str[0]=0; + return str; + } if (str[0]=='\n' || str[0]=='\r') str[0]=0; diff --git a/Source/src/http_connection.cpp b/Source/src/http_connection.cpp index bff0dd416..4ad93169b 100644 --- a/Source/src/http_connection.cpp +++ b/Source/src/http_connection.cpp @@ -67,8 +67,8 @@ void HTTPConnection::Get(const char *path) bool HTTPConnection::HasBadResponse(int *code, MafiaNet::RakString *data) { - if(badResponses.IsEmpty()) - return false; + if(badResponses.IsEmpty()) + return false; if (code) *code = badResponses.Peek().code; diff --git a/Source/src/incremental_read_interface.cpp b/Source/src/incremental_read_interface.cpp index 6734dd76f..78f3975e9 100644 --- a/Source/src/incremental_read_interface.cpp +++ b/Source/src/incremental_read_interface.cpp @@ -20,7 +20,7 @@ using namespace MafiaNet; -unsigned int IncrementalReadInterface::GetFilePart( const char *filename, unsigned int startReadBytes, unsigned int numBytesToRead, void *preallocatedDestination, FileListNodeContext context) +unsigned int IncrementalReadInterface::GetFilePart( const char *filename, unsigned int startReadBytes, unsigned int numBytesToRead, void *preallocatedDestination, FileListNodeContext /*context*/) { FILE *fp; if (fopen_s(&fp, filename, "rb")!=0) diff --git a/Source/src/linux_adapter.cpp b/Source/src/linux_adapter.cpp index b0610f8e8..5b37b08df 100644 --- a/Source/src/linux_adapter.cpp +++ b/Source/src/linux_adapter.cpp @@ -121,7 +121,7 @@ int sprintf_s(char *buffer, size_t sizeOfBuffer, const char *format, ...) return -1; } - if (numCharsWritten >= sizeOfBuffer) { + if (static_cast(numCharsWritten) >= sizeOfBuffer) { buffer[0] = '\0'; // buffer too small, write empty string to ensure terminating \0-char } @@ -249,7 +249,9 @@ errno_t strncpy_s(char *strDest, size_t numberOfElements, const char *strSource, } } - (void)strncpy(strDest, strSource, numChars); + // numChars never exceeds the source length, so this is the copy strncpy did, without + // its zero padding (there is none to do) or its truncation warning. + memcpy(strDest, strSource, numChars); // enforce the trailing \0 strDest[numChars] = '\0'; @@ -273,14 +275,15 @@ int vsnprintf_s(char *buffer, size_t sizeOfBuffer, size_t count, const char *for } int numCharsWritten = vsnprintf(buffer, maxChars, format, argptr); - if (numCharsWritten >= maxChars) { + if ((numCharsWritten < 0) || (static_cast(numCharsWritten) >= maxChars)) { if (count != _TRUNCATE) { buffer[0] = '\0'; // buffer set to empty string return -1; } - // truncation occurred, add terminating \0 - buffer[sizeOfBuffer] = '\0'; + // Truncation occurred. vsnprintf already terminated the output inside the + // maxChars (== sizeOfBuffer) bytes it was given; this used to also write + // buffer[sizeOfBuffer], one byte past the end. } return numCharsWritten; diff --git a/Source/src/log_command_parser.cpp b/Source/src/log_command_parser.cpp index b2bea50f6..7f6e031a5 100644 --- a/Source/src/log_command_parser.cpp +++ b/Source/src/log_command_parser.cpp @@ -112,8 +112,7 @@ void LogCommandParser::SendHelp(TransportInterface *transport, const SystemAddre } void LogCommandParser::AddChannel(const char *channelName) { - unsigned channelIndex=0; - channelIndex = GetChannelIndexFromName(channelName); + [[maybe_unused]] unsigned channelIndex = GetChannelIndexFromName(channelName); // only checked by RakAssert (debug builds) // Each channel can only be added once. RakAssert(channelIndex==(unsigned)-1); diff --git a/Source/src/network_id_manager.cpp b/Source/src/network_id_manager.cpp index c9ae900a0..7c7f8f62f 100644 --- a/Source/src/network_id_manager.cpp +++ b/Source/src/network_id_manager.cpp @@ -56,8 +56,8 @@ NetworkIDObject *NetworkIDManager::GET_BASE_OBJECT_FROM_ID(NetworkID x) } NetworkID NetworkIDManager::GetNewNetworkID(void) { - while (GET_BASE_OBJECT_FROM_ID(++startingOffset)) - ; + while (GET_BASE_OBJECT_FROM_ID(++startingOffset)) + ; if (startingOffset==UNASSIGNED_NETWORK_ID) { while (GET_BASE_OBJECT_FROM_ID(++startingOffset)) diff --git a/Source/src/osx_adapter.cpp b/Source/src/osx_adapter.cpp index 5b894781e..92cf20dba 100644 --- a/Source/src/osx_adapter.cpp +++ b/Source/src/osx_adapter.cpp @@ -136,7 +136,7 @@ int sprintf_s(char *buffer, size_t sizeOfBuffer, const char *format, ...) return -1; } - if (numCharsWritten >= sizeOfBuffer) { + if (static_cast(numCharsWritten) >= sizeOfBuffer) { buffer[0] = '\0'; // buffer too small, write empty string to ensure terminating \0-char } @@ -264,7 +264,9 @@ errno_t strncpy_s(char *strDest, size_t numberOfElements, const char *strSource, } } - (void)strncpy(strDest, strSource, numChars); + // numChars never exceeds the source length, so this is the copy strncpy did, without + // its zero padding (there is none to do) or its truncation warning. + memcpy(strDest, strSource, numChars); // enforce the trailing \0 strDest[numChars] = '\0'; @@ -288,14 +290,15 @@ int vsnprintf_s(char *buffer, size_t sizeOfBuffer, size_t count, const char *for } int numCharsWritten = vsnprintf(buffer, maxChars, format, argptr); - if (numCharsWritten >= maxChars) { + if ((numCharsWritten < 0) || (static_cast(numCharsWritten) >= maxChars)) { if (count != _TRUNCATE) { buffer[0] = '\0'; // buffer set to empty string return -1; } - // truncation occurred, add terminating \0 - buffer[sizeOfBuffer] = '\0'; + // Truncation occurred. vsnprintf already terminated the output inside the + // maxChars (== sizeOfBuffer) bytes it was given; this used to also write + // buffer[sizeOfBuffer], one byte past the end. } return numCharsWritten; diff --git a/Source/src/packet_logger.cpp b/Source/src/packet_logger.cpp index effdd7143..c4af03f30 100644 --- a/Source/src/packet_logger.cpp +++ b/Source/src/packet_logger.cpp @@ -163,8 +163,13 @@ void PacketLogger::FormatLine(char* into, const char* dir, const char* type, uns sprintf_s(str3, "%5u", reliableMessageNumber); } -#pragma warning(push) -#pragma warning(disable:4996) + // This legacy overload is given no size for into, and the line length depends on the + // user-set prefix/suffix, so it cannot be bounded here; the caller owns the size + // contract. Use the intoLength overload instead. +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#endif sprintf(into, "%s,%s%s,%s,%s,%5u,%s,%u,%" PRINTF_64_BIT_MODIFIER "u,%s,%s,%i,%i,%i,%i,%s," , localtime , prefix @@ -183,7 +188,9 @@ void PacketLogger::FormatLine(char* into, const char* dir, const char* type, uns , orderingIndex , suffix ); -#pragma warning(pop) +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic pop +#endif } void PacketLogger::OnDirectSocketSend(const char *data, const BitSize_t bitsUsed, SystemAddress remoteSystemAddress) { diff --git a/Source/src/packet_output_window_logger.cpp b/Source/src/packet_output_window_logger.cpp index 79d024ac6..b3365b116 100644 --- a/Source/src/packet_output_window_logger.cpp +++ b/Source/src/packet_output_window_logger.cpp @@ -34,7 +34,7 @@ PacketOutputWindowLogger::PacketOutputWindowLogger() PacketOutputWindowLogger::~PacketOutputWindowLogger() { } -void PacketOutputWindowLogger::WriteLog(const char *str) +void PacketOutputWindowLogger::WriteLog([[maybe_unused]] const char *str) // only Windows has an output window { #if defined(_WIN32) diff --git a/Source/src/peer.cpp b/Source/src/peer.cpp index 5bdce011f..c50fbfadb 100644 --- a/Source/src/peer.cpp +++ b/Source/src/peer.cpp @@ -3980,36 +3980,12 @@ RakPeer::RemoteSystemStruct * RakPeer::AssignSystemAddressToRemoteSystemList( co } else { - char str[256]; - bindingAddress.ToString(true,str,static_cast(256)); - // See if this is an internal IP address. - // If so, force binding on it so we reply on the same IP address as they sent to. - unsigned int ipListIndex, foundIndex=(unsigned int)-1; - - for (ipListIndex=0; ipListIndex < MAXIMUM_NUMBER_OF_INTERNAL_IDS; ipListIndex++) - { - if (ipList[ipListIndex]==UNASSIGNED_SYSTEM_ADDRESS) - break; - - if (bindingAddress.EqualsExcludingPort(ipList[ipListIndex])) - { - foundIndex=ipListIndex; - break; - } - } - - // 06/26/09 Unconfirmed report that Vista firewall blocks the reply if we force a binding - // For now use the incoming socket only - // Originally this code was to force a machine with multiple IP addresses to reply back on the IP - // that the datagram came in on - //if (1 || foundIndex==(unsigned int)-1) - //{ - // Must not be an internal LAN address. Just use whatever socket it came in on - remoteSystem->rakNetSocket=incomingRakNetSocket; - //} - //else - //{ - //} + // Originally this searched ipList for the internal address the datagram came in on and + // forced the reply onto it, so a multi-homed machine answers from the same IP. That was + // disabled upstream (06/26/09, unconfirmed report that the Vista firewall blocks the reply + // if we force a binding), which left the search computing an index nobody read. Always + // reply on the socket the datagram came in on. + remoteSystem->rakNetSocket=incomingRakNetSocket; } for ( j = 0; j < (unsigned) PING_TIMES_ARRAY_SIZE; j++ ) @@ -4357,7 +4333,8 @@ void RakPeer::CountRemoteInitiatedPeers(const SystemAddress &address, unsigned i break; case RemoteSystemStruct::EXCHANGING_SESSION_DATA: exchanging++; - // fallthrough: a peer in the handshake is pending too + // A peer in the handshake is pending too. + [[fallthrough]]; case RemoteSystemStruct::UNVERIFIED_SENDER: case RemoteSystemStruct::HANDLING_CONNECTION_REQUEST: pending++; @@ -4963,7 +4940,7 @@ bool ProcessOfflineNetworkPacket( SystemAddress systemAddress, const char *data, else if ( ((unsigned char)data[0] == ID_UNCONNECTED_PING || (unsigned char)data[0] == ID_UNCONNECTED_PING_OPEN_CONNECTIONS) && - length >= sizeof(unsigned char) + sizeof(MafiaNet::Time) + sizeof(OFFLINE_MESSAGE_DATA_ID)) + (size_t) length >= sizeof(unsigned char) + sizeof(MafiaNet::Time) + sizeof(OFFLINE_MESSAGE_DATA_ID)) { *isOfflineMessage=memcmp(data+sizeof(unsigned char) + sizeof(MafiaNet::Time), OFFLINE_MESSAGE_DATA_ID, sizeof(OFFLINE_MESSAGE_DATA_ID))==0; } @@ -5009,7 +4986,7 @@ bool ProcessOfflineNetworkPacket( SystemAddress systemAddress, const char *data, // These are all messages from unconnected systems. Messages here can be any size, but are never processed from connected systems. if ( ( (unsigned char) data[ 0 ] == ID_UNCONNECTED_PING_OPEN_CONNECTIONS - || (unsigned char)(data)[0] == ID_UNCONNECTED_PING) && length >= sizeof(unsigned char)+sizeof(MafiaNet::Time)+sizeof(OFFLINE_MESSAGE_DATA_ID) ) + || (unsigned char)(data)[0] == ID_UNCONNECTED_PING) && (size_t) length >= sizeof(unsigned char)+sizeof(MafiaNet::Time)+sizeof(OFFLINE_MESSAGE_DATA_ID) ) { if ( (unsigned char)(data)[0] == ID_UNCONNECTED_PING || rakPeer->AllowIncomingConnections() ) // Open connections with players @@ -5254,7 +5231,7 @@ bool ProcessOfflineNetworkPacket( SystemAddress systemAddress, const char *data, RakNetGUID guid; bs.Read(guid); SystemAddress bindingAddress; - bool b = bs.Read(bindingAddress); + [[maybe_unused]] bool b = bs.Read(bindingAddress); // only checked by RakAssert (debug builds) RakAssert(b); uint16_t mtu; b=bs.Read(mtu); diff --git a/Source/src/relay_plugin.cpp b/Source/src/relay_plugin.cpp index 0a9be4ed4..5c258b566 100644 --- a/Source/src/relay_plugin.cpp +++ b/Source/src/relay_plugin.cpp @@ -336,7 +336,7 @@ void RelayPlugin::NotifyUsersInRoom(RP_Group *room, int msg, const RakString& me SendUnified(&bsOut, MafiaNet::Priority::High, MafiaNet::Reliability::ReliableOrdered, 0, room->usersInRoom[i].guid, false); } } -void RelayPlugin::SendMessageToRoom(StrAndGuidAndRoom **strAndGuidSender, BitStream* message) +void RelayPlugin::SendMessageToRoom(StrAndGuidAndRoom **strAndGuidSender, BitStream* message, MafiaNet::Priority priority, MafiaNet::Reliability reliability, char orderingChannel) { if ((*strAndGuidSender)->currentRoom.IsEmpty()) return; @@ -357,7 +357,7 @@ void RelayPlugin::SendMessageToRoom(StrAndGuidAndRoom **strAndGuidSender, BitStr for (unsigned int j=0; j < room->usersInRoom.Size(); j++) { if (room->usersInRoom[j].guid!=(*strAndGuidSender)->guid) - SendUnified(&bsOut, MafiaNet::Priority::High, MafiaNet::Reliability::ReliableOrdered, 0, room->usersInRoom[j].guid, false); + SendUnified(&bsOut, priority, reliability, orderingChannel, room->usersInRoom[j].guid, false); } break; @@ -397,7 +397,10 @@ void RelayPlugin::OnGroupMessageFromClient(Packet *packet) StrAndGuidAndRoom **strAndGuidSender = guidToStrHash.Peek(packet->guid); if (strAndGuidSender) { - SendMessageToRoom(strAndGuidSender,&bsData); + // Relay with the priority, reliability and channel the sender asked for in + // SendGroupMessage(), as RPE_MESSAGE_TO_SERVER_FROM_CLIENT already does. These were + // read and then dropped, so every group message went out High/ReliableOrdered/0. + SendMessageToRoom(strAndGuidSender,&bsData,priority,reliability,orderingChannel); } } void RelayPlugin::OnJoinGroupRequestFromClient(Packet *packet) diff --git a/Source/src/reliability_layer.cpp b/Source/src/reliability_layer.cpp index 17a09b80d..cfe895cd7 100644 --- a/Source/src/reliability_layer.cpp +++ b/Source/src/reliability_layer.cpp @@ -433,10 +433,15 @@ MafiaNet::TimeMS ReliabilityLayer::GetTimeoutTime() //------------------------------------------------------------------------------------------------------- void ReliabilityLayer::InitializeVariables() { - memset( orderedWriteIndex, 0, NUMBER_OF_ORDERED_STREAMS * sizeof(OrderingIndexType)); - memset( sequencedWriteIndex, 0, NUMBER_OF_ORDERED_STREAMS * sizeof(OrderingIndexType) ); - memset( orderedReadIndex, 0, NUMBER_OF_ORDERED_STREAMS * sizeof(OrderingIndexType) ); - memset( highestSequencedReadIndex, 0, NUMBER_OF_ORDERED_STREAMS * sizeof(OrderingIndexType) ); + // OrderingIndexType (uint24_t) is a wrapper around one uint32_t; assigning 0 leaves the + // same all-zero state the memset used to, without memset-ing a non-trivial class. + for (unsigned int stream=0; stream < NUMBER_OF_ORDERED_STREAMS; stream++) + { + orderedWriteIndex[stream]=0; + sequencedWriteIndex[stream]=0; + orderedReadIndex[stream]=0; + highestSequencedReadIndex[stream]=0; + } memset( &statistics, 0, sizeof( statistics ) ); memset( &heapIndexOffsets, 0, sizeof( heapIndexOffsets ) ); @@ -495,7 +500,7 @@ void ReliabilityLayer::InitializeVariables() datagramHistoryPopCount=0; InitHeapWeights(); - for (int i=0; i < MafiaNet::NUMBER_OF_PRIORITIES; i++) + for (unsigned int i=0; i < MafiaNet::NUMBER_OF_PRIORITIES; i++) { statistics.messageInSendBuffer[i]=0; statistics.bytesInSendBuffer[i]=0.0; @@ -995,7 +1000,7 @@ bool ReliabilityLayer::HandleSocketReceiveFromConnectedPlayer( // or an older out of order packet // The subtraction unsigned overflow is intentional holeCount = (DatagramSequenceNumberType)(internalPacket->reliableMessageNumber - receivedPacketsBaseIndex); - const DatagramSequenceNumberType typeRange = (DatagramSequenceNumberType)(const uint32_t)-1; + const DatagramSequenceNumberType typeRange = (DatagramSequenceNumberType)(uint32_t)-1; // TESTING1 // printf("waiting on reliableMessageNumber=%i holeCount=%i datagramNumber=%i\n", receivedPacketsBaseIndex.val, holeCount.val, dhf.datagramNumber.val); @@ -1079,7 +1084,7 @@ bool ReliabilityLayer::HandleSocketReceiveFromConnectedPlayer( hasReceivedPacketQueue.Push(false, _FILE_AND_LINE_ ); // Got the packet #ifdef _DEBUG // If this assert hits then DatagramSequenceNumberType has overflowed - RakAssert(hasReceivedPacketQueue.Size() < (unsigned int)((DatagramSequenceNumberType)(const uint32_t)(-1))); + RakAssert(hasReceivedPacketQueue.Size() < (unsigned int)((DatagramSequenceNumberType)(uint32_t)(-1))); #endif } @@ -1751,7 +1756,7 @@ void ReliabilityLayer::UpdateInternal( RakNetSocket2 *s, SystemAddress &systemAd { (void) MTUSize; - MafiaNet::TimeMS timeMs; + [[maybe_unused]] MafiaNet::TimeMS timeMs; // only read by the _DEBUG simulated-lag queue below #if CC_TIME_TYPE_BYTES==4 time/=1000; timeMs=time; @@ -2807,7 +2812,7 @@ InternalPacket* ReliabilityLayer::CreateInternalPacketFromBitStream(MafiaNet::Bi // (Incoming data may be all zeros due to padding) bitStream->AlignReadToByteBoundary(); // Potentially unaligned bitStream->ReadBits( ( unsigned char* ) ( &( tempChar ) ), 3 ); - internalPacket->reliability = ( const MafiaNet::Reliability ) tempChar; + internalPacket->reliability = ( MafiaNet::Reliability ) tempChar; readSuccess=bitStream->Read(hasSplitPacket); // Read 1 bit to indicate if splitPacketCount>0 bitStream->AlignReadToByteBoundary(); unsigned short s; bitStream->ReadAlignedVar16((char*)&s); internalPacket->dataBitLength=s; // Length of message (2 bytes) @@ -2822,7 +2827,7 @@ InternalPacket* ReliabilityLayer::CreateInternalPacketFromBitStream(MafiaNet::Bi ) bitStream->Read(internalPacket->reliableMessageNumber); // Message sequence number else - internalPacket->reliableMessageNumber=(MessageNumberType)(const uint32_t)-1; + internalPacket->reliableMessageNumber=(MessageNumberType)(uint32_t)-1; bitStream->AlignReadToByteBoundary(); // Potentially nothing else to Read if ( internalPacket->reliability == MafiaNet::Reliability::UnreliableSequenced || @@ -3004,7 +3009,7 @@ void ReliabilityLayer::DeleteSequencedPacketsInList( unsigned char orderingChann //------------------------------------------------------------------------------------------------------- bool ReliabilityLayer::IsOlderOrderedPacket( OrderingIndexType newPacketOrderingIndex, OrderingIndexType waitingForPacketOrderingIndex ) { - OrderingIndexType maxRange = (OrderingIndexType) (const uint32_t)-1; + OrderingIndexType maxRange = (OrderingIndexType) (uint32_t)-1; if ( waitingForPacketOrderingIndex > maxRange/(OrderingIndexType)2 ) { @@ -3013,16 +3018,17 @@ bool ReliabilityLayer::IsOlderOrderedPacket( OrderingIndexType newPacketOrdering return true; } } - else + { if ( newPacketOrderingIndex >= ( OrderingIndexType ) ( waitingForPacketOrderingIndex - (( OrderingIndexType ) maxRange/(OrderingIndexType)2+(OrderingIndexType)1) ) || newPacketOrderingIndex < waitingForPacketOrderingIndex ) { return true; } + } - // Old packet - return false; + // Old packet + return false; } //------------------------------------------------------------------------------------------------------- @@ -3619,8 +3625,10 @@ InternalPacket * ReliabilityLayer::CreateInternalPacketCopy( InternalPacket *ori { InternalPacket * copy = AllocateFromInternalPacketPool(); #ifdef _DEBUG - // Remove accessing undefined memory error - memset( copy, 255, sizeof( InternalPacket ) ); + // Remove accessing undefined memory error: deliberately poison every byte so a field this + // function forgets to set shows up as 0xFF. The only non-trivial members are uint24_t + // wrappers around a uint32_t, so a byte fill is sound; the void* cast says so to the compiler. + memset( static_cast(copy), 255, sizeof( InternalPacket ) ); #endif // Copy over our chunk of data @@ -4004,7 +4012,7 @@ datagramMessageIDPool.Release(d); InternalPacket* ReliabilityLayer::AllocateFromInternalPacketPool(void) { InternalPacket *ip = internalPacketPool.Allocate( _FILE_AND_LINE_ ); - ip->reliableMessageNumber = (MessageNumberType) (const uint32_t)-1; + ip->reliableMessageNumber = (MessageNumberType) (uint32_t)-1; ip->messageNumberAssigned=false; ip->nextActionTime = 0; ip->splitPacketCount = 0; @@ -4255,7 +4263,7 @@ BitSize_t ReliabilityLayer::GetMaxDatagramSizeExcludingMessageHeaderBits(void) //------------------------------------------------------------------------------------------------------- void ReliabilityLayer::InitHeapWeights(void) { - for (int priorityLevel=0; priorityLevel < MafiaNet::NUMBER_OF_PRIORITIES; priorityLevel++) + for (int priorityLevel=0; priorityLevel < static_cast(MafiaNet::NUMBER_OF_PRIORITIES); priorityLevel++) outgoingPacketBufferNextWeights[priorityLevel]=(1<valueOverLastSecond[ACTUAL_BYTES_RECEIVED], s->packetlossLastSecond*100.0f ); -#pragma warning(pop) } else if (verbosityLevel == 1) { -#pragma warning(push) -#pragma warning(disable:4996) sprintf(buffer, "Actual bytes per second sent %" PRINTF_64_BIT_MODIFIER "u\n" "Actual bytes per second received %" PRINTF_64_BIT_MODIFIER "u\n" @@ -78,7 +78,6 @@ void RAK_DLL_EXPORT MafiaNet::StatisticsToString(RakNetStatistics *s, char *buff s->packetlossTotal*100.0f, (long long unsigned int) (uint64_t)((MafiaNet::GetTimeUS() - s->connectionStartTime) / 1000000) ); -#pragma warning(pop) if (s->BPSLimitByCongestionControl != 0) { @@ -88,10 +87,7 @@ void RAK_DLL_EXPORT MafiaNet::StatisticsToString(RakNetStatistics *s, char *buff (long long unsigned int) s->BPSLimitByCongestionControl, 100.0f * s->valueOverLastSecond[ACTUAL_BYTES_SENT] / s->BPSLimitByCongestionControl ); -#pragma warning(push) -#pragma warning(disable:4996) strcat(buffer, buff2); -#pragma warning(pop) } if (s->BPSLimitByOutgoingBandwidthLimit != 0) { @@ -101,16 +97,11 @@ void RAK_DLL_EXPORT MafiaNet::StatisticsToString(RakNetStatistics *s, char *buff (long long unsigned int) s->BPSLimitByOutgoingBandwidthLimit, 100.0f * s->valueOverLastSecond[ACTUAL_BYTES_SENT] / s->BPSLimitByOutgoingBandwidthLimit ); -#pragma warning(push) -#pragma warning(disable:4996) strcat(buffer, buff2); -#pragma warning(pop) } } else { -#pragma warning(push) -#pragma warning(disable:4996) sprintf(buffer, "Actual bytes per second sent %" PRINTF_64_BIT_MODIFIER "u\n" "Actual bytes per second received %" PRINTF_64_BIT_MODIFIER "u\n" @@ -155,7 +146,6 @@ void RAK_DLL_EXPORT MafiaNet::StatisticsToString(RakNetStatistics *s, char *buff s->packetlossTotal*100.0f, (long long unsigned int) (uint64_t)((MafiaNet::GetTimeUS() - s->connectionStartTime) / 1000000) ); -#pragma warning(pop) if (s->BPSLimitByCongestionControl != 0) { @@ -165,10 +155,7 @@ void RAK_DLL_EXPORT MafiaNet::StatisticsToString(RakNetStatistics *s, char *buff (long long unsigned int) s->BPSLimitByCongestionControl, 100.0f * s->valueOverLastSecond[ACTUAL_BYTES_SENT] / s->BPSLimitByCongestionControl ); -#pragma warning(push) -#pragma warning(disable:4996) strcat(buffer, buff2); -#pragma warning(pop) } if (s->BPSLimitByOutgoingBandwidthLimit != 0) { @@ -178,13 +165,13 @@ void RAK_DLL_EXPORT MafiaNet::StatisticsToString(RakNetStatistics *s, char *buff (long long unsigned int) s->BPSLimitByOutgoingBandwidthLimit, 100.0f * s->valueOverLastSecond[ACTUAL_BYTES_SENT] / s->BPSLimitByOutgoingBandwidthLimit ); -#pragma warning(push) -#pragma warning(disable:4996) strcat(buffer, buff2); -#pragma warning(pop) } } } +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic pop +#endif void RAK_DLL_EXPORT MafiaNet::StatisticsToString( RakNetStatistics *s, char *buffer, size_t bufferLength, int verbosityLevel ) { if ( s == 0 ) diff --git a/Source/src/statistics_history.cpp b/Source/src/statistics_history.cpp index 3ba358c6c..325b379bd 100644 --- a/Source/src/statistics_history.cpp +++ b/Source/src/statistics_history.cpp @@ -533,14 +533,12 @@ void StatisticsHistory::TimeAndValueQueue::ResizeSampleSet( int maxSamples, Data unsigned int dataIndex; Time timeBoundary; StatisticsHistory::TimeAndValue currentSum; - Time currentTime; SHValueType numSamples; Time endTime; numSamples=0; endTime = values[values.Size()-1].time; dataIndex=0; - currentTime=values[0].time; currentSum.val=0; currentSum.time=values[0].time + interval / 2; timeBoundary = values[0].time + interval; @@ -664,8 +662,8 @@ SHValueType StatisticsHistory::TimeAndValueQueue::Interpolate(StatisticsHistory: return (t1.val + t2.val) / 2; // if (t2.time > t1.time) // { - SHValueType slope = (t2.val - t1.val) / ((SHValueType) t2.time - (SHValueType) t1.time); - return t1.val + slope * ((SHValueType) time - (SHValueType) t1.time); + SHValueType slope = (t2.val - t1.val) / ((SHValueType) t2.time - (SHValueType) t1.time); + return t1.val + slope * ((SHValueType) time - (SHValueType) t1.time); // } // else // { diff --git a/Source/src/string.cpp b/Source/src/string.cpp index a92399952..dba38c5c8 100644 --- a/Source/src/string.cpp +++ b/Source/src/string.cpp @@ -31,7 +31,7 @@ using namespace MafiaNet; //DataStructures::MemoryPool RakString::pool; -RakString::SharedString RakString::emptyString={0,0,0,(char*) "",(char*) ""}; +RakString::SharedString RakString::emptyString={0,0,0,(char*) "",(char*) "",{}}; //RakString::SharedString *RakString::sharedStringFreeList=0; //unsigned int RakString::sharedStringFreeListAllocationCount=0; DataStructures::List RakString::freeList; @@ -1234,7 +1234,7 @@ void RakString::Serialize(const char *str, BitStream *bs) { unsigned short l = (unsigned short) strlen(str); bs->Write(l); - bs->WriteAlignedBytes((const unsigned char*) str, (const unsigned int) l); + bs->WriteAlignedBytes((const unsigned char*) str, (unsigned int) l); } void RakString::SerializeCompressed(BitStream *bs, uint8_t languageId, bool writeLanguageId) const { diff --git a/Source/src/super_fast_hash.cpp b/Source/src/super_fast_hash.cpp index 636a3a4a5..a6e95d76a 100644 --- a/Source/src/super_fast_hash.cpp +++ b/Source/src/super_fast_hash.cpp @@ -123,13 +123,16 @@ uint32_t SuperFastHashFilePtr (FILE *fp) char readBlock[INCREMENTAL_READ_BLOCK]; while (bytesRemaining>=(int) sizeof(readBlock)) { - fread(readBlock, sizeof(readBlock), 1, fp); + // A short read would hash uninitialized stack bytes; report it like an unopenable file. + if (fread(readBlock, sizeof(readBlock), 1, fp) != 1) + return 0; lastHash=SuperFastHashIncremental (readBlock, (int) sizeof(readBlock), lastHash ); bytesRemaining-=(int) sizeof(readBlock); } if (bytesRemaining>0) { - fread(readBlock, bytesRemaining, 1, fp); + if (fread(readBlock, bytesRemaining, 1, fp) != 1) + return 0; lastHash=SuperFastHashIncremental (readBlock, bytesRemaining, lastHash ); } return lastHash; diff --git a/Source/src/table_serializer.cpp b/Source/src/table_serializer.cpp index 6f679cfdd..b084bf794 100644 --- a/Source/src/table_serializer.cpp +++ b/Source/src/table_serializer.cpp @@ -186,7 +186,7 @@ void TableSerializer::SerializeCell(MafiaNet::BitStream *out, DataStructures::Ta unsigned binaryLength; binaryLength=(unsigned)cell->i; out->Write(binaryLength); - out->WriteAlignedBytes((const unsigned char*) cell->c, (const unsigned int) cell->i); + out->WriteAlignedBytes((const unsigned char*) cell->c, (unsigned int) cell->i); } } } @@ -245,7 +245,7 @@ void TableSerializer::SerializeFilterQuery(MafiaNet::BitStream *in, DataStructur if (query->cellValue->isEmpty==false) { in->Write(query->cellValue->i); - in->WriteAlignedBytesSafe((const char*)query->cellValue->c,(const unsigned int)query->cellValue->i,10000000); // Sanity check to max binary cell of 10 megabytes + in->WriteAlignedBytesSafe((const char*)query->cellValue->c,(unsigned int)query->cellValue->i,10000000); // Sanity check to max binary cell of 10 megabytes in->Write(query->cellValue->ptr); } diff --git a/Source/src/tcp_interface.cpp b/Source/src/tcp_interface.cpp index f1b2640e2..043d7f4a3 100644 --- a/Source/src/tcp_interface.cpp +++ b/Source/src/tcp_interface.cpp @@ -271,9 +271,9 @@ void TCPInterface::Stop(void) while ( threadRunning.GetValue()>0 ) RakSleep(15); - RakSleep(100); + RakSleep(100); - listenSocket=0; + listenSocket=0; // Stuff from here on to the end of the function is not threadsafe for (i=0; i < remoteClientsLength; i++) @@ -793,9 +793,9 @@ __TCPSOCKET__ TCPInterface::SocketConnect(const char* host, unsigned short remot return 0; - __TCPSOCKET__ sockfd = socket__(AF_INET, SOCK_STREAM, 0); - if (sockfd < 0) - return 0; + __TCPSOCKET__ sockfd = socket__(AF_INET, SOCK_STREAM, 0); + if (sockfd < 0) + return 0; memset(&serverAddress, 0, sizeof(serverAddress)); serverAddress.sin_family = AF_INET; @@ -886,11 +886,13 @@ RAK_THREAD_DECLARATION(MafiaNet::ConnectionAttemptLoop) TCPInterface *tcpInterface = s->tcpInterface; int newRemoteClientIndex=systemAddress.systemIndex; unsigned short socketFamily = s->socketFamily; + char bindAddress[sizeof(s->bindAddress)]; + memcpy(bindAddress, s->bindAddress, sizeof(bindAddress)); MafiaNet::OP_DELETE(s, _FILE_AND_LINE_); char str1[64]; systemAddress.ToString(false, str1, static_cast(64)); - __TCPSOCKET__ sockfd = tcpInterface->SocketConnect(str1, systemAddress.GetPort(), socketFamily, s->bindAddress); + __TCPSOCKET__ sockfd = tcpInterface->SocketConnect(str1, systemAddress.GetPort(), socketFamily, bindAddress); if (sockfd==0) { tcpInterface->remoteClients[newRemoteClientIndex].isActiveMutex.Lock(); diff --git a/Source/src/types.cpp b/Source/src/types.cpp index 5f95b0362..8e8018845 100644 --- a/Source/src/types.cpp +++ b/Source/src/types.cpp @@ -132,13 +132,7 @@ SocketDescriptor::SocketDescriptor(unsigned short _port, const char *_hostAddres // All systems must use the same value for this variable. //bool RAK_DLL_EXPORT NetworkID::peerToPeerMode=false; -SystemAddress& SystemAddress::operator = ( const SystemAddress& input ) -{ - memcpy(&address, &input.address, sizeof(address)); - systemIndex = input.systemIndex; - debugPort = input.debugPort; - return *this; -} +SystemAddress& SystemAddress::operator = ( const SystemAddress& input ) = default; bool SystemAddress::EqualsExcludingPort( const SystemAddress& right ) const { return (address.addr4.sin_family==AF_INET && address.addr4.sin_addr.s_addr==right.address.addr4.sin_addr.s_addr) @@ -279,10 +273,7 @@ void SystemAddress::ToString_Old(bool writePort, char *dest, char portDelineator { if (*this == UNASSIGNED_SYSTEM_ADDRESS) { -#pragma warning(push) -#pragma warning(disable:4996) strcpy(dest, "UNASSIGNED_SYSTEM_ADDRESS"); -#pragma warning(pop) return; } @@ -294,16 +285,10 @@ void SystemAddress::ToString_Old(bool writePort, char *dest, char portDelineator in.s_addr = address.addr4.sin_addr.s_addr; char buf[1024]; inet_ntop(AF_INET, &in, buf, 1024); -#pragma warning(push) -#pragma warning(disable:4996) strcpy(dest, buf); -#pragma warning(pop) if (writePort) { -#pragma warning(push) -#pragma warning(disable:4996) strcat(dest, portStr); -#pragma warning(pop) Itoa(GetPort(), dest + strlen(dest), 10); } } @@ -353,10 +338,7 @@ void SystemAddress::ToString_New(bool writePort, char *dest, char portDelineator if (*this == UNASSIGNED_SYSTEM_ADDRESS) { -#pragma warning(push) -#pragma warning(disable:4996) strcpy(dest, "UNASSIGNED_SYSTEM_ADDRESS"); -#pragma warning(pop) return; } @@ -378,10 +360,7 @@ void SystemAddress::ToString_New(bool writePort, char *dest, char portDelineator unsigned char ch[2]; ch[0] = portDelineator; ch[1] = 0; -#pragma warning(push) -#pragma warning(disable:4996) strcat(dest, (const char*)ch); -#pragma warning(pop) Itoa(ntohs(address.addr4.sin_port), dest + strlen(dest), 10); } } @@ -730,16 +709,11 @@ bool RakNetGUID::operator < ( const RakNetGUID& right ) const void RakNetGUID::ToString(char *dest) const { if (*this == UNASSIGNED_RAKNET_GUID) -#pragma warning(push) -#pragma warning(disable:4996) strcpy(dest, "UNASSIGNED_RAKNET_GUID"); -#pragma warning(pop) else - //sprintf_s(dest, destLength, "%u.%u.%u.%u.%u.%u", g[0], g[1], g[2], g[3], g[4], g[5]); -#pragma warning(push) -#pragma warning(disable:4996) - sprintf(dest, "%" PRINTF_64_BIT_MODIFIER "u", (long long unsigned int) g); -#pragma warning(pop) + // dest has no size in this legacy overload. The output is bounded by the value + // itself: UINT64_MAX is 20 decimal digits, plus the terminator. + snprintf(dest, 21, "%" PRINTF_64_BIT_MODIFIER "u", (long long unsigned int) g); // sprintf_s(dest, destLength, "%u.%u.%u.%u.%u.%u", g[0], g[1], g[2], g[3], g[4], g[5]); } void RakNetGUID::ToString(char *dest, size_t destLength) const diff --git a/Source/src/voice.cpp b/Source/src/voice.cpp index e877879ed..41b7f7a55 100644 --- a/Source/src/voice.cpp +++ b/Source/src/voice.cpp @@ -165,7 +165,11 @@ void RakVoice::SetRelayHost(bool enable) bool RakVoice::SetOrderingChannels(char frameChannel, char controlChannel) { - if (frameChannel < 0 || frameChannel >= NUMBER_OF_ORDERED_STREAMS || controlChannel < 0 || controlChannel >= NUMBER_OF_ORDERED_STREAMS) + // char is unsigned on ARM; compare as signed so a negative channel is rejected + // the same way on every platform. + const int frame = static_cast(frameChannel); + const int control = static_cast(controlChannel); + if (frame < 0 || frame >= NUMBER_OF_ORDERED_STREAMS || control < 0 || control >= NUMBER_OF_ORDERED_STREAMS) return false; frameOrderingChannel = frameChannel; controlOrderingChannel = controlChannel; diff --git a/Source/src/wstring.cpp b/Source/src/wstring.cpp index 72e1a042f..5eebdca7c 100644 --- a/Source/src/wstring.cpp +++ b/Source/src/wstring.cpp @@ -102,7 +102,7 @@ RakWString& RakWString::operator = ( const wchar_t * const str ) } RakWString& RakWString::operator = ( wchar_t *str ) { - *this = ( const wchar_t * const) str; + *this = ( const wchar_t *) str; return *this; } RakWString& RakWString::operator = ( const char * const str ) @@ -141,7 +141,7 @@ RakWString& RakWString::operator = ( const char * const str ) } RakWString& RakWString::operator = ( char *str ) { - *this = ( const char * const) str; + *this = ( const char *) str; return *this; } RakWString& RakWString::operator +=( const RakWString& right) @@ -205,7 +205,7 @@ RakWString& RakWString::operator += ( const wchar_t * const right ) } RakWString& RakWString::operator += ( wchar_t *right ) { - return *this += (const wchar_t * const) right; + return *this += (const wchar_t *) right; } bool RakWString::operator==(const RakWString &right) const { @@ -301,7 +301,7 @@ void RakWString::Serialize(const wchar_t * const str, BitStream *bs) multiByteBuffer = (char*) rakMalloc_Ex(allocated, _FILE_AND_LINE_); size_t used = wcstombs(multiByteBuffer, str, allocated); bs->WriteCasted(used); - bs->WriteAlignedBytes((const unsigned char*) multiByteBuffer,(const unsigned int) used); + bs->WriteAlignedBytes((const unsigned char*) multiByteBuffer,(unsigned int) used); rakFree_Ex(multiByteBuffer, _FILE_AND_LINE_); #else size_t mbByteLength = wcslen(str); @@ -326,7 +326,7 @@ bool RakWString::Deserialize(BitStream *bs) #if 0 char *multiByteBuffer; multiByteBuffer = (char*) rakMalloc_Ex(mbByteLength+1, _FILE_AND_LINE_); - bool result = bs->ReadAlignedBytes((unsigned char*) multiByteBuffer,(const unsigned int) mbByteLength); + bool result = bs->ReadAlignedBytes((unsigned char*) multiByteBuffer,(unsigned int) mbByteLength); if (result==false) { rakFree_Ex(multiByteBuffer, _FILE_AND_LINE_); @@ -365,7 +365,7 @@ bool RakWString::Deserialize(wchar_t *str, BitStream *bs) #if 0 char *multiByteBuffer; multiByteBuffer = (char*)rakMalloc_Ex(mbByteLength + 1, _FILE_AND_LINE_); - bool result = bs->ReadAlignedBytes((unsigned char*)multiByteBuffer, (const unsigned int)mbByteLength); + bool result = bs->ReadAlignedBytes((unsigned char*)multiByteBuffer, (unsigned int)mbByteLength); if (result == false) { rakFree_Ex(multiByteBuffer, _FILE_AND_LINE_); @@ -390,10 +390,7 @@ bool RakWString::Deserialize(wchar_t *str, BitStream *bs) } else { -#pragma warning(push) -#pragma warning(disable:4996) wcscpy(str, L""); -#pragma warning(pop) } return true; } @@ -406,7 +403,7 @@ bool RakWString::Deserialize(wchar_t *str, size_t strLength, BitStream *bs) #if 0 char *multiByteBuffer; multiByteBuffer = (char*) rakMalloc_Ex(mbByteLength+1, _FILE_AND_LINE_); - bool result = bs->ReadAlignedBytes((unsigned char*) multiByteBuffer,(const unsigned int) mbByteLength); + bool result = bs->ReadAlignedBytes((unsigned char*) multiByteBuffer,(unsigned int) mbByteLength); if (result==false) { rakFree_Ex(multiByteBuffer, _FILE_AND_LINE_); diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 100604d12..1c8002960 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -48,6 +48,7 @@ target_include_directories(TestSupport PUBLIC ${PROJECT_SOURCE_DIR}/Source/include ) set_target_properties(TestSupport PROPERTIES FOLDER "Tests") +mafianet_enable_warnings(TestSupport) # ----------------------------------------------------------------------------- # Unit suite @@ -62,6 +63,7 @@ target_link_libraries(UnitTests PRIVATE TestSupport GTest::gtest_main) # release-only backstops that would trip the assert in Debug. target_compile_definitions(UnitTests PRIVATE $<$:_DEBUG>) set_target_properties(UnitTests PROPERTIES FOLDER "Tests") +mafianet_enable_warnings(UnitTests) gtest_discover_tests(UnitTests PROPERTIES LABELS unit @@ -82,6 +84,7 @@ target_link_libraries(IntegrationTests PRIVATE TestSupport GTest::gtest_main) # and memory corruption the moment one constructs a RakPeer-derived object itself. target_compile_definitions(IntegrationTests PRIVATE $<$:_DEBUG>) set_target_properties(IntegrationTests PROPERTIES FOLDER "Tests") +mafianet_enable_warnings(IntegrationTests) gtest_discover_tests(IntegrationTests PROPERTIES LABELS integration RUN_SERIAL TRUE TIMEOUT 600 diff --git a/Tests/Integration/ComprehensiveConvertTests.cpp b/Tests/Integration/ComprehensiveConvertTests.cpp index bb844e98a..dd2fc40ca 100644 --- a/Tests/Integration/ComprehensiveConvertTests.cpp +++ b/Tests/Integration/ComprehensiveConvertTests.cpp @@ -197,7 +197,7 @@ TEST_F(ComprehensiveConvert, DoesALittleBitOfEverything) #endif peerIndex=randomMT()%NUM_PEERS; - sprintf(data+3, "dataLength=%i priority=%i reliability=%i orderingChannel=%i target=%i broadcast=%i\n", dataLength, priority, reliability, orderingChannel, target.GetPort(), broadcast); + snprintf(data+3, sizeof(data)-3, "dataLength=%i priority=%i reliability=%i orderingChannel=%i target=%i broadcast=%i\n", dataLength, (int)priority, (int)reliability, orderingChannel, target.GetPort(), broadcast); #ifdef _VERIFY_RECIPIENTS memcpy((char*)data+1, (char*)&target.GetPort(), sizeof(unsigned short)); #endif @@ -231,12 +231,12 @@ TEST_F(ComprehensiveConvert, DoesALittleBitOfEverything) broadcast=false; // Temporarily in so I can check recipients #endif - sprintf(data+3, "dataLength=%i priority=%i reliability=%i orderingChannel=%i target=%i broadcast=%i\n", dataLength, priority, reliability, orderingChannel, target.GetPort(), broadcast); + snprintf(data+3, sizeof(data)-3, "dataLength=%i priority=%i reliability=%i orderingChannel=%i target=%i broadcast=%i\n", dataLength, (int)priority, (int)reliability, orderingChannel, target.GetPort(), broadcast); #ifdef _VERIFY_RECIPIENTS memcpy((char*)data, (char*)&target.GetPort(), sizeof(unsigned short)); #endif data[dataLength-1]=0; - sprintf(RPCName, "RPC%i", (randomMT()%4)+1); + snprintf(RPCName, sizeof(RPCName), "RPC%u", (unsigned int)(randomMT()%4)+1); // autoRpc[i]->Call(RPCName); //peers[peerIndex]->RPC(RPCName, data, dataLength*8, priority, reliability, orderingChannel, target, broadcast, 0, UNASSIGNED_NETWORK_ID,0); } diff --git a/Tests/Integration/CrossConnectionConvertTests.cpp b/Tests/Integration/CrossConnectionConvertTests.cpp index 5f98b96c7..e77a39040 100644 --- a/Tests/Integration/CrossConnectionConvertTests.cpp +++ b/Tests/Integration/CrossConnectionConvertTests.cpp @@ -56,7 +56,7 @@ TEST_F(CrossConnectionConvert, SimultaneousConnectHandshakes) { // Reduce test duration in CI environments const bool isCI = (getenv("CI") != nullptr); - const int testDurationMs = isCI ? 2000 : 10000; // 2 seconds in CI + const TimeMS testDurationMs = isCI ? 2000 : 10000; // 2 seconds in CI static const unsigned short SERVER_PORT=1234; char serverIP[64]; diff --git a/Tests/Integration/PingTestsTests.cpp b/Tests/Integration/PingTestsTests.cpp index 21e8b9ad1..fbaf3eb91 100644 --- a/Tests/Integration/PingTestsTests.cpp +++ b/Tests/Integration/PingTestsTests.cpp @@ -269,7 +269,6 @@ TEST_F(PingTests, OccasionalPingIsObservableAsPingTraffic) SocketDescriptor receiverSd(0, "127.0.0.1"); ASSERT_EQ(receiver->Startup(2, &receiverSd, 1), RAKNET_STARTED); receiver->SetMaximumIncomingConnections(2); - const unsigned short receiverPort = receiver->GetInternalID(UNASSIGNED_SYSTEM_ADDRESS).GetPort(); RakPeerInterface *pinging, *quiet; TestHelpers::StandardClientPrep(pinging, destroyList); diff --git a/Tests/Integration/RelayPluginGroupMessageLiveTests.cpp b/Tests/Integration/RelayPluginGroupMessageLiveTests.cpp new file mode 100644 index 000000000..200d436a9 --- /dev/null +++ b/Tests/Integration/RelayPluginGroupMessageLiveTests.cpp @@ -0,0 +1,167 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * RelayPlugin::SendGroupMessage() takes a priority, reliability and ordering channel and + * sends them to the server, but the server read them and then relayed every group message + * as High / ReliableOrdered / channel 0. This checks, on the receiving client's reliability + * layer, that the relayed message carries what the sender asked for. + */ + +#include + +#include "mafianet/relay_plugin.h" +#include "mafianet/internal_packet.h" +#include "mafianet/message_identifiers.h" +#include "mafianet/bit_stream.h" +#include "mafianet/peer_handle.h" +#include "mafianet/get_time.h" +#include "mafianet/sleep.h" + +#include + +using namespace MafiaNet; + +namespace +{ + // Records the reliability and ordering channel of every inbound RPE_GROUP_MSG_FROM_SERVER. + class GroupMessageObserver : public PluginInterface2 + { + public: + struct Seen + { + Reliability reliability; + unsigned char orderingChannel; + unsigned char tag; + }; + std::vector seen; + + bool UsesReliabilityLayer(void) const override { return true; } + void OnInternalPacket(InternalPacket *internalPacket, unsigned /*frameNumber*/, SystemAddress /*remoteSystemAddress*/, TimeMS /*time*/, int isSend) override + { + // Unsplit payload: ID_RELAY_PLUGIN, RPE_GROUP_MSG_FROM_SERVER, sender name, data. + if (isSend || internalPacket->splitPacketCount > 0 || internalPacket->dataBitLength < 16) + return; + if (internalPacket->data[0] != ID_RELAY_PLUGIN || internalPacket->data[1] != RPE_GROUP_MSG_FROM_SERVER) + return; + const unsigned int bytes = BITS_TO_BYTES(internalPacket->dataBitLength); + seen.push_back({internalPacket->reliability, internalPacket->orderingChannel, internalPacket->data[bytes - 1]}); + } + }; + + // Pump every peer; return true once pred() holds, false at the deadline. + template + bool PumpUntil(std::vector peers, Pred pred, std::vector *clientRelayEvents = nullptr) + { + const TimeMS start = GetTimeMS(); + while (GetTimeMS() - start < 10000) + { + for (size_t i = 0; i < peers.size(); i++) + { + for (auto pkt : peers[i]->incoming()) + { + if (clientRelayEvents && i > 0 && pkt->length >= 2 && pkt->data[0] == ID_RELAY_PLUGIN) + clientRelayEvents->push_back(pkt->data[1]); + } + } + if (pred()) + return true; + RakSleep(30); + } + return pred(); + } +} + +class RelayPluginGroupMessageLive : public ::testing::Test +{ + // Peers are RAII MafiaNet::Peer locals declared after their plugins, so they shut down + // (and stop calling the plugins) before the plugins are destroyed. +}; + +TEST_F(RelayPluginGroupMessageLive, GroupMessageKeepsRequestedReliabilityAndChannel) +{ + RelayPlugin serverRelay, relayA, relayB; + GroupMessageObserver observerB; + + // Space the constructions: POSIX GUIDs are seeded from the microsecond clock. + Peer server; + RakSleep(2); + Peer clientA; + RakSleep(2); + Peer clientB; + + // Attach before Startup: the observer uses the reliability layer, whose plugin list the + // network thread reads without a lock. + server->AttachPlugin(&serverRelay); + serverRelay.SetAcceptAddParticipantRequests(true); + clientA->AttachPlugin(&relayA); + clientB->AttachPlugin(&relayB); + clientB->AttachPlugin(&observerB); + + SocketDescriptor serverSd(0, "127.0.0.1"); + ASSERT_EQ(server->Startup(2, &serverSd, 1), RAKNET_STARTED); + server->SetMaximumIncomingConnections(2); + const unsigned short serverPort = server->GetInternalID().GetPort(); + + SocketDescriptor clientSd(0, "127.0.0.1"); + ASSERT_EQ(clientA->Startup(1, &clientSd, 1), RAKNET_STARTED); + ASSERT_EQ(clientB->Startup(1, &clientSd, 1), RAKNET_STARTED); + + ASSERT_EQ(clientA->Connect("127.0.0.1", serverPort, nullptr, 0), CONNECTION_ATTEMPT_STARTED); + ASSERT_EQ(clientB->Connect("127.0.0.1", serverPort, nullptr, 0), CONNECTION_ATTEMPT_STARTED); + + std::vector peers{&server, &clientA, &clientB}; + ASSERT_TRUE(PumpUntil(peers, [&] { + return server->NumberOfConnections() == 2 && clientA->NumberOfConnections() == 1 && clientB->NumberOfConnections() == 1; + })) << "peers did not connect"; + + const RakNetGUID serverGuid = server->GetMyGUID(); + std::vector relayEvents; + auto count = [&](unsigned char e) { + size_t n = 0; + for (unsigned char x : relayEvents) + n += (x == e); + return n; + }; + + relayA.AddParticipantRequestFromClient("a", serverGuid); + relayB.AddParticipantRequestFromClient("b", serverGuid); + ASSERT_TRUE(PumpUntil(peers, [&] { return count(RPE_ADD_CLIENT_SUCCESS) == 2; }, &relayEvents)) + << "participants were not added"; + + relayA.JoinGroupRequest(serverGuid, "room"); + relayB.JoinGroupRequest(serverGuid, "room"); + ASSERT_TRUE(PumpUntil(peers, [&] { return count(RPE_JOIN_GROUP_SUCCESS) == 2; }, &relayEvents)) + << "clients did not join the group"; + + // Two group messages from A, each with settings the old relay overwrote. + BitStream ordered; + ordered.Write((unsigned char) 1); + relayA.SendGroupMessage(serverGuid, &ordered, Priority::Low, Reliability::ReliableOrdered, 7); + BitStream reliable; + reliable.Write((unsigned char) 2); + relayA.SendGroupMessage(serverGuid, &reliable, Priority::Medium, Reliability::Reliable, 0); + + ASSERT_TRUE(PumpUntil(peers, [&] { return observerB.seen.size() >= 2; }, &relayEvents)) + << "client B did not receive both group messages"; + + bool sawOrdered = false, sawReliable = false; + for (const auto &s : observerB.seen) + { + if (s.tag == 1) + { + sawOrdered = true; + EXPECT_EQ(s.reliability, Reliability::ReliableOrdered); + EXPECT_EQ(s.orderingChannel, 7); + } + else if (s.tag == 2) + { + sawReliable = true; + EXPECT_EQ(s.reliability, Reliability::Reliable); + } + } + EXPECT_TRUE(sawOrdered); + EXPECT_TRUE(sawReliable); +} diff --git a/Tests/Integration/SessionConfigLiveTests.cpp b/Tests/Integration/SessionConfigLiveTests.cpp index 4de0c7d64..5c987359d 100644 --- a/Tests/Integration/SessionConfigLiveTests.cpp +++ b/Tests/Integration/SessionConfigLiveTests.cpp @@ -160,22 +160,9 @@ namespace return failure; } - // True if the id shows up within the window. Used for the negative assertions, where the point - // is that a packet must NOT arrive. - bool SawWithin(RakPeerInterface *wanted, int wantedId, RakPeerInterface *alsoPump, int windowMs) - { - Packet *p = PumpUntil(wanted, wantedId, alsoPump, windowMs); - if (p) - { - wanted->DeallocatePacket(p); - return true; - } - return false; - } - // Drain everything a peer receives over a fixed window and keep the ids. // - // Negative assertions must not be built out of successive SawWithin() probes: that helper discards + // Negative assertions must not be built out of successive PumpUntil() probes: that helper discards // every packet that is not the one it is waiting for, so the first probe swallows the very packet a // later probe looks for and the test then passes for the wrong reason. Collect once, assert after. std::vector CollectIds(RakPeerInterface *wanted, RakPeerInterface *alsoPump, int windowMs) @@ -200,7 +187,7 @@ namespace // Drain BOTH peers over a window, appending every id each one yields to its own ledger. // // The single-peer CollectIds() above discards the other peer's packets, and two successive - // SawWithin()/CollectIds() probes swallow each other's evidence for the same reason. A negative + // PumpUntil()/CollectIds() probes swallow each other's evidence for the same reason. A negative // assertion about a PAIR of events -- neither peer may report a connection -- therefore has to // drain both in one loop. Appends rather than assigns so a ledger already carrying what an earlier // PumpUntil() drained keeps it. diff --git a/Tests/Support/CommonFunctions.cpp b/Tests/Support/CommonFunctions.cpp index 0c02a6125..a7dadc246 100644 --- a/Tests/Support/CommonFunctions.cpp +++ b/Tests/Support/CommonFunctions.cpp @@ -18,7 +18,7 @@ CommonFunctions::~CommonFunctions(void) { } -bool CommonFunctions::ConnectionStateMatchesOptions(RakPeerInterface *peer,SystemAddress currentSystem,bool isConnected,bool isConnecting,bool isPending,bool isDisconnecting,bool isNotConnected,bool isLoopBack , bool isSilentlyDisconnecting) +bool CommonFunctions::ConnectionStateMatchesOptions(RakPeerInterface *peer,SystemAddress currentSystem,bool isConnected,bool isConnecting,bool isPending,bool isDisconnecting,bool isNotConnected,bool /*isLoopBack*/ , bool isSilentlyDisconnecting) { ConnectionState connectionState=peer->GetConnectionState(currentSystem); switch(connectionState) @@ -53,7 +53,7 @@ bool CommonFunctions::ConnectionStateMatchesOptions(RakPeerInterface *peer,Syste } } -bool CommonFunctions::WaitAndConnect(RakPeerInterface *peer,char* ip,unsigned short int port,int millisecondsToWait) +bool CommonFunctions::WaitAndConnect(RakPeerInterface *peer,const char* ip,unsigned short int port,int millisecondsToWait) { SystemAddress connectToAddress; @@ -62,7 +62,7 @@ bool CommonFunctions::WaitAndConnect(RakPeerInterface *peer,char* ip,unsigned sh connectToAddress.SetPortHostOrder(port); TimeMS entryTime=GetTimeMS(); - while(!CommonFunctions::ConnectionStateMatchesOptions (peer,connectToAddress,true)&&GetTimeMS()-entryTimetimerLength); + return (MafiaNet::GetTimeMS()-startTime>(MafiaNet::TimeMS)timerLength); } diff --git a/Tests/Unit/FileListDirectoryTests.cpp b/Tests/Unit/FileListDirectoryTests.cpp index b08477662..47769b99f 100644 --- a/Tests/Unit/FileListDirectoryTests.cpp +++ b/Tests/Unit/FileListDirectoryTests.cpp @@ -85,6 +85,73 @@ TEST(FileListDirectory, NonRecursiveWalkStaysInTheTopDirectory) EXPECT_EQ(list.fileList[0].fileLengthBytes, 10u); } +TEST(FileListDirectory, WriteDataReadsContentsIncludingAnEmptyFile) +{ + TempTree tree; + tree.Write("empty.bin", 0); + const std::string dir = tree.root.string() + "/"; + + FileList list; + list.AddFilesFromDirectory(dir.c_str(), nullptr, false, true, false, FileListNodeContext(0, 0, 0, 0)); + + // The read checks must not mistake a zero-length file for a failed read. + ASSERT_EQ(Names(list), (std::set{"a.bin", "empty.bin"})); + for (unsigned i = 0; i < list.fileList.Size(); ++i) + { + const FileListNode &node = list.fileList[i]; + if (node.filename == "a.bin") + { + ASSERT_EQ(node.dataLengthBytes, 10u); + EXPECT_EQ(std::string(node.data, node.dataLengthBytes), std::string(10, 'x')); + } + else + { + EXPECT_EQ(node.dataLengthBytes, 0u); + } + } +} + +TEST(FileListDirectory, PopulateDataFromDiskReadsEveryFile) +{ + TempTree tree; + const std::string dir = tree.root.string() + "/"; + + FileList list; + list.AddFilesFromDirectory(dir.c_str(), nullptr, false, false, true, FileListNodeContext(0, 0, 0, 0)); + ASSERT_EQ(list.fileList.Size(), 3u); + + list.PopulateDataFromDisk(dir.c_str(), true, false, false); + + for (unsigned i = 0; i < list.fileList.Size(); ++i) + { + const FileListNode &node = list.fileList[i]; + ASSERT_NE(node.data, nullptr) << node.filename.C_String(); + EXPECT_EQ(node.dataLengthBytes, node.fileLengthBytes); + EXPECT_EQ(std::string(node.data, node.dataLengthBytes), std::string(node.fileLengthBytes, 'x')); + } +} + +TEST(FileListDirectory, PopulateDataFromDiskWithoutDataOrHashTerminates) +{ + // With neither data nor hash requested, the loop used to skip i++ (and fclose) for every + // file that exists, re-processing the first entry forever. + TempTree tree; + const std::string dir = tree.root.string() + "/"; + + FileList list; + list.AddFilesFromDirectory(dir.c_str(), nullptr, false, true, true, FileListNodeContext(0, 0, 0, 0)); + ASSERT_EQ(list.fileList.Size(), 3u); + + list.PopulateDataFromDisk(dir.c_str(), false, false, false); + + ASSERT_EQ(list.fileList.Size(), 3u); + for (unsigned i = 0; i < list.fileList.Size(); ++i) + { + EXPECT_EQ(list.fileList[i].data, nullptr); + EXPECT_EQ(list.fileList[i].dataLengthBytes, 0u); + } +} + #ifndef _WIN32 // The shim is only compiled on Linux and macOS; MSVC provides the real function. diff --git a/Tests/Unit/GuidUtilTests.cpp b/Tests/Unit/GuidUtilTests.cpp index b0669cc1d..5706dfe42 100644 --- a/Tests/Unit/GuidUtilTests.cpp +++ b/Tests/Unit/GuidUtilTests.cpp @@ -46,7 +46,7 @@ TEST(GuidUtil, UnassignedSentinelStringifiesToDocumentedLabel) TEST(GuidUtil, ToStringIsThreadSafe) { const unsigned threadCount = 16; - const unsigned iterations = 20000; + constexpr unsigned iterations = 20000; std::atomic corrupted(false); std::vector threads; @@ -57,7 +57,7 @@ TEST(GuidUtil, ToStringIsThreadSafe) const uint64_t value = 0x1000000000000000ull + t; const RakNetGUID g(value); const std::string expected = to_string(g); - threads.emplace_back([g, expected, iterations, &corrupted]() { + threads.emplace_back([g, expected, &corrupted]() { for (unsigned k = 0; k < iterations && !corrupted.load(std::memory_order_relaxed); ++k) { if (to_string(g) != expected) @@ -113,6 +113,8 @@ TEST_F(GuidUtilConnectedAddressTest, MirrorsLegacyLookup) const bool legacyHas = (legacy != UNASSIGNED_SYSTEM_ADDRESS); ASSERT_EQ(modern.has_value(), legacyHas) << "guid value 0x" << std::hex << sample; if (legacyHas) + { EXPECT_EQ(modern.value(), legacy) << "guid value 0x" << std::hex << sample; + } } } diff --git a/Tests/Unit/SafeStringShimTests.cpp b/Tests/Unit/SafeStringShimTests.cpp new file mode 100644 index 000000000..dfdb3cc77 --- /dev/null +++ b/Tests/Unit/SafeStringShimTests.cpp @@ -0,0 +1,83 @@ +/* + * Copyright (c) 2026, MafiaHub + * Licensed under MIT-style license + * + * The POSIX vsnprintf_s shim (linux_adapter.cpp / osx_adapter.cpp) used to write the + * terminator at buffer[sizeOfBuffer] whenever _TRUNCATE truncated the output: one byte past + * the end. FormatString / FormatStringTS reach it with _TRUNCATE, so any formatted string + * longer than their buffers corrupted the byte after them. A guard byte after each + * destination pins the bound. Hermetic: no networking. + */ + +#include + +#include "mafianet/format_string.h" +#include "mafianet/linux_adapter.h" +#include "mafianet/osx_adapter.h" + +#include +#include +#include + +#ifndef _WIN32 +// The shims are only compiled on Linux and macOS; MSVC provides the real functions. + +namespace +{ +int CallVsnprintfS(char *buffer, size_t sizeOfBuffer, size_t count, const char *format, ...) +{ + va_list ap; + va_start(ap, format); + const int result = vsnprintf_s(buffer, sizeOfBuffer, count, format, ap); + va_end(ap); + return result; +} +} // namespace + +TEST(VsnprintfSShim, TruncateKeepsTheTerminatorInsideTheBuffer) +{ + char block[4 + 1]; + block[4] = 'G'; + CallVsnprintfS(block, 4, _TRUNCATE, "%s", "abcdefgh"); + EXPECT_STREQ(block, "abc"); + EXPECT_EQ(block[4], 'G'); +} + +TEST(VsnprintfSShim, OutputThatFitsIsWrittenWhole) +{ + char block[8 + 1]; + block[8] = 'G'; + EXPECT_EQ(CallVsnprintfS(block, 8, _TRUNCATE, "%d-%s", 42, "ab"), 5); + EXPECT_STREQ(block, "42-ab"); + EXPECT_EQ(block[8], 'G'); +} + +TEST(VsnprintfSShim, ExplicitCountTooSmallIsRejectedWithoutOverflow) +{ + char block[8 + 1]; + block[8] = 'G'; + EXPECT_EQ(CallVsnprintfS(block, 8, 4, "%s", "abcdefgh"), -1); + EXPECT_STREQ(block, ""); + EXPECT_EQ(block[8], 'G'); +} + +TEST(VsnprintfSShim, FormatStringTSTruncatesInsideItsBuffer) +{ + // FormatStringTS formats into a caller buffer it assumes is 512 bytes. + char block[512 + 1]; + block[512] = 'G'; + const std::string longText(600, 'x'); + FormatStringTS(block, "%s", longText.c_str()); + EXPECT_EQ(std::strlen(block), 511u); + EXPECT_EQ(block[512], 'G'); +} + +TEST(SprintfSShim, OutputThatDoesNotFitLeavesAnEmptyString) +{ + char block[4 + 1]; + block[4] = 'G'; + sprintf_s(block, 4, "%s", "abcdefgh"); + EXPECT_STREQ(block, ""); + EXPECT_EQ(block[4], 'G'); +} +#endif diff --git a/Tests/Unit/TableCellTests.cpp b/Tests/Unit/TableCellTests.cpp new file mode 100644 index 000000000..517742da5 --- /dev/null +++ b/Tests/Unit/TableCellTests.cpp @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + */ + +#include + +#include +#include + +#include "mafianet/ds_table.h" + +using namespace DataStructures; + +// The copy constructor used to test and free its own uninitialized `c`, and +// left `c` uninitialized when copying a cell without string/binary data, so +// the copy's destructor freed garbage. Constructing into storage filled with +// a non-null pattern makes the old behaviour deterministic instead of relying +// on whatever the stack held. +namespace +{ + Table::Cell *CopyIntoDirtyStorage(void *storage, const Table::Cell &source) + { + std::memset(storage, 0xAB, sizeof(Table::Cell)); + return new (storage) Table::Cell(source); + } +} + +TEST(TableCell, CopyOfANumericCellOwnsNoBuffer) +{ + Table::Cell source; + source.Set(42); + + alignas(Table::Cell) unsigned char storage[sizeof(Table::Cell)]; + Table::Cell *copy = CopyIntoDirtyStorage(storage, source); + EXPECT_EQ(nullptr, copy->c); + EXPECT_FALSE(copy->isEmpty); + EXPECT_EQ(42.0, copy->i); + copy->~Cell(); +} + +TEST(TableCell, CopyOfAStringCellOwnsItsOwnBuffer) +{ + Table::Cell source; + source.Set("hello"); + + alignas(Table::Cell) unsigned char storage[sizeof(Table::Cell)]; + Table::Cell *copy = CopyIntoDirtyStorage(storage, source); + ASSERT_NE(nullptr, copy->c); + EXPECT_NE(source.c, copy->c); + EXPECT_STREQ("hello", copy->c); + copy->~Cell(); + EXPECT_STREQ("hello", source.c); +} + +TEST(TableCell, CopyOfAnEmptyCellOwnsNoBuffer) +{ + Table::Cell source; + + alignas(Table::Cell) unsigned char storage[sizeof(Table::Cell)]; + Table::Cell *copy = CopyIntoDirtyStorage(storage, source); + EXPECT_EQ(nullptr, copy->c); + EXPECT_TRUE(copy->isEmpty); + copy->~Cell(); +} diff --git a/cmake/MafiaNetHelpers.cmake b/cmake/MafiaNetHelpers.cmake index 92343c282..13b4dd3df 100644 --- a/cmake/MafiaNetHelpers.cmake +++ b/cmake/MafiaNetHelpers.cmake @@ -103,3 +103,20 @@ function(mafianet_add_extension) FOLDER "Extensions" ) endfunction() + +# mafianet_enable_warnings(...) +# +# Compiler warnings for MafiaNet's own code. Applied per target, never +# globally, so fetched dependencies keep their own flags. +# MAFIANET_WARNINGS_AS_ERRORS turns them into errors (CI). +function(mafianet_enable_warnings) + foreach(target IN LISTS ARGN) + if(MSVC) + target_compile_options(${target} PRIVATE /W4 + $<$:/WX>) + else() + target_compile_options(${target} PRIVATE -Wall -Wextra + $<$:-Werror>) + endif() + endforeach() +endfunction() From a703a9bd259a54d2790d1c805e8426a95a7d6746 Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:22:54 +0200 Subject: [PATCH 04/12] test: add libFuzzer harnesses, microbenchmarks and a coverage preset Fuzzing (Tests/Fuzz): harnesses for BitStream deserialization and for the datagram parser of a connected ReliabilityLayer, the code that parses what remote peers send. Each _fuzz.cpp is built two ways: - _replay, always with the tests, on every compiler: replays the checked-in corpus (seeds plus minimized crash reproducers) as the ctest case FuzzCorpus., label unit. A fixed crash stays fixed everywhere. - _fuzz, a libFuzzer binary, with MAFIANET_BUILD_FUZZERS=ON (Clang). The fuzz preset builds them under ASan + UBSan with the library instrumented for coverage. Benchmarks (Benchmarks/, google/benchmark v1.9.5, MAFIANET_BUILD_BENCHMARKS, bench preset): BitStream serialization and the ReliabilityLayer send / ack / split path between two layers joined in memory. Coverage: MAFIANET_COVERAGE (--coverage) and the coverage preset; report with gcovr --root . --filter Source/ build/coverage. --- Benchmarks/CMakeLists.txt | 30 +++++ Benchmarks/bit_stream_bench.cpp | 110 ++++++++++++++++ Benchmarks/reliability_layer_bench.cpp | 123 ++++++++++++++++++ CMakeLists.txt | 28 ++++ CMakePresets.json | 61 ++++++++- Tests/CMakeLists.txt | 5 + Tests/Fuzz/CMakeLists.txt | 44 +++++++ Tests/Fuzz/bit_stream_fuzz.cpp | 96 ++++++++++++++ Tests/Fuzz/corpus/bit_stream/numbers | Bin 0 -> 26 bytes .../corpus/bit_stream/regress-safe-alloc-leak | Bin 0 -> 70 bytes Tests/Fuzz/corpus/bit_stream/strings | Bin 0 -> 44 bytes .../Fuzz/corpus/bit_stream/vectors_and_blobs | Bin 0 -> 36 bytes Tests/Fuzz/corpus/reliability_layer/acks | Bin 0 -> 24 bytes .../reliability_layer/ordered_split_sequenced | Bin 0 -> 3112 bytes .../regress-split-fragment-use-after-free | Bin 0 -> 2421 bytes Tests/Fuzz/reliability_layer_fuzz.cpp | 82 ++++++++++++ Tests/Fuzz/replay_main.cpp | 65 +++++++++ 17 files changed, 643 insertions(+), 1 deletion(-) create mode 100644 Benchmarks/CMakeLists.txt create mode 100644 Benchmarks/bit_stream_bench.cpp create mode 100644 Benchmarks/reliability_layer_bench.cpp create mode 100644 Tests/Fuzz/CMakeLists.txt create mode 100644 Tests/Fuzz/bit_stream_fuzz.cpp create mode 100644 Tests/Fuzz/corpus/bit_stream/numbers create mode 100644 Tests/Fuzz/corpus/bit_stream/regress-safe-alloc-leak create mode 100644 Tests/Fuzz/corpus/bit_stream/strings create mode 100644 Tests/Fuzz/corpus/bit_stream/vectors_and_blobs create mode 100644 Tests/Fuzz/corpus/reliability_layer/acks create mode 100644 Tests/Fuzz/corpus/reliability_layer/ordered_split_sequenced create mode 100644 Tests/Fuzz/corpus/reliability_layer/regress-split-fragment-use-after-free create mode 100644 Tests/Fuzz/reliability_layer_fuzz.cpp create mode 100644 Tests/Fuzz/replay_main.cpp diff --git a/Benchmarks/CMakeLists.txt b/Benchmarks/CMakeLists.txt new file mode 100644 index 000000000..2d4f18e54 --- /dev/null +++ b/Benchmarks/CMakeLists.txt @@ -0,0 +1,30 @@ +# Microbenchmarks (google/benchmark). Not tests: nothing here asserts, and the +# numbers only mean something compared against a baseline on the same machine. +# +# cmake --preset bench && cmake --build --preset bench +# ./build/bench/Benchmarks/MafiaNetBenchmarks --benchmark_filter=BitStream +# +# Compare two builds with google/benchmark's tools/compare.py, using +# --benchmark_out= --benchmark_out_format=json from each. + +include(FetchContent) + +FetchContent_Declare( + googlebenchmark + GIT_REPOSITORY https://github.com/google/benchmark.git + GIT_TAG v1.9.5 + GIT_SHALLOW TRUE +) +set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_GTEST_TESTS OFF CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_WERROR OFF CACHE BOOL "" FORCE) +FetchContent_MakeAvailable(googlebenchmark) + +file(GLOB MAFIANET_BENCHMARK_SOURCES CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/*_bench.cpp") +add_executable(MafiaNetBenchmarks ${MAFIANET_BENCHMARK_SOURCES}) +target_link_libraries(MafiaNetBenchmarks PRIVATE MafiaNet::MafiaNetStatic benchmark::benchmark_main) +# Must match the library: some classes carry _DEBUG-only members. +target_compile_definitions(MafiaNetBenchmarks PRIVATE $<$:_DEBUG>) +set_target_properties(MafiaNetBenchmarks PROPERTIES FOLDER "Benchmarks") +mafianet_enable_warnings(MafiaNetBenchmarks) diff --git a/Benchmarks/bit_stream_bench.cpp b/Benchmarks/bit_stream_bench.cpp new file mode 100644 index 000000000..25f2af0d9 --- /dev/null +++ b/Benchmarks/bit_stream_bench.cpp @@ -0,0 +1,110 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + */ + +#include + +#include + +#include "mafianet/bit_stream.h" +#include "mafianet/string.h" +#include "mafianet/string_compressor.h" + +using namespace MafiaNet; + +namespace +{ + // A typical entity-state message: id, position, compressed orientation, + // flags and a few range-packed values. + void WriteEntityState(BitStream &bs, uint32_t i) + { + bs.Write((uint8_t) 135); + bs.Write(i); + bs.Write(1.5f * i); + bs.Write(2.5f * i); + bs.Write(-3.5f * i); + bs.WriteNormQuat(1.f, 0.f, 0.f, 0.f); + bs.Write((i & 1) != 0); + bs.Write((i & 2) != 0); + bs.WriteCompressed((uint16_t) (i & 0xfff)); + bs.WriteBitsFromIntegerRange((uint32_t) (i % 100), 0u, 100u); + } + + void ReadEntityState(BitStream &bs, benchmark::State &state) + { + uint8_t id; + uint32_t index; + float x, y, z, qw, qx, qy, qz; + bool a, b; + uint16_t c; + uint32_t r; + if (!(bs.Read(id) && bs.Read(index) && bs.Read(x) && bs.Read(y) && bs.Read(z) && + bs.ReadNormQuat(qw, qx, qy, qz) && bs.Read(a) && bs.Read(b) && bs.ReadCompressed(c) && + bs.ReadBitsFromIntegerRange(r, 0u, 100u))) + state.SkipWithError("read failed"); + benchmark::DoNotOptimize(x + y + z + qw + c + r); + } +} + +static void BitStream_WriteEntityState(benchmark::State &state) +{ + BitStream bs(1024); + uint32_t i = 0; + for (auto _ : state) + { + bs.Reset(); + WriteEntityState(bs, i++); + benchmark::DoNotOptimize(bs.GetData()); + } + state.SetBytesProcessed(state.iterations() * bs.GetNumberOfBytesUsed()); +} +BENCHMARK(BitStream_WriteEntityState); + +static void BitStream_ReadEntityState(benchmark::State &state) +{ + BitStream written; + WriteEntityState(written, 42); + for (auto _ : state) + { + BitStream bs(written.GetData(), written.GetNumberOfBytesUsed(), false); + ReadEntityState(bs, state); + } + state.SetBytesProcessed(state.iterations() * written.GetNumberOfBytesUsed()); +} +BENCHMARK(BitStream_ReadEntityState); + +static void BitStream_RakStringCompressedRoundTrip(benchmark::State &state) +{ + StringCompressor::AddReference(); + const RakString text("player joined the server and said hello to everyone in the lobby"); + BitStream bs(256); + for (auto _ : state) + { + bs.Reset(); + text.SerializeCompressed(&bs); + RakString out; + out.DeserializeCompressed(&bs); + benchmark::DoNotOptimize(out.C_String()); + } + StringCompressor::RemoveReference(); +} +BENCHMARK(BitStream_RakStringCompressedRoundTrip); + +static void BitStream_WriteAlignedBytes(benchmark::State &state) +{ + const size_t size = (size_t) state.range(0); + std::vector payload(size, 0xAB); + BitStream bs((unsigned int) size + 16); + for (auto _ : state) + { + bs.Reset(); + bs.Write(true); // misalign, as real messages usually are + bs.WriteAlignedBytes(payload.data(), (unsigned int) size); + benchmark::DoNotOptimize(bs.GetData()); + } + state.SetBytesProcessed(state.iterations() * (int64_t) size); +} +BENCHMARK(BitStream_WriteAlignedBytes)->Arg(64)->Arg(1400)->Arg(64 * 1024); diff --git a/Benchmarks/reliability_layer_bench.cpp b/Benchmarks/reliability_layer_bench.cpp new file mode 100644 index 000000000..ffb8104bc --- /dev/null +++ b/Benchmarks/reliability_layer_bench.cpp @@ -0,0 +1,123 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * The send -> datagram -> receive -> ack path of two ReliabilityLayer + * instances joined in memory (the same wiring as the hermetic unit tests), + * so the numbers cover packing, splitting, reassembly and ack processing + * without the OS socket. + */ + +#include + +#include + +#include "mafianet/ds_list.h" +#include "mafianet/get_time.h" +#include "mafianet/mtu_size.h" +#include "mafianet/packet_priority.h" +#include "mafianet/plugin_interface2.h" +#include "mafianet/rand.h" +#include "mafianet/reliability_layer.h" +#include "mafianet/socket2.h" + +using namespace MafiaNet; + +namespace +{ + struct QueueSocket : public RakNetSocket2 + { + std::vector > sent; + + RNS2SendResult Send(RNS2_SendParameters *sendParameters, const char *, unsigned int) override + { + sent.emplace_back(sendParameters->data, sendParameters->data + sendParameters->length); + return sendParameters->length; + } + }; + + struct LayerPair + { + ReliabilityLayer a, b; + QueueSocket aSock, bSock; + SystemAddress aAddr{"127.0.0.1", 40001}, bAddr{"127.0.0.1", 40002}; + DataStructures::List handlers; + RakNetRandom rnr; + BitStream updateBitStream{MAXIMUM_MTU_SIZE}; + CCTimeType now = GetTimeUS(); + + LayerPair() + { + a.Reset(true, MAXIMUM_MTU_SIZE, false); + b.Reset(true, MAXIMUM_MTU_SIZE, false); + } + + // One 10ms tick in both directions; returns messages b completed. + int Tick() + { + now += 10000; + a.Update(&aSock, bAddr, MAXIMUM_MTU_SIZE, now, 0, handlers, &rnr, updateBitStream); + for (auto &d : aSock.sent) + b.HandleSocketReceiveFromConnectedPlayer(d.data(), (unsigned int) d.size(), aAddr, handlers, + MAXIMUM_MTU_SIZE, &bSock, &rnr, now, updateBitStream); + aSock.sent.clear(); + + b.Update(&bSock, aAddr, MAXIMUM_MTU_SIZE, now, 0, handlers, &rnr, updateBitStream); + for (auto &d : bSock.sent) + a.HandleSocketReceiveFromConnectedPlayer(d.data(), (unsigned int) d.size(), bAddr, handlers, + MAXIMUM_MTU_SIZE, &aSock, &rnr, now, updateBitStream); + bSock.sent.clear(); + + int received = 0; + unsigned char *data; + while (b.Receive(&data) > 0) + { + rakFree_Ex(data, _FILE_AND_LINE_); + ++received; + } + return received; + } + }; +} + +// Many small reliable-ordered messages per tick: packing and ack throughput. +static void ReliabilityLayer_SmallReliableOrdered(benchmark::State &state) +{ + LayerPair pair; + std::vector message(32, 7); + message[0] = (char) 200; + const int perTick = (int) state.range(0); + int64_t delivered = 0; + for (auto _ : state) + { + for (int i = 0; i < perTick; ++i) + pair.a.Send(message.data(), BYTES_TO_BITS((BitSize_t) message.size()), Priority::High, + Reliability::ReliableOrdered, 0, true, MAXIMUM_MTU_SIZE, pair.now, 0); + delivered += pair.Tick(); + } + state.SetItemsProcessed(delivered); + state.SetBytesProcessed(delivered * (int64_t) message.size()); +} +BENCHMARK(ReliabilityLayer_SmallReliableOrdered)->Arg(16)->Arg(256); + +// One large message per iteration: split and reassembly. +static void ReliabilityLayer_SplitMessage(benchmark::State &state) +{ + LayerPair pair; + std::vector message((size_t) state.range(0), 9); + message[0] = (char) 200; + for (auto _ : state) + { + pair.a.Send(message.data(), BYTES_TO_BITS((BitSize_t) message.size()), Priority::Medium, + Reliability::ReliableOrdered, 0, true, MAXIMUM_MTU_SIZE, pair.now, 0); + int got = 0; + for (int tick = 0; got == 0 && tick < 10000; ++tick) + got = pair.Tick(); + if (got == 0) + state.SkipWithError("message not delivered"); + } + state.SetBytesProcessed(state.iterations() * state.range(0)); +} +BENCHMARK(ReliabilityLayer_SplitMessage)->Arg(16 * 1024)->Arg(256 * 1024); diff --git a/CMakeLists.txt b/CMakeLists.txt index 80c8234bc..1e6fbc839 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,6 +31,12 @@ option(MAFIANET_BUILD_SHARED "Build shared library" ON) option(MAFIANET_BUILD_STATIC "Build static library" ON) option(MAFIANET_BUILD_SAMPLES "Build sample applications" OFF) option(MAFIANET_BUILD_TESTS "Build test suite" OFF) +# libFuzzer harnesses (Clang only). Their corpus is replayed by the test suite +# on every compiler regardless; see Tests/Fuzz/. +option(MAFIANET_BUILD_FUZZERS "Build libFuzzer harnesses (Clang)" OFF) +option(MAFIANET_BUILD_BENCHMARKS "Build the google/benchmark microbenchmarks" OFF) +# Line coverage (GCC/Clang --coverage), reported with gcovr; see the coverage preset. +option(MAFIANET_COVERAGE "Instrument the build for line coverage" OFF) # Non-networking utilities (EmailSender, HTTPConnection/HTTPConnection2, # TelnetTransport, RakNetTransport2, ConsoleServer, LogCommandParser, # RakNetCommandParser, PacketConsoleLogger) live in the separate MafiaNetExtras @@ -93,6 +99,14 @@ if(MAFIANET_SANITIZER) add_link_options(-fsanitize=${_mafianet_san}) endif() +if(MAFIANET_COVERAGE) + if(MSVC) + message(FATAL_ERROR "MAFIANET_COVERAGE is not supported with MSVC") + endif() + add_compile_options(--coverage -O0) + add_link_options(--coverage) +endif() + # Dependencies find_package(OpenSSL 3.0.0 REQUIRED) find_package(Threads REQUIRED) @@ -145,3 +159,17 @@ if(MAFIANET_BUILD_TESTS) endif() add_subdirectory(Tests) endif() + +if(MAFIANET_BUILD_BENCHMARKS) + if(NOT MAFIANET_BUILD_STATIC) + message(FATAL_ERROR "MAFIANET_BUILD_BENCHMARKS requires MAFIANET_BUILD_STATIC") + endif() + add_subdirectory(Benchmarks) +endif() + +if(MAFIANET_BUILD_FUZZERS AND NOT MAFIANET_BUILD_TESTS) + if(NOT MAFIANET_BUILD_STATIC) + message(FATAL_ERROR "MAFIANET_BUILD_FUZZERS requires MAFIANET_BUILD_STATIC") + endif() + add_subdirectory(Tests/Fuzz) +endif() diff --git a/CMakePresets.json b/CMakePresets.json index f088a4990..530972b03 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -81,6 +81,49 @@ "cacheVariables": { "MAFIANET_SANITIZER": "thread" } + }, + { + "name": "coverage", + "inherits": "base", + "displayName": "Line coverage (GCC/Clang)", + "description": "Run the tests, then: gcovr --root . --filter Source/ build/coverage", + "condition": { + "type": "notEquals", + "lhs": "${hostSystemName}", + "rhs": "Windows" + }, + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "MAFIANET_BUILD_TESTS": "ON", + "MAFIANET_COVERAGE": "ON" + } + }, + { + "name": "bench", + "inherits": "base", + "displayName": "Benchmarks (Release)", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "MAFIANET_BUILD_BENCHMARKS": "ON" + } + }, + { + "name": "fuzz", + "inherits": "base", + "displayName": "libFuzzer harnesses (Clang, ASan + UBSan)", + "description": "fuzzer-no-link instruments the library for coverage; the harnesses link libFuzzer itself.", + "condition": { + "type": "notEquals", + "lhs": "${hostSystemName}", + "rhs": "Windows" + }, + "cacheVariables": { + "CMAKE_BUILD_TYPE": "RelWithDebInfo", + "CMAKE_C_COMPILER": "clang", + "CMAKE_CXX_COMPILER": "clang++", + "MAFIANET_BUILD_FUZZERS": "ON", + "MAFIANET_SANITIZER": "address+undefined+fuzzer-no-link" + } } ], "buildPresets": [ @@ -89,7 +132,10 @@ { "name": "ci", "configurePreset": "ci" }, { "name": "minimal", "configurePreset": "minimal" }, { "name": "asan", "configurePreset": "asan" }, - { "name": "tsan", "configurePreset": "tsan" } + { "name": "tsan", "configurePreset": "tsan" }, + { "name": "fuzz", "configurePreset": "fuzz" }, + { "name": "coverage", "configurePreset": "coverage" }, + { "name": "bench", "configurePreset": "bench", "targets": ["MafiaNetBenchmarks"] } ], "testPresets": [ { @@ -123,6 +169,11 @@ "configurePreset": "ci", "configuration": "Release" }, + { + "name": "coverage", + "inherits": "test-base", + "configurePreset": "coverage" + }, { "name": "asan", "inherits": "test-base", @@ -160,6 +211,14 @@ { "type": "test", "name": "asan" } ] }, + { + "name": "fuzz", + "displayName": "Configure and build the libFuzzer harnesses", + "steps": [ + { "type": "configure", "name": "fuzz" }, + { "type": "build", "name": "fuzz" } + ] + }, { "name": "tsan", "displayName": "Configure, build and test under TSan", diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 1c8002960..47561cffa 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -90,3 +90,8 @@ gtest_discover_tests(IntegrationTests PROPERTIES LABELS integration RUN_SERIAL TRUE TIMEOUT 600 DISCOVERY_TIMEOUT 60 ) + +# ----------------------------------------------------------------------------- +# Fuzz harnesses: corpus replay (always) and libFuzzer targets (opt-in) +# ----------------------------------------------------------------------------- +add_subdirectory(Fuzz) diff --git a/Tests/Fuzz/CMakeLists.txt b/Tests/Fuzz/CMakeLists.txt new file mode 100644 index 000000000..0eeded167 --- /dev/null +++ b/Tests/Fuzz/CMakeLists.txt @@ -0,0 +1,44 @@ +# Fuzz harnesses. Each _fuzz.cpp defines LLVMFuzzerTestOneInput and is +# built two ways: +# +# _replay Always, with the tests. Runs the harness over the checked-in +# corpus (corpus//: seeds plus minimized crash +# reproducers) as a ctest case labelled `unit`, on every +# compiler. A fixed crash stays fixed everywhere. +# _fuzz With MAFIANET_BUILD_FUZZERS=ON and Clang: a libFuzzer +# binary. Use the `fuzz` preset (ASan + UBSan): +# cmake --workflow --preset fuzz +# ./build/fuzz/Tests/Fuzz/bit_stream_fuzz Tests/Fuzz/corpus/bit_stream -max_total_time=60 +# +# Adding a harness: drop _fuzz.cpp here and create corpus//. + +file(GLOB MAFIANET_FUZZ_HARNESSES CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/*_fuzz.cpp") + +if(MAFIANET_BUILD_FUZZERS AND NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang") + message(FATAL_ERROR "MAFIANET_BUILD_FUZZERS needs Clang (libFuzzer); compiler is ${CMAKE_CXX_COMPILER_ID}") +endif() + +foreach(harness_source IN LISTS MAFIANET_FUZZ_HARNESSES) + get_filename_component(harness "${harness_source}" NAME_WE) + string(REGEX REPLACE "_fuzz$" "" corpus_name "${harness}") + set(corpus_dir "${CMAKE_CURRENT_SOURCE_DIR}/corpus/${corpus_name}") + + if(MAFIANET_BUILD_TESTS) + add_executable(${corpus_name}_replay "${harness_source}" replay_main.cpp) + target_link_libraries(${corpus_name}_replay PRIVATE MafiaNet::MafiaNetStatic) + target_compile_definitions(${corpus_name}_replay PRIVATE $<$:_DEBUG>) + set_target_properties(${corpus_name}_replay PROPERTIES FOLDER "Tests/Fuzz") + mafianet_enable_warnings(${corpus_name}_replay) + add_test(NAME FuzzCorpus.${corpus_name} COMMAND ${corpus_name}_replay "${corpus_dir}") + set_tests_properties(FuzzCorpus.${corpus_name} PROPERTIES LABELS unit TIMEOUT 600) + endif() + + if(MAFIANET_BUILD_FUZZERS) + add_executable(${harness} "${harness_source}") + target_link_libraries(${harness} PRIVATE MafiaNet::MafiaNetStatic) + target_compile_options(${harness} PRIVATE -fsanitize=fuzzer) + target_link_options(${harness} PRIVATE -fsanitize=fuzzer) + set_target_properties(${harness} PROPERTIES FOLDER "Tests/Fuzz") + mafianet_enable_warnings(${harness}) + endif() +endforeach() diff --git a/Tests/Fuzz/bit_stream_fuzz.cpp b/Tests/Fuzz/bit_stream_fuzz.cpp new file mode 100644 index 000000000..332d186b5 --- /dev/null +++ b/Tests/Fuzz/bit_stream_fuzz.cpp @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * Fuzzes BitStream deserialization: every Read* that game code applies to + * bytes received from the network. The first byte of each step picks the + * operation, so the fuzzer explores sequences of reads over one stream, as a + * real message handler does. Only bounded/safe read APIs are driven; the + * legacy unbounded ones (Read(char*), Read(wchar_t*&)) are unsafe by contract. + */ + +#include +#include +#include + +#include "mafianet/bit_stream.h" +#include "mafianet/string_compressor.h" +#include "mafianet/string.h" +#include "mafianet/wstring.h" + +using namespace MafiaNet; + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + // DeserializeCompressed uses the shared Huffman tables, which normally + // exist because a RakPeer holds a reference. Hold one for the process. + static const bool compressorReady = (StringCompressor::AddReference(), true); + (void) compressorReady; + + BitStream in(const_cast(data), (unsigned int) size, false); + + for (int step = 0; step < 64 && in.GetNumberOfUnreadBits() > 0; ++step) + { + uint8_t op = 0; + if (!in.Read(op)) + break; + + switch (op % 20) + { + case 0: { uint8_t v; in.Read(v); break; } + case 1: { uint32_t v; in.Read(v); break; } + case 2: { uint64_t v; in.ReadCompressed(v); break; } + case 3: { int32_t v; in.ReadCompressed(v); break; } + case 4: { float v; in.ReadCompressed(v); break; } + case 5: { double v; in.Read(v); break; } + case 6: { uint16_t v = 0; in.ReadDelta(v); break; } + case 7: { uint32_t v = 0; in.ReadCompressedDelta(v); break; } + case 8: { RakString s; s.Deserialize(&in); break; } + case 9: { RakString s; s.DeserializeCompressed(&in, op & 0x80); break; } + case 10: { RakWString s; s.Deserialize(&in); break; } + case 11: + { + char *buf = nullptr; + unsigned int len = 0; + in.ReadAlignedBytesSafeAlloc(&buf, len, 4096); + rakFree_Ex(buf, _FILE_AND_LINE_); + break; + } + case 12: + { + char buf[64]; + unsigned int len = 0; + in.ReadAlignedBytesSafe(buf, len, sizeof(buf)); + break; + } + case 13: { uint32_t v; in.ReadBitsFromIntegerRange(v, 10u, 5000u); break; } + case 14: { float x, y, z; in.ReadNormVector(x, y, z); break; } + case 15: { float x, y, z; in.ReadVector(x, y, z); break; } + case 16: { float w, x, y, z; in.ReadNormQuat(w, x, y, z); break; } + case 17: + { + float m[9]; + in.ReadOrthMatrix(m[0], m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8]); + break; + } + case 18: + { + // Nested stream: what message handlers do with sub-payloads. + BitStream sub; + uint16_t bits = 0; + if (in.Read(bits)) + in.Read(sub, bits % 4096); + break; + } + case 19: + { + unsigned char bytes[16]; + in.ReadBits(bytes, op % 128, (op & 1) != 0); + break; + } + } + } + return 0; +} diff --git a/Tests/Fuzz/corpus/bit_stream/numbers b/Tests/Fuzz/corpus/bit_stream/numbers new file mode 100644 index 0000000000000000000000000000000000000000..cdd6116b6f472a397c7930431d446d6228aec726 GIT binary patch literal 26 fcmZQ#00QQ<&$=4-|NsBBQUCwJ2S9;>O9~ADi*gHy literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/corpus/bit_stream/regress-safe-alloc-leak b/Tests/Fuzz/corpus/bit_stream/regress-safe-alloc-leak new file mode 100644 index 0000000000000000000000000000000000000000..8aec20e8324c2594902032eea8b8efe11608f087 GIT binary patch literal 70 ycmd<9-yp@nkiRK_fsp}-Bp4Wa86Y5lfq{c50W1X~bQu^Nz!D&}5K)LykRkw?Eelcr literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/corpus/bit_stream/strings b/Tests/Fuzz/corpus/bit_stream/strings new file mode 100644 index 0000000000000000000000000000000000000000..36d3393ea5c320f3805ecf635c0d4e9c90b28b43 GIT binary patch literal 44 zcmd;J;Lb?R$;nqJ&o9bJ;bdT7FbR~>oVd5W_FllU-;AeP8JHNX8%!FE7>pVKEw&B^ literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/corpus/bit_stream/vectors_and_blobs b/Tests/Fuzz/corpus/bit_stream/vectors_and_blobs new file mode 100644 index 0000000000000000000000000000000000000000..2d00b54a59b326902a5f7056ef7f20c9382b257a GIT binary patch literal 36 hcmd<9|NsAg{eJ-lFkpnz+&~@+LqTF?PJUtv0|2%W2qORh literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/corpus/reliability_layer/acks b/Tests/Fuzz/corpus/reliability_layer/acks new file mode 100644 index 0000000000000000000000000000000000000000..a7a84a71a056944a49a2374f98de5575c489ba66 GIT binary patch literal 24 Zcmd;LIKaTj00cmS3(RI>U|?oo000@l0i6H< literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/corpus/reliability_layer/ordered_split_sequenced b/Tests/Fuzz/corpus/reliability_layer/ordered_split_sequenced new file mode 100644 index 0000000000000000000000000000000000000000..fb264e49489bf73ea69afca25a880787e78d257e GIT binary patch literal 3112 zcmWe)XklPrNMKL^k|1z`k%>8kwTF>`p+NHikOTq{$qZ&OoTwNDqaiRF0;3@?8Umz- z0I*bL0+*^l5(p5bDkG^TjjA6Ffzc2c4S~@RsHjL|>;bk-6@U;7m_Y5<0xoEK6;Z}A KjfNFvApiibk%$xk literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/corpus/reliability_layer/regress-split-fragment-use-after-free b/Tests/Fuzz/corpus/reliability_layer/regress-split-fragment-use-after-free new file mode 100644 index 0000000000000000000000000000000000000000..4d12f751a06dad6b4d247965254b916f27414fbb GIT binary patch literal 2421 zcmeHJF-`+P44k`zf~`n_1XPizK#KB&^yyO|9+5BP2}Epq>J&));Dq2C{DL-)?d5Ja ziBVRHfE2enU9s}oJNAsd&Ulx83cL XZx2kPX>*{aY0HD7Z9+0{zWeqXStm58 literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/reliability_layer_fuzz.cpp b/Tests/Fuzz/reliability_layer_fuzz.cpp new file mode 100644 index 000000000..87f88606e --- /dev/null +++ b/Tests/Fuzz/reliability_layer_fuzz.cpp @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * Fuzzes the datagram parser of a connected ReliabilityLayer: datagram + * headers, ack/nak ranges, split-packet reassembly and ordering channels -- + * everything a connected remote controls. Input is a sequence of datagrams, + * each prefixed by a 16-bit little-endian length. Time advances 10ms per + * datagram so resend, ack and split timeouts run too. + */ + +#include +#include +#include + +#include "mafianet/bit_stream.h" +#include "mafianet/ds_list.h" +#include "mafianet/get_time.h" +#include "mafianet/mtu_size.h" +#include "mafianet/plugin_interface2.h" +#include "mafianet/rand.h" +#include "mafianet/reliability_layer.h" +#include "mafianet/socket2.h" + +using namespace MafiaNet; + +namespace +{ + // Acks and resends the layer emits are discarded. + struct NullSocket : public RakNetSocket2 + { + RNS2SendResult Send(RNS2_SendParameters *sendParameters, const char *, unsigned int) override + { + return sendParameters->length; + } + }; +} + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + static NullSocket socket; + static SystemAddress remote("127.0.0.1", 40001); + static RakNetRandom rnr; + DataStructures::List handlers; + BitStream updateBitStream(MAXIMUM_MTU_SIZE); + + ReliabilityLayer layer; + layer.Reset(true, MAXIMUM_MTU_SIZE, false); + layer.SetTimeoutTime(1000000); + CCTimeType now = GetTimeUS(); + + size_t offset = 0; + while (offset + 2 <= size) + { + size_t length = (size_t) data[offset] | ((size_t) data[offset + 1] << 8); + offset += 2; + if (length > size - offset) + length = size - offset; + if (length > MAXIMUM_MTU_SIZE) + length = MAXIMUM_MTU_SIZE; + + // The layer's parser takes a mutable buffer; give it its own copy so a + // write past the datagram is caught by ASan instead of corrupting input. + std::vector datagram(data + offset, data + offset + length); + offset += length; + if (datagram.empty()) + continue; + + layer.HandleSocketReceiveFromConnectedPlayer(datagram.data(), (unsigned int) datagram.size(), + remote, handlers, MAXIMUM_MTU_SIZE, &socket, &rnr, now, updateBitStream); + + now += 10000; + layer.Update(&socket, remote, MAXIMUM_MTU_SIZE, now, 0, handlers, &rnr, updateBitStream); + + unsigned char *message; + while (layer.Receive(&message) > 0) + rakFree_Ex(message, _FILE_AND_LINE_); + } + return 0; +} diff --git a/Tests/Fuzz/replay_main.cpp b/Tests/Fuzz/replay_main.cpp new file mode 100644 index 000000000..424bce9ea --- /dev/null +++ b/Tests/Fuzz/replay_main.cpp @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * Runs a fuzz harness over files, without libFuzzer. Every harness is built + * this way alongside the test suites, so the checked-in corpus (seeds and + * minimized crash reproducers) is replayed on every compiler and platform, + * not just in the Clang fuzzing job. + * + * Usage: _replay ... + */ + +#include +#include +#include +#include +#include +#include + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size); + +namespace +{ + int RunFile(const std::filesystem::path &path) + { + std::ifstream in(path, std::ios::binary); + if (!in) + { + std::fprintf(stderr, "cannot read %s\n", path.string().c_str()); + return 1; + } + std::vector bytes((std::istreambuf_iterator(in)), std::istreambuf_iterator()); + LLVMFuzzerTestOneInput(bytes.empty() ? nullptr : bytes.data(), bytes.size()); + return 0; + } +} + +int main(int argc, char **argv) +{ + int failures = 0; + int ran = 0; + for (int i = 1; i < argc; ++i) + { + const std::filesystem::path arg(argv[i]); + if (std::filesystem::is_directory(arg)) + { + for (const auto &entry : std::filesystem::directory_iterator(arg)) + { + if (!entry.is_regular_file()) + continue; + failures += RunFile(entry.path()); + ++ran; + } + } + else + { + failures += RunFile(arg); + ++ran; + } + } + std::printf("replayed %d input(s)\n", ran); + return failures == 0 ? 0 : 1; +} From 95b3ca34c963080da15ac9768d9fee77370048af Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:23:09 +0200 Subject: [PATCH 05/12] ci: fuzzing, benchmark and coverage jobs; warnings as errors on Linux/macOS - fuzz: each libFuzzer harness for 60 s from the checked-in corpus under ASan + UBSan; a crash uploads its reproducer. - fuzz.yml (nightly, and on demand): 15 minutes per harness, with the grown corpus cached from night to night. - benchmarks: build and smoke-run the microbenchmarks, upload the JSON. Shared runners are too noisy to gate on numbers. - coverage: both suites under --coverage, gcovr summary in the job summary and the HTML report as an artifact. Informational. - linux-native, linux-minimal, macos and the sanitizer jobs build with MAFIANET_WARNINGS_AS_ERRORS. Windows (MSVC /W4) and the Ubuntu 22.04 Docker job keep warnings visible but non-fatal until measured. - Documents why the Docker job stays next to linux-native: it is the oldest supported toolchain (GCC 11, CMake 3.22). --- .github/workflows/build.yml | 140 +++++++++++++++++++++++++++++++++++- .github/workflows/fuzz.yml | 74 +++++++++++++++++++ 2 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/fuzz.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 79f27459a..174f5bc14 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,7 +23,9 @@ env: SCCACHE_GHA_ENABLED: "true" jobs: - # Linux: Build and test in Docker container (full network access). + # Linux in Docker on Ubuntu 22.04 (GCC 11, CMake 3.22): the oldest toolchain + # we support, kept on purpose next to linux-native (ubuntu-latest), so a + # change that needs a newer compiler or CMake fails here first. # Tests run through CTest inside the container: one process per test, # per-test timeout, and bounded retries for transient timing misses. linux: @@ -99,6 +101,7 @@ jobs: run: | cmake -B build \ -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} \ + -DMAFIANET_WARNINGS_AS_ERRORS=ON \ -DCMAKE_C_COMPILER_LAUNCHER=sccache \ -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \ -DMAFIANET_BUILD_TESTS=ON \ @@ -157,6 +160,7 @@ jobs: run: | cmake -B build \ -DCMAKE_BUILD_TYPE=Release \ + -DMAFIANET_WARNINGS_AS_ERRORS=ON \ -DCMAKE_C_COMPILER_LAUNCHER=sccache \ -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \ -DMAFIANET_MINIMAL=ON @@ -205,6 +209,7 @@ jobs: - name: Configure CMake run: | cmake --preset ${{ matrix.preset }} -G Ninja \ + -DMAFIANET_WARNINGS_AS_ERRORS=ON \ -DCMAKE_C_COMPILER=clang \ -DCMAKE_CXX_COMPILER=clang++ \ -DCMAKE_C_COMPILER_LAUNCHER=sccache \ @@ -249,6 +254,136 @@ jobs: build/${{ matrix.preset }}/integration.log if-no-files-found: ignore + # Fuzzing smoke run: each libFuzzer harness (Tests/Fuzz) for one minute from + # the checked-in corpus, under ASan + UBSan. The nightly fuzz.yml runs longer + # and keeps a growing corpus; this catches what a PR breaks right away. A + # crash uploads the reproducer: minimize it and add it to + # Tests/Fuzz/corpus// with the fix, so every build replays it. + fuzz: + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Install dependencies + run: sudo apt-get update && sudo apt-get install -y libssl-dev ninja-build clang + + - name: Set up sccache + uses: mozilla-actions/sccache-action@v0.0.11 + + - name: Build harnesses + run: | + cmake --preset fuzz -G Ninja \ + -DCMAKE_C_COMPILER_LAUNCHER=sccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=sccache + cmake --build --preset fuzz + + - name: Fuzz each harness for 60 seconds + env: + ASAN_OPTIONS: detect_leaks=0 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 + run: | + mkdir -p fuzz-artifacts + status=0 + for harness in build/fuzz/Tests/Fuzz/*_fuzz; do + name=$(basename "$harness" _fuzz) + work=$(mktemp -d) + cp Tests/Fuzz/corpus/"$name"/* "$work"/ + echo "::group::$name" + "$harness" "$work" -max_total_time=60 -print_final_stats=1 \ + -artifact_prefix=fuzz-artifacts/"$name"- || status=1 + echo "::endgroup::" + done + exit $status + + - name: Upload crash reproducers + if: failure() + uses: actions/upload-artifact@v4 + with: + name: fuzz-crashes + path: fuzz-artifacts/ + + # Benchmarks smoke run: proves they build and run. Shared runners are too + # noisy to gate on numbers; compare locally with the bench preset instead. + benchmarks: + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Install dependencies + run: sudo apt-get update && sudo apt-get install -y libssl-dev ninja-build + + - name: Set up sccache + uses: mozilla-actions/sccache-action@v0.0.11 + + - name: Build + run: | + cmake --preset bench -G Ninja \ + -DCMAKE_C_COMPILER_LAUNCHER=sccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=sccache + cmake --build --preset bench + + - name: Run + run: | + ./build/bench/Benchmarks/MafiaNetBenchmarks \ + --benchmark_min_time=0.05s \ + --benchmark_out=benchmarks.json --benchmark_out_format=json + + - name: Upload results + uses: actions/upload-artifact@v4 + with: + name: benchmarks + path: benchmarks.json + + # Line coverage of Source/ by both suites, written to the job summary with + # an HTML report as an artifact. Informational: nothing gates on it. + coverage: + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Install dependencies + run: | + sudo apt-get update && sudo apt-get install -y libssl-dev ninja-build + pipx install gcovr==8.6 + + - name: Build + run: | + cmake --preset coverage -G Ninja + cmake --build --preset coverage + + - name: Run tests + run: | + ctest --preset coverage -L unit --parallel 4 + ctest --preset coverage -L integration --repeat until-pass:3 + + - name: Report + if: always() + run: | + mkdir -p coverage-html + gcovr --root . --filter 'Source/' build/coverage \ + --html-details coverage-html/index.html \ + --markdown-summary coverage-summary.md --print-summary + cat coverage-summary.md >> "$GITHUB_STEP_SUMMARY" + + - name: Upload report + if: always() + uses: actions/upload-artifact@v4 + with: + name: coverage-html + path: coverage-html/ + # macOS: Build and run both suites (unit + integration). macos: runs-on: macos-latest @@ -267,7 +402,8 @@ jobs: - name: Configure CMake run: | cmake -B build \ - -DOPENSSL_ROOT_DIR=$(brew --prefix openssl@3) \ + -DOPENSSL_ROOT_DIR="$(brew --prefix openssl@3)" \ + -DMAFIANET_WARNINGS_AS_ERRORS=ON \ -DCMAKE_C_COMPILER_LAUNCHER=sccache \ -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \ -DMAFIANET_BUILD_SAMPLES=ON \ diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 000000000..70ef52a6c --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,74 @@ +name: Nightly fuzzing + +# Longer libFuzzer runs than the per-PR smoke job in build.yml. The corpus each +# run grows is cached and restored by the next, so coverage keeps building up +# across nights instead of restarting from Tests/Fuzz/corpus every time. +on: + schedule: + - cron: "17 3 * * *" + workflow_dispatch: + inputs: + seconds: + description: Seconds per harness + default: "900" + +permissions: + contents: read + +jobs: + fuzz: + runs-on: ubuntu-latest + timeout-minutes: 120 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Install dependencies + run: sudo apt-get update && sudo apt-get install -y libssl-dev ninja-build clang + + - name: Build harnesses + run: | + cmake --preset fuzz -G Ninja + cmake --build --preset fuzz + + - name: Restore corpus + uses: actions/cache/restore@v4 + with: + path: fuzz-corpus + key: fuzz-corpus-${{ github.run_id }} + restore-keys: fuzz-corpus- + + - name: Fuzz + env: + ASAN_OPTIONS: detect_leaks=0 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 + SECONDS_PER_HARNESS: ${{ inputs.seconds || '900' }} + run: | + mkdir -p fuzz-artifacts + status=0 + for harness in build/fuzz/Tests/Fuzz/*_fuzz; do + name=$(basename "$harness" _fuzz) + mkdir -p fuzz-corpus/"$name" + cp -n Tests/Fuzz/corpus/"$name"/* fuzz-corpus/"$name"/ || true + echo "::group::$name" + "$harness" fuzz-corpus/"$name" -max_total_time="$SECONDS_PER_HARNESS" \ + -print_final_stats=1 -artifact_prefix=fuzz-artifacts/"$name"- || status=1 + echo "::endgroup::" + done + exit $status + + - name: Save corpus + if: always() + uses: actions/cache/save@v4 + with: + path: fuzz-corpus + key: fuzz-corpus-${{ github.run_id }} + + - name: Upload crash reproducers + if: failure() + uses: actions/upload-artifact@v4 + with: + name: fuzz-crashes + path: fuzz-artifacts/ From 3a752dcb86d74a6266e3581bf92289b7d7698b13 Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:29:30 +0200 Subject: [PATCH 06/12] test: run whole RakPeer instances over a simulated network Tests/Support/SimulatedNetwork drives real peers through connect, data transfer, MTU discovery, loss, reordering and timeouts with no sockets, no threads and a virtual clock, so those scenarios are deterministic, take milliseconds, and belong in the unit suite instead of loopback integration tests. Per-direction link rules model loss, latency, jitter, a path MTU or a dark link; randomness is a seeded LCG. Library seams, all opt-in and inert for normal use: - SetTimeSourceOverride (get_time.h): route GetTimeUS to a test clock. One atomic load per time query when unset. - RakNetSocket2Factory + RNS2T_USER (socket2.h): RakPeer::SetSocketFactory makes Startup take its sockets from a factory instead of binding OS sockets; IsBerkleySocket() is now false only for such sockets. - RakPeer::SetManualUpdate / RunManualUpdate: no internal update thread; the caller runs the update cycle. Shutdown(blockDuration) and GetSocket(s) work without the thread in that mode. Fixes found on the way: - RakNetSocket2 never initialized socketType outside AllocRNS2, so IsBerkleySocket() on any other subclass read garbage. - On Windows, Get64BitUniqueRandomNumber used only timer jitter, which collides for peers created back to back (and always under a virtual clock); it now mixes in random_device and a counter like POSIX. SimulatedPeerTests covers the handshake, ReliableOrdered under 20% loss with reordering, split reassembly, disconnect notification, timeout on a dark link, MTU discovery under a path ceiling, the incoming-connection limit, and run-to-run determinism. --- Source/include/mafianet/get_time.h | 9 + Source/include/mafianet/peer.h | 18 ++ Source/include/mafianet/socket2.h | 20 +- Source/src/get_time.cpp | 14 ++ Source/src/peer.cpp | 83 ++++++-- Source/src/socket2.cpp | 14 +- Tests/Support/SimulatedNetwork.cpp | 291 +++++++++++++++++++++++++++++ Tests/Support/SimulatedNetwork.h | 141 ++++++++++++++ Tests/Unit/SimulatedPeerTests.cpp | 202 ++++++++++++++++++++ 9 files changed, 776 insertions(+), 16 deletions(-) create mode 100644 Tests/Support/SimulatedNetwork.cpp create mode 100644 Tests/Support/SimulatedNetwork.h create mode 100644 Tests/Unit/SimulatedPeerTests.cpp diff --git a/Source/include/mafianet/get_time.h b/Source/include/mafianet/get_time.h index 5fd21f39c..5d11e4c5c 100644 --- a/Source/include/mafianet/get_time.h +++ b/Source/include/mafianet/get_time.h @@ -38,6 +38,15 @@ namespace MafiaNet /// \note The maximum delta between returned calls is 1 second - however, RakNet calls this constantly anyway. See NormalizeTime() in the cpp. MafiaNet::TimeUS RAK_DLL_EXPORT GetTimeUS( void ); + /// A replacement clock for GetTimeUS (and so GetTime / GetTimeMS). + typedef MafiaNet::TimeUS (*TimeSourceUS)( void ); + + /// \internal Testing seam: route every MafiaNet time query to \a source, + /// process wide; nullptr restores the real clock. The source must be + /// monotonic, and nothing should run a real (threaded) peer while it is set. + /// Used by the simulated network in Tests/Support. + void RAK_DLL_EXPORT SetTimeSourceOverride( TimeSourceUS source ); + /// a > b? extern RAK_DLL_EXPORT bool GreaterThan(MafiaNet::Time a, MafiaNet::Time b); /// a < b? diff --git a/Source/include/mafianet/peer.h b/Source/include/mafianet/peer.h index 37173f912..16408dd58 100644 --- a/Source/include/mafianet/peer.h +++ b/Source/include/mafianet/peer.h @@ -46,6 +46,7 @@ namespace MafiaNet { /// Forward declarations class HuffmanEncodingTree; +class RakNetSocket2Factory; class PluginInterface2; // Sucks but this struct has to be outside the class. Inside and DevCPP won't let you refer to the struct as RakPeer::RemoteSystemIndex while GCC @@ -630,6 +631,20 @@ class RAK_DLL_EXPORT RakPeer : public RakPeerInterface, public RNS2EventHandler /// \param[in] _userUpdateThreadData Passed to C callback function virtual void SetUserUpdateThread(void (*_userUpdateThreadPtr)(RakPeerInterface *, void *), void *_userUpdateThreadData); + /// \internal Testing seams, used by Tests/Support/SimulatedNetwork to run + /// peers over an in-process network with a virtual clock. Call the two + /// setters before Startup. + /// + /// Sockets come from \a factory instead of being bound on the OS. + void SetSocketFactory(RakNetSocket2Factory *factory); + /// No internal update thread: the caller drives the peer with + /// RunManualUpdate(). Shutdown(blockDuration) then flushes once instead of + /// waiting, since nothing else would advance the peer meanwhile. + void SetManualUpdate(bool manual); + /// One update-thread iteration: the user update callback, then the update + /// cycle. Only valid with SetManualUpdate(true). + void RunManualUpdate(); + /// Set a C callback to be called whenever a datagram arrives /// Return true from the callback to have RakPeer handle the datagram. Return false and RakPeer will ignore the datagram. /// This can be used to filter incoming datagrams by system, or to share a recvfrom socket with RakPeer @@ -854,6 +869,9 @@ class RAK_DLL_EXPORT RakPeer : public RakPeerInterface, public RNS2EventHandler std::atomic endThreads; /// true if the peer thread is active. std::atomic isMainLoopThreadActive; + /// See SetSocketFactory / SetManualUpdate. + RakNetSocket2Factory *socketFactory; + bool manualUpdate; // MafiaNet::LocklessUint32_t isRecvFromLoopThreadActive; diff --git a/Source/include/mafianet/socket2.h b/Source/include/mafianet/socket2.h index 625937ae2..d7ee08b1a 100644 --- a/Source/include/mafianet/socket2.h +++ b/Source/include/mafianet/socket2.h @@ -98,7 +98,10 @@ bool RNS2_IsDatagramTooLargeError(int err); enum RNS2Type { RNS2T_WINDOWS, - RNS2T_LINUX + RNS2T_LINUX, + /// Supplied by a RakNetSocket2Factory, not a Berkeley socket: RakPeer never + /// binds it, sets options on it or starts a receive thread for it. + RNS2T_USER }; struct RNS2_SendParameters @@ -121,6 +124,21 @@ struct RNS2RecvStruct RakNetSocket2 *socket; }; +/// \internal Testing seam: hands RakPeer its sockets instead of binding OS +/// sockets. See RakPeer::SetSocketFactory and Tests/Support/SimulatedNetwork. +class RAK_DLL_EXPORT RakNetSocket2Factory +{ +public: + virtual ~RakNetSocket2Factory() {} + /// An already-bound socket of type RNS2T_USER for socket descriptor + /// \a index, or nullptr (Startup then fails with SOCKET_PORT_ALREADY_IN_USE). + /// RakPeer installs itself as the socket's event handler; deliver received + /// datagrams through it (AllocRNS2RecvStruct, then OnRNS2Recv). + virtual RakNetSocket2 *Create( const SocketDescriptor &descriptor, unsigned int index ) = 0; + /// Called instead of deleting a socket Create returned. + virtual void Destroy( RakNetSocket2 *socket ) = 0; +}; + class RakNetSocket2Allocator { public: diff --git a/Source/src/get_time.cpp b/Source/src/get_time.cpp index 120d31910..405d51570 100644 --- a/Source/src/get_time.cpp +++ b/Source/src/get_time.cpp @@ -27,6 +27,8 @@ #include "mafianet/get_time.h" +#include + @@ -205,8 +207,20 @@ MafiaNet::TimeUS GetTimeUS_Linux( void ) } #endif +namespace +{ + std::atomic g_timeSourceOverride{nullptr}; +} + +void MafiaNet::SetTimeSourceOverride( TimeSourceUS source ) +{ + g_timeSourceOverride.store(source, std::memory_order_release); +} + MafiaNet::TimeUS MafiaNet::GetTimeUS( void ) { + if (TimeSourceUS source = g_timeSourceOverride.load(std::memory_order_acquire)) + return source(); diff --git a/Source/src/peer.cpp b/Source/src/peer.cpp index c50fbfadb..47e49a2be 100644 --- a/Source/src/peer.cpp +++ b/Source/src/peer.cpp @@ -246,6 +246,8 @@ RakPeer::RakPeer() bytesSentPerSecond = bytesReceivedPerSecond = 0; endThreads = true; isMainLoopThreadActive = false; + socketFactory = nullptr; + manualUpdate = false; incomingDatagramEventHandler=0; sessionConfigInteractive=false; maximumPendingSessions=0; @@ -473,7 +475,12 @@ StartupResult RakPeer::Startup( unsigned int maxConnections, SocketDescriptor *s addrToBind=socketDescriptors[i].hostAddress; */ - RakNetSocket2 *r2 = RakNetSocket2Allocator::AllocRNS2(); + RakNetSocket2 *r2 = socketFactory ? socketFactory->Create(socketDescriptors[i], i) : RakNetSocket2Allocator::AllocRNS2(); + if (r2 == nullptr) + { + DerefAllSockets(); + return SOCKET_PORT_ALREADY_IN_USE; + } r2->SetUserConnectionSocketIndex(i); if (r2->IsBerkleySocket()) { @@ -520,7 +527,8 @@ StartupResult RakPeer::Startup( unsigned int maxConnections, SocketDescriptor *s } else { - RakAssert("TODO" && 0); + // Factory-supplied: already bound; it delivers datagrams to us. + r2->SetRecvEventHandler(this); } socketList.Push(r2, _FILE_AND_LINE_ ); @@ -545,6 +553,10 @@ StartupResult RakPeer::Startup( unsigned int maxConnections, SocketDescriptor *s ipList[i].SetPortHostOrder(port); } + else + { + ipList[i].SetPortHostOrder(socketList[0]->GetBoundAddress().GetPort()); + } } if ( maximumNumberOfPeers == 0 ) @@ -618,7 +630,7 @@ StartupResult RakPeer::Startup( unsigned int maxConnections, SocketDescriptor *s ClearBufferedPackets(); ClearSocketQueryOutput(); - if ( isMainLoopThreadActive == false ) + if ( isMainLoopThreadActive == false && !manualUpdate ) { #if RAKPEER_USER_THREADED!=1 @@ -674,7 +686,7 @@ StartupResult RakPeer::Startup( unsigned int maxConnections, SocketDescriptor *s #if RAKPEER_USER_THREADED!=1 // Wait for the threads to activate. When they are active they will set these variables to true - while ( isMainLoopThreadActive == false ) + while ( !manualUpdate && isMainLoopThreadActive == false ) RakSleep(10); #endif // RAKPEER_USER_THREADED!=1 } @@ -1015,7 +1027,11 @@ void RakPeer::Shutdown( unsigned int blockDuration, unsigned char orderingChanne time = MafiaNet::GetTimeMS(); startWaitingTime = time; - while ( time - startWaitingTime < blockDuration ) + // Nothing else advances a manually updated peer (or its clock, under a + // test's virtual time): flush the notifications once and stop. + if (manualUpdate) + RunManualUpdate(); + while ( !manualUpdate && time - startWaitingTime < blockDuration ) { anyActive=false; for (j=0; j < systemListSize; j++) @@ -3009,12 +3025,14 @@ RakNetSocket2* RakPeer::GetSocket( const SystemAddress target ) SocketQueryOutput *sqo; MafiaNet::TimeMS stopWaiting = MafiaNet::GetTimeMS()+1000; DataStructures::List output; - while (MafiaNet::GetTimeMS() < stopWaiting) + while (manualUpdate || MafiaNet::GetTimeMS() < stopWaiting) { - if (isMainLoopThreadActive==false) + if (manualUpdate) + RunManualUpdate(); // answers the query on this thread + else if (isMainLoopThreadActive==false) return 0; - - RakSleep(0); + else + RakSleep(0); sqo = socketQueryOutput.Pop(); if (sqo) @@ -3048,10 +3066,12 @@ void RakPeer::GetSockets( DataStructures::List &sockets ) // RakNetSocket2* output; for(;;) { - if (isMainLoopThreadActive==false) + if (manualUpdate) + RunManualUpdate(); // answers the query on this thread + else if (isMainLoopThreadActive==false) return; - - RakSleep(0); + else + RakSleep(0); sqo = socketQueryOutput.Pop(); if (sqo) @@ -3131,6 +3151,32 @@ void RakPeer::SetUserUpdateThread(void (*_userUpdateThreadPtr)(RakPeerInterface userUpdateThreadData=_userUpdateThreadData; } // -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +void RakPeer::SetSocketFactory(RakNetSocket2Factory *factory) +{ + RakAssert(IsActive() == false); + socketFactory = factory; +} +// -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +void RakPeer::SetManualUpdate(bool manual) +{ + RakAssert(IsActive() == false); + manualUpdate = manual; +} +// -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +void RakPeer::RunManualUpdate() +{ + RakAssert(manualUpdate); + // The same steps as one pass of UpdateNetworkLoop. + if (userUpdateThreadPtr) + userUpdateThreadPtr(this, userUpdateThreadData); + BitStream updateBitStream( MAXIMUM_MTU_SIZE +#if LIBCAT_SECURITY==1 + + cat::AuthenticatedEncryption::OVERHEAD_BYTES +#endif + ); + RunUpdateCycle(updateBitStream); +} +// -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- void RakPeer::SetIncomingDatagramEventHandler( bool (*_incomingDatagramEventHandler)(RNS2RecvStruct *) ) { incomingDatagramEventHandler=_incomingDatagramEventHandler; @@ -4852,6 +4898,14 @@ uint64_t RakPeerInterface::Get64BitUniqueRandomNumber(void) diff4Bits >>= j*4; ((char*)&g)[j] ^= diff4Bits; } + // Timer jitter alone collides when peers are created back to back, and is + // constant under a test's virtual clock. Mix in the same entropy as below. + static std::atomic sequence {0}; + std::random_device entropy; + g ^= ((uint64_t)entropy() << 32) ^ (uint64_t)entropy(); + g ^= (sequence.fetch_add(1, std::memory_order_relaxed) + 1) * 0x9E3779B97F4A7C15ull; + if (g == 0 || g == (uint64_t)-1) + g = 1; return g; #else @@ -5840,7 +5894,10 @@ void RakPeer::DerefAllSockets(void) unsigned int i; for (i=0; i < socketList.Size(); i++) { - MafiaNet::OP_DELETE(socketList[i], _FILE_AND_LINE_); + if (socketFactory && !socketList[i]->IsBerkleySocket()) + socketFactory->Destroy(socketList[i]); + else + MafiaNet::OP_DELETE(socketList[i], _FILE_AND_LINE_); } socketList.Clear(false, _FILE_AND_LINE_); } diff --git a/Source/src/socket2.cpp b/Source/src/socket2.cpp index 445d0fc9c..856632a41 100644 --- a/Source/src/socket2.cpp +++ b/Source/src/socket2.cpp @@ -77,7 +77,16 @@ bool MafiaNet::RNS2_IsDatagramTooLargeError(int err) } void RakNetSocket2Allocator::DeallocRNS2(RakNetSocket2 *s) { MafiaNet::OP_DELETE(s,_FILE_AND_LINE_);} -RakNetSocket2::RakNetSocket2() {eventHandler=0;} +RakNetSocket2::RakNetSocket2() + : eventHandler(0) +#ifdef _WIN32 + , socketType(RNS2T_WINDOWS) +#else + , socketType(RNS2T_LINUX) +#endif + , userConnectionSocketIndex(0) +{ +} RakNetSocket2::~RakNetSocket2() {} void RakNetSocket2::SetRecvEventHandler(RNS2EventHandler *_eventHandler) {eventHandler=_eventHandler;} RNS2SendResult RakNetSocket2::SendBatch( RNS2_SendParameters *sends, unsigned count, const char *file, unsigned int line ) @@ -103,7 +112,8 @@ RNS2SendResult RakNetSocket2::SendBatch( RNS2_SendParameters *sends, unsigned co RNS2Type RakNetSocket2::GetSocketType(void) const {return socketType;} void RakNetSocket2::SetSocketType(RNS2Type t) {socketType=t;} bool RakNetSocket2::IsBerkleySocket(void) const { - return true; // All supported platforms use Berkeley sockets + // Every OS socket is a Berkeley socket; only factory-supplied ones are not. + return socketType != RNS2T_USER; } SystemAddress RakNetSocket2::GetBoundAddress(void) const {return boundAddress;} diff --git a/Tests/Support/SimulatedNetwork.cpp b/Tests/Support/SimulatedNetwork.cpp new file mode 100644 index 000000000..853920953 --- /dev/null +++ b/Tests/Support/SimulatedNetwork.cpp @@ -0,0 +1,291 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + */ + +#include "SimulatedNetwork.h" + +#include +#include +#include + +#include "mafianet/assert.h" +#include "mafianet/message_identifiers.h" + +namespace MafiaNet +{ + namespace + { + // Starts away from zero: a time of 0 means "never" in several places. + const TimeUS kEpochUS = 1000000; + TimeUS g_virtualNowUS = kEpochUS; + bool g_networkAlive = false; + } + + // A bound endpoint on the simulated network. Send() hands the datagram to + // the network; delivery happens in SimulatedNetwork::Step. + class SimulatedNetwork::SimSocket : public RakNetSocket2 + { + public: + SimSocket(SimulatedNetwork *network, const SystemAddress &address) + : network(network) + { + SetSocketType(RNS2T_USER); + boundAddress = address; + } + + RNS2SendResult Send(RNS2_SendParameters *sendParameters, const char *, unsigned int) override + { + network->Enqueue(boundAddress, sendParameters->systemAddress, sendParameters->data, sendParameters->length); + return sendParameters->length; + } + + private: + SimulatedNetwork *network; + }; + + SimulatedNetwork::SimulatedNetwork(uint32_t seed) + : lcgState(seed ? seed : 1) + { + RakAssert(!g_networkAlive && "only one SimulatedNetwork at a time"); + g_networkAlive = true; + g_virtualNowUS = kEpochUS; + SetTimeSourceOverride(&SimulatedNetwork::VirtualClock); + } + + SimulatedNetwork::~SimulatedNetwork() + { + for (RakPeer *peer : peers) + { + peer->Shutdown(0); + RakPeerInterface::DestroyInstance(peer); + } + SetTimeSourceOverride(nullptr); + g_networkAlive = false; + } + + TimeUS SimulatedNetwork::VirtualClock() + { + return g_virtualNowUS; + } + + TimeUS SimulatedNetwork::Now() const + { + return g_virtualNowUS; + } + + RakPeer *SimulatedNetwork::AddPeer(unsigned int maxConnections, unsigned short maxIncoming, unsigned short port) + { + RakPeer *peer = static_cast(RakPeerInterface::GetInstance()); + peer->SetSocketFactory(this); + peer->SetManualUpdate(true); + + char host[32]; + std::snprintf(host, sizeof(host), "10.0.0.%u", (unsigned) nextHost++); + // Skip the OS address enumeration: the peer reports this address as + // its own, so handshakes carry the same bytes on every machine. + peer->SetInternalID(SystemAddress(host, port ? port : 60000), 0); + + SocketDescriptor descriptor(port ? port : 60000, host); + const StartupResult result = peer->Startup(maxConnections, &descriptor, 1); + RakAssert(result == RAKNET_STARTED); + (void) result; + peer->SetMaximumIncomingConnections(maxIncoming); + + peers.push_back(peer); + inboxes[peer]; + return peer; + } + + SystemAddress SimulatedNetwork::AddressOf(const RakPeer *peer) const + { + return const_cast(peer)->GetInternalID(UNASSIGNED_SYSTEM_ADDRESS, 0); + } + + RakNetSocket2 *SimulatedNetwork::Create(const SocketDescriptor &descriptor, unsigned int) + { + SystemAddress address(descriptor.hostAddress, descriptor.port); + if (sockets.count(address)) + return nullptr; // port in use + SimSocket *socket = new SimSocket(this, address); + sockets[address] = socket; + return socket; + } + + void SimulatedNetwork::Destroy(RakNetSocket2 *socket) + { + for (auto it = sockets.begin(); it != sockets.end(); ++it) + { + if (it->second == socket) + { + sockets.erase(it); + break; + } + } + delete static_cast(socket); + } + + void SimulatedNetwork::SetLink(const RakPeer *from, const RakPeer *to, const LinkRule &rule) + { + links[std::make_pair(AddressOf(from), AddressOf(to))] = rule; + } + + void SimulatedNetwork::SetLinkBothWays(const RakPeer *a, const RakPeer *b, const LinkRule &rule) + { + SetLink(a, b, rule); + SetLink(b, a, rule); + } + + const SimulatedNetwork::LinkRule &SimulatedNetwork::RuleFor(const SystemAddress &from, const SystemAddress &to) const + { + auto it = links.find(std::make_pair(from, to)); + return it == links.end() ? defaultRule : it->second; + } + + uint32_t SimulatedNetwork::NextRandom() + { + lcgState = lcgState * 1664525u + 1013904223u; + return lcgState >> 8; + } + + void SimulatedNetwork::InjectDatagram(const SystemAddress &from, const RakPeer *to, const unsigned char *data, int length) + { + InFlight datagram; + datagram.deliverAt = g_virtualNowUS; + datagram.sequence = nextSequence++; + datagram.from = from; + datagram.to = AddressOf(to); + datagram.bytes.assign(data, data + length); + inFlight.push_back(std::move(datagram)); + } + + void SimulatedNetwork::Enqueue(const SystemAddress &from, const SystemAddress &to, const char *data, int length) + { + ++datagramsSent; + if (tap) + tap(from, to, data, length); + const LinkRule &rule = RuleFor(from, to); + if (rule.dark || length > rule.maxDatagramBytes || + (rule.lossPercent > 0 && (int) (NextRandom() % 100u) < rule.lossPercent)) + { + ++datagramsDropped; + return; + } + uint32_t delayMs = rule.delayMs; + if (rule.jitterMs) + delayMs += NextRandom() % (rule.jitterMs + 1); + + InFlight datagram; + datagram.deliverAt = g_virtualNowUS + (TimeUS) delayMs * 1000; + datagram.sequence = nextSequence++; + datagram.from = from; + datagram.to = to; + datagram.bytes.assign(data, data + length); + inFlight.push_back(std::move(datagram)); + } + + void SimulatedNetwork::DeliverDue() + { + // Deliveries can enqueue replies; take the due set first. + std::vector due; + auto split = std::stable_partition(inFlight.begin(), inFlight.end(), + [](const InFlight &d) { return d.deliverAt > g_virtualNowUS; }); + std::move(split, inFlight.end(), std::back_inserter(due)); + inFlight.erase(split, inFlight.end()); + std::sort(due.begin(), due.end(), [](const InFlight &a, const InFlight &b) { + return a.deliverAt != b.deliverAt ? a.deliverAt < b.deliverAt : a.sequence < b.sequence; + }); + + for (InFlight &datagram : due) + { + auto it = sockets.find(datagram.to); + if (it == sockets.end()) + { + ++datagramsDropped; + continue; + } + SimSocket *socket = it->second; + RNS2EventHandler *handler = socket->GetEventHandler(); + if (!handler) + continue; + if (datagram.bytes.empty() || datagram.bytes.size() > MAXIMUM_MTU_SIZE) + continue; + RNS2RecvStruct *recv = handler->AllocRNS2RecvStruct(_FILE_AND_LINE_); + std::memcpy(recv->data, datagram.bytes.data(), datagram.bytes.size()); + recv->bytesRead = (int) datagram.bytes.size(); + recv->systemAddress = datagram.from; + recv->timeRead = g_virtualNowUS; + recv->socket = socket; + handler->OnRNS2Recv(recv); + } + } + + void SimulatedNetwork::Step(uint32_t ms) + { + for (uint32_t tick = 0; tick < ms; ++tick) + { + g_virtualNowUS += 1000; + DeliverDue(); + for (RakPeer *peer : peers) + { + peer->RunManualUpdate(); + for (Packet *packet = peer->Receive(); packet; packet = peer->Receive()) + { + Message message; + message.id = packet->length ? packet->data[0] : 0; + message.data.assign(packet->data, packet->data + packet->length); + message.systemAddress = packet->systemAddress; + message.guid = packet->guid; + inboxes[peer].push_back(std::move(message)); + peer->DeallocatePacket(packet); + } + } + } + } + + bool SimulatedNetwork::RunUntil(const std::function &pred, uint32_t maxSimMs, uint32_t stepMs) + { + for (uint32_t elapsed = 0; elapsed < maxSimMs; elapsed += stepMs) + { + if (pred()) + return true; + Step(stepMs); + } + return pred(); + } + + std::vector &SimulatedNetwork::Inbox(const RakPeer *peer) + { + return inboxes[peer]; + } + + const SimulatedNetwork::Message *SimulatedNetwork::Find(const RakPeer *peer, MessageID id) const + { + auto it = inboxes.find(peer); + if (it == inboxes.end()) + return nullptr; + for (const Message &message : it->second) + if (message.id == id) + return &message; + return nullptr; + } + + bool SimulatedNetwork::RunUntilReceived(const RakPeer *peer, MessageID id, uint32_t maxSimMs) + { + return RunUntil([&] { return Received(peer, id); }, maxSimMs); + } + + bool SimulatedNetwork::Connect(RakPeer *client, RakPeer *server, uint32_t maxSimMs) + { + const SystemAddress target = AddressOf(server); + char host[64]; + target.ToString(false, host, sizeof(host)); + if (client->Connect(host, target.GetPort(), nullptr, 0) != CONNECTION_ATTEMPT_STARTED) + return false; + return RunUntil([&] { + return Received(client, ID_CONNECTION_REQUEST_ACCEPTED) && Received(server, ID_NEW_INCOMING_CONNECTION); + }, maxSimMs); + } +} diff --git a/Tests/Support/SimulatedNetwork.h b/Tests/Support/SimulatedNetwork.h new file mode 100644 index 000000000..d7a0868ce --- /dev/null +++ b/Tests/Support/SimulatedNetwork.h @@ -0,0 +1,141 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * An in-process network for hermetic RakPeer tests: no sockets, no threads, + * no wall clock. + * + * Peers created here get their sockets from this network (RakPeer's socket + * factory seam), run without internal threads (RakPeer::SetManualUpdate), and + * read a virtual clock this network installs. Step() advances that clock, + * delivers the datagrams that are due, runs every peer's update cycle and + * collects what each peer's Receive() returns. A whole connect / transfer / + * timeout scenario therefore runs in milliseconds and gives the same result on + * every run, so it belongs in the unit suite. + * + * Per-direction link rules model loss, latency, jitter (and so reordering), a + * path MTU, or a link that has gone dark. Randomness comes from a seeded LCG. + * + * Only one SimulatedNetwork may exist at a time: the clock override is process + * wide. Do not run a real (threaded, loopback) peer while one exists. + */ + +#pragma once + +#include +#include +#include +#include +#include + +#include "mafianet/get_time.h" +#include "mafianet/peer.h" +#include "mafianet/socket2.h" +#include "mafianet/types.h" + +namespace MafiaNet +{ + class SimulatedNetwork : public RakNetSocket2Factory + { + public: + struct LinkRule + { + int lossPercent = 0; + uint32_t delayMs = 1; + uint32_t jitterMs = 0; + int maxDatagramBytes = MAXIMUM_MTU_SIZE; // larger datagrams are dropped + bool dark = false; // drop everything + }; + + // A message a peer's Receive() returned, copied so it outlives the Packet. + struct Message + { + MessageID id; + std::vector data; + SystemAddress systemAddress; + RakNetGUID guid; + }; + + explicit SimulatedNetwork(uint32_t seed = 1); + ~SimulatedNetwork() override; + + SimulatedNetwork(const SimulatedNetwork &) = delete; + SimulatedNetwork &operator=(const SimulatedNetwork &) = delete; + + // A started peer bound to 10.0.0.: (port 0 picks one). + RakPeer *AddPeer(unsigned int maxConnections, unsigned short maxIncoming = 0, unsigned short port = 0); + SystemAddress AddressOf(const RakPeer *peer) const; + + // Rules apply per direction; unset links use the default rule. + void SetDefaultLink(const LinkRule &rule) { defaultRule = rule; } + void SetLink(const RakPeer *from, const RakPeer *to, const LinkRule &rule); + void SetLinkBothWays(const RakPeer *a, const RakPeer *b, const LinkRule &rule); + + // Advance the clock by ms in 1ms ticks: deliver due datagrams, run every + // peer's update, collect received messages. + void Step(uint32_t ms = 10); + + // Step until pred() holds or maxSimMs of simulated time passed. + bool RunUntil(const std::function &pred, uint32_t maxSimMs, uint32_t stepMs = 10); + + std::vector &Inbox(const RakPeer *peer); + // First message with this id in peer's inbox, or null. + const Message *Find(const RakPeer *peer, MessageID id) const; + bool Received(const RakPeer *peer, MessageID id) const { return Find(peer, id) != nullptr; } + // Wait until peer has received a message with this id. + bool RunUntilReceived(const RakPeer *peer, MessageID id, uint32_t maxSimMs); + // Client connects to server and both see the connection. + bool Connect(RakPeer *client, RakPeer *server, uint32_t maxSimMs = 5000); + + // Deliver raw bytes to a peer as if `from` had sent them, bypassing link + // rules: for fuzzing what an unconnected remote can send. + void InjectDatagram(const SystemAddress &from, const RakPeer *to, const unsigned char *data, int length); + // Called with every datagram any peer sends (before link rules). + void SetTap(std::function tap) + { + this->tap = std::move(tap); + } + + TimeUS Now() const; + uint64_t DatagramsSent() const { return datagramsSent; } + uint64_t DatagramsDropped() const { return datagramsDropped; } + + // RakNetSocket2Factory + RakNetSocket2 *Create(const SocketDescriptor &descriptor, unsigned int index) override; + void Destroy(RakNetSocket2 *socket) override; + + private: + class SimSocket; + friend class SimSocket; + + struct InFlight + { + TimeUS deliverAt; + uint64_t sequence; + SystemAddress from; + SystemAddress to; + std::vector bytes; + }; + + void Enqueue(const SystemAddress &from, const SystemAddress &to, const char *data, int length); + void DeliverDue(); + const LinkRule &RuleFor(const SystemAddress &from, const SystemAddress &to) const; + uint32_t NextRandom(); + static TimeUS VirtualClock(); + + std::vector peers; + std::map > inboxes; + std::map sockets; + std::map, LinkRule> links; + LinkRule defaultRule; + std::vector inFlight; + uint64_t nextSequence = 0; + uint64_t datagramsSent = 0; + uint64_t datagramsDropped = 0; + uint32_t lcgState; + std::function tap; + unsigned short nextHost = 1; + }; +} diff --git a/Tests/Unit/SimulatedPeerTests.cpp b/Tests/Unit/SimulatedPeerTests.cpp new file mode 100644 index 000000000..a451e1ed2 --- /dev/null +++ b/Tests/Unit/SimulatedPeerTests.cpp @@ -0,0 +1,202 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * Whole RakPeer instances over the in-process SimulatedNetwork: real + * handshakes, reliability, MTU discovery and timeouts, but no sockets, no + * threads and a virtual clock. Every scenario is deterministic and runs in + * milliseconds, so it lives in the unit suite. See Tests/Support/SimulatedNetwork.h. + */ + +#include + +#include +#include + +#include "SimulatedNetwork.h" + +#include "mafianet/bit_stream.h" +#include "mafianet/message_identifiers.h" +#include "mafianet/mtu_size.h" + +using namespace MafiaNet; + +namespace +{ + const MessageID ID_TEST_PAYLOAD = ID_USER_PACKET_ENUM + 7; + + void SendNumbered(RakPeer *from, const SystemAddress &to, uint32_t number, size_t payloadBytes, + Reliability reliability = Reliability::ReliableOrdered) + { + BitStream bs; + bs.Write(ID_TEST_PAYLOAD); + bs.Write(number); + for (size_t i = 0; i < payloadBytes; ++i) + bs.Write((unsigned char) (number + i)); + ASSERT_NE(0u, from->Send(&bs, Priority::High, reliability, 0, to, false)); + } + + std::vector NumbersReceived(SimulatedNetwork &net, const RakPeer *peer) + { + std::vector numbers; + for (const auto &message : net.Inbox(peer)) + { + if (message.id != ID_TEST_PAYLOAD) + continue; + BitStream bs(const_cast(message.data.data()), (unsigned int) message.data.size(), false); + bs.IgnoreBytes(1); + uint32_t number = 0; + bs.Read(number); + numbers.push_back(number); + } + return numbers; + } +} + +TEST(SimulatedPeer, ConnectHandshakeCompletes) +{ + SimulatedNetwork net; + RakPeer *server = net.AddPeer(4, 4); + RakPeer *client = net.AddPeer(1); + + ASSERT_TRUE(net.Connect(client, server, 2000)); + EXPECT_EQ(IS_CONNECTED, client->GetConnectionState(net.AddressOf(server))); + EXPECT_EQ(IS_CONNECTED, server->GetConnectionState(net.AddressOf(client))); + EXPECT_EQ(server->GetMyGUID(), net.Find(client, ID_CONNECTION_REQUEST_ACCEPTED)->guid); + EXPECT_EQ(client->GetMyGUID(), net.Find(server, ID_NEW_INCOMING_CONNECTION)->guid); + EXPECT_NE(client->GetMyGUID(), server->GetMyGUID()); +} + +TEST(SimulatedPeer, ReliableOrderedSurvivesLossAndReordering) +{ + SimulatedNetwork net(42); + RakPeer *server = net.AddPeer(1, 1); + RakPeer *client = net.AddPeer(1); + ASSERT_TRUE(net.Connect(client, server)); + + SimulatedNetwork::LinkRule lossy; + lossy.lossPercent = 20; + lossy.delayMs = 15; + lossy.jitterMs = 40; // enough to reorder datagrams + net.SetLinkBothWays(client, server, lossy); + + const uint32_t kMessages = 300; + for (uint32_t i = 0; i < kMessages; ++i) + SendNumbered(client, net.AddressOf(server), i, 40); + + ASSERT_TRUE(net.RunUntil([&] { return NumbersReceived(net, server).size() == kMessages; }, 60000)); + const std::vector numbers = NumbersReceived(net, server); + for (uint32_t i = 0; i < kMessages; ++i) + ASSERT_EQ(i, numbers[i]) << "out of order at " << i; + EXPECT_GT(net.DatagramsDropped(), 0u); +} + +TEST(SimulatedPeer, LargeMessageIsSplitAndReassembled) +{ + SimulatedNetwork net; + RakPeer *server = net.AddPeer(1, 1); + RakPeer *client = net.AddPeer(1); + ASSERT_TRUE(net.Connect(client, server)); + + SendNumbered(client, net.AddressOf(server), 99, 50000); + ASSERT_TRUE(net.RunUntilReceived(server, ID_TEST_PAYLOAD, 10000)); + const SimulatedNetwork::Message *message = net.Find(server, ID_TEST_PAYLOAD); + ASSERT_EQ(1u + 4u + 50000u, message->data.size()); + for (size_t i = 0; i < 50000; ++i) + ASSERT_EQ((unsigned char) (99 + i), message->data[5 + i]) << "at byte " << i; +} + +TEST(SimulatedPeer, DisconnectNotifiesTheOtherSide) +{ + SimulatedNetwork net; + RakPeer *server = net.AddPeer(1, 1); + RakPeer *client = net.AddPeer(1); + ASSERT_TRUE(net.Connect(client, server)); + + client->CloseConnection(net.AddressOf(server), true); + ASSERT_TRUE(net.RunUntilReceived(server, ID_DISCONNECTION_NOTIFICATION, 2000)); + const ConnectionState state = server->GetConnectionState(net.AddressOf(client)); + EXPECT_TRUE(state == IS_DISCONNECTED || state == IS_NOT_CONNECTED) << "state " << state; +} + +TEST(SimulatedPeer, ConnectionTimesOutWhenTheLinkGoesDark) +{ + SimulatedNetwork net; + RakPeer *server = net.AddPeer(1, 1); + RakPeer *client = net.AddPeer(1); + server->SetTimeoutTime(2000, UNASSIGNED_SYSTEM_ADDRESS); + client->SetTimeoutTime(2000, UNASSIGNED_SYSTEM_ADDRESS); + ASSERT_TRUE(net.Connect(client, server)); + + SimulatedNetwork::LinkRule dark; + dark.dark = true; + net.SetLinkBothWays(client, server, dark); + // Reliable traffic gives the timeout something to detect. + SendNumbered(client, net.AddressOf(server), 1, 8); + + const TimeUS wentDark = net.Now(); + ASSERT_TRUE(net.RunUntilReceived(client, ID_CONNECTION_LOST, 20000)); + const TimeUS lostAfterMs = (net.Now() - wentDark) / 1000; + EXPECT_GE(lostAfterMs, 2000u); + EXPECT_LE(lostAfterMs, 6000u); + ASSERT_TRUE(net.RunUntilReceived(server, ID_CONNECTION_LOST, 20000)); +} + +TEST(SimulatedPeer, MtuDiscoveryStopsBelowAPathCeiling) +{ + SimulatedNetwork net; + RakPeer *server = net.AddPeer(1, 1); + RakPeer *client = net.AddPeer(1); + + SimulatedNetwork::LinkRule narrow; + narrow.maxDatagramBytes = 1200; + net.SetLinkBothWays(client, server, narrow); + ASSERT_TRUE(net.Connect(client, server, 20000)); + + EXPECT_LE(client->GetMTUSize(net.AddressOf(server)), 1200 + UDP_HEADER_SIZE); + SendNumbered(client, net.AddressOf(server), 5, 10 * 1200); + EXPECT_TRUE(net.RunUntilReceived(server, ID_TEST_PAYLOAD, 10000)); +} + +TEST(SimulatedPeer, ServerRefusesConnectionsBeyondItsLimit) +{ + SimulatedNetwork net; + RakPeer *server = net.AddPeer(2, 2); + RakPeer *first = net.AddPeer(1); + RakPeer *second = net.AddPeer(1); + RakPeer *third = net.AddPeer(1); + ASSERT_TRUE(net.Connect(first, server)); + ASSERT_TRUE(net.Connect(second, server)); + + const SystemAddress target = net.AddressOf(server); + char host[64]; + target.ToString(false, host, sizeof(host)); + ASSERT_EQ(CONNECTION_ATTEMPT_STARTED, third->Connect(host, target.GetPort(), nullptr, 0)); + EXPECT_TRUE(net.RunUntilReceived(third, ID_NO_FREE_INCOMING_CONNECTIONS, 5000)); +} + +TEST(SimulatedPeer, SameSeedGivesTheSameTraffic) +{ + // Determinism is the point of the harness: identical runs, identical wire. + uint64_t sent[2], dropped[2]; + for (int run = 0; run < 2; ++run) + { + SimulatedNetwork net(7); + RakPeer *server = net.AddPeer(1, 1); + RakPeer *client = net.AddPeer(1); + ASSERT_TRUE(net.Connect(client, server)); + SimulatedNetwork::LinkRule lossy; + lossy.lossPercent = 30; + lossy.jitterMs = 20; + net.SetLinkBothWays(client, server, lossy); + for (uint32_t i = 0; i < 50; ++i) + SendNumbered(client, net.AddressOf(server), i, 100); + net.Step(5000); + sent[run] = net.DatagramsSent(); + dropped[run] = net.DatagramsDropped(); + } + EXPECT_EQ(sent[0], sent[1]); + EXPECT_EQ(dropped[0], dropped[1]); +} From fd4eff89e51cac0cd1a1e26f211f6eaa9d523663 Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:29:30 +0200 Subject: [PATCH 07/12] test: fuzz a whole server peer with arbitrary datagrams peer_datagram drives a RakPeer server on the simulated network with datagrams from a remote that need not follow the protocol: offline messages, the handshake and, from the recorded session seed, everything a connected peer can send. Each input is a fresh, deterministic run. --- Tests/Fuzz/CMakeLists.txt | 11 +++- Tests/Fuzz/corpus/peer_datagram/handshake | Bin 0 -> 1631 bytes Tests/Fuzz/corpus/peer_datagram/offline_ping | Bin 0 -> 35 bytes Tests/Fuzz/corpus/peer_datagram/session | Bin 0 -> 5806 bytes Tests/Fuzz/peer_datagram_fuzz.cpp | 51 +++++++++++++++++++ 5 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 Tests/Fuzz/corpus/peer_datagram/handshake create mode 100644 Tests/Fuzz/corpus/peer_datagram/offline_ping create mode 100644 Tests/Fuzz/corpus/peer_datagram/session create mode 100644 Tests/Fuzz/peer_datagram_fuzz.cpp diff --git a/Tests/Fuzz/CMakeLists.txt b/Tests/Fuzz/CMakeLists.txt index 0eeded167..9a819f6cf 100644 --- a/Tests/Fuzz/CMakeLists.txt +++ b/Tests/Fuzz/CMakeLists.txt @@ -14,6 +14,11 @@ file(GLOB MAFIANET_FUZZ_HARNESSES CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/*_fuzz.cpp") +# Harnesses that drive whole peers use the simulated network from Tests/Support. +set(MAFIANET_FUZZ_SUPPORT + "${CMAKE_CURRENT_SOURCE_DIR}/../Support/SimulatedNetwork.cpp" +) + if(MAFIANET_BUILD_FUZZERS AND NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang") message(FATAL_ERROR "MAFIANET_BUILD_FUZZERS needs Clang (libFuzzer); compiler is ${CMAKE_CXX_COMPILER_ID}") endif() @@ -24,7 +29,8 @@ foreach(harness_source IN LISTS MAFIANET_FUZZ_HARNESSES) set(corpus_dir "${CMAKE_CURRENT_SOURCE_DIR}/corpus/${corpus_name}") if(MAFIANET_BUILD_TESTS) - add_executable(${corpus_name}_replay "${harness_source}" replay_main.cpp) + add_executable(${corpus_name}_replay "${harness_source}" replay_main.cpp ${MAFIANET_FUZZ_SUPPORT}) + target_include_directories(${corpus_name}_replay PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/../Support") target_link_libraries(${corpus_name}_replay PRIVATE MafiaNet::MafiaNetStatic) target_compile_definitions(${corpus_name}_replay PRIVATE $<$:_DEBUG>) set_target_properties(${corpus_name}_replay PROPERTIES FOLDER "Tests/Fuzz") @@ -34,7 +40,8 @@ foreach(harness_source IN LISTS MAFIANET_FUZZ_HARNESSES) endif() if(MAFIANET_BUILD_FUZZERS) - add_executable(${harness} "${harness_source}") + add_executable(${harness} "${harness_source}" ${MAFIANET_FUZZ_SUPPORT}) + target_include_directories(${harness} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/../Support") target_link_libraries(${harness} PRIVATE MafiaNet::MafiaNetStatic) target_compile_options(${harness} PRIVATE -fsanitize=fuzzer) target_link_options(${harness} PRIVATE -fsanitize=fuzzer) diff --git a/Tests/Fuzz/corpus/peer_datagram/handshake b/Tests/Fuzz/corpus/peer_datagram/handshake new file mode 100644 index 0000000000000000000000000000000000000000..c5270a9e22a98d8e2510a69027e215bfb2e87ab5 GIT binary patch literal 1631 zcma!HWo7vPpWz=6`~?CblduYQhEXsY0;3@?8UmvsKw$_dF|ebTt}I{w|Nr+Yfwe-t zOMgp)rl+C|LkrLr2Zjkik`pEfv6uNRLo-7QBLhPM(+4040>V)JAnNa{1eX6mz`#H* z1u>KPJ(T{Kz`()8zyPwUfq}sTBFe!0QHP-gD9pgn!3+^%{tTtRViV(HH~=;RL;wJl C^IvWN literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/corpus/peer_datagram/offline_ping b/Tests/Fuzz/corpus/peer_datagram/offline_ping new file mode 100644 index 0000000000000000000000000000000000000000..51001465bc529c32b96d98a43a1b7c9949a0a482 GIT binary patch literal 35 kcmY#nU}OLT<`)eA|1_}RaDi~H8i!fb#(Rg4GfKpO-#+qEiA39ZEWrA z9UPsUU0mJVJv_aQ&Q8?GcvQXb8_?Y3kr*h zOG?YiD=Mq1YijH28ycIMTUy)NJ370%dwTo&Crq3)dCJsj(`U?_HG9t7dGi-6T(o$} z(q+q6tX#Ev&DwSAH*DOrdCS&q+js2TwR_Lreftj_JaqWT(PPI?oIG{<%-M72FI>EI z`O4L6*Kgdsb^FfUd-oqaeDwIq(`V0LynOZg&D(eHKYaZ3`ODXD-+%o4_507?fB#4Q zKN|m|>3=l;lTrSuGW4(lD-Q+@APFi_K}9Wd1A_(|mUb}dG`R-;`@0$po< Th8AGq2^^H*1P&1ZOHok(xQQB3 literal 0 HcmV?d00001 diff --git a/Tests/Fuzz/peer_datagram_fuzz.cpp b/Tests/Fuzz/peer_datagram_fuzz.cpp new file mode 100644 index 000000000..4127895e1 --- /dev/null +++ b/Tests/Fuzz/peer_datagram_fuzz.cpp @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + * + * Fuzzes a whole RakPeer server with datagrams from a remote that need not + * follow the protocol: offline messages (ping, open-connection requests), + * the connection handshake, and -- once the input replays a handshake, as + * the seed corpus does -- everything a connected peer can send. + * + * Runs on the SimulatedNetwork (no sockets, no threads, virtual clock), so + * each input is a deterministic, self-contained run. Input is a sequence of + * datagrams, each prefixed by a 16-bit little-endian length, all sent from + * 10.0.0.2:60000 to the server at 10.0.0.1:60000 (the addresses the seeds + * were recorded with). + */ + +#include +#include + +#include "SimulatedNetwork.h" + +#include "mafianet/mtu_size.h" + +using namespace MafiaNet; + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + SimulatedNetwork net(1); + RakPeer *server = net.AddPeer(4, 4); + const SystemAddress remote("10.0.0.2", 60000); + + size_t offset = 0; + while (offset + 2 <= size) + { + size_t length = (size_t) data[offset] | ((size_t) data[offset + 1] << 8); + offset += 2; + if (length > size - offset) + length = size - offset; + if (length > MAXIMUM_MTU_SIZE) + length = MAXIMUM_MTU_SIZE; + if (length > 0) + net.InjectDatagram(remote, server, data + offset, (int) length); + offset += length; + net.Step(10); + } + // Let timers run: resends, timeouts, connection cleanup. + net.Step(200); + return 0; +} From 5449399dd250d90dc777060a749d195c7a56bf75 Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:35:27 +0200 Subject: [PATCH 08/12] test: run the integration suite in parallel on OS-assigned ports Every integration test now binds ports the OS assigns (TestHelpers::StartOnLoopback / BoundPort; StandardServerPrep included), so RUN_SERIAL is gone and the suite runs under ctest -j: 74 tests in ~40 s locally with no retries, against several minutes serially. A taken port can no longer fail a test either (ComprehensiveConvert failed on a macOS runner that way, with Connect() returning INVALID_PARAMETER after an unchecked Startup). Every Startup() result is now asserted. The file-transfer tests also got per-run temp directories, which two parallel runs would otherwise share. Tests that check a peer's own addresses (LocalIsConnected, SystemAddressAndGuid) still bind all interfaces, on port 0. PeerBuilder's port-clash case gets a free port from the OS and then clashes on it. --- Tests/CMakeLists.txt | 10 ++++-- .../Integration/ComprehensiveConvertTests.cpp | 33 ++++++++++++------- Tests/Integration/ConnectWithSocketTests.cpp | 8 +++-- .../CrossConnectionConvertTests.cpp | 16 ++++----- ...rectoryDeltaTransferStreamingLiveTests.cpp | 9 +++-- Tests/Integration/DisconnectReasonTests.cpp | 6 +--- .../DroppedConnectionConvertTests.cpp | 12 +++---- Tests/Integration/EightPeerTests.cpp | 9 +++-- .../FileListTransferWindowLiveTests.cpp | 16 +++++---- Tests/Integration/LocalIsConnectedTests.cpp | 12 ++++--- .../ManyClientsOneServerBlockingTests.cpp | 20 +++++------ ...lientsOneServerDeallocateBlockingTests.cpp | 21 ++++++------ .../ManyClientsOneServerNonBlockingTests.cpp | 18 +++++----- Tests/Integration/MaximumConnectTests.cpp | 9 +++-- Tests/Integration/MiscellaneousTestsTests.cpp | 4 ++- .../OfflineMessagesConvertTests.cpp | 13 ++++---- Tests/Integration/PacketAndLowLevelTests.cpp | 8 ++--- Tests/Integration/PeerBuilderTests.cpp | 31 ++++++++--------- .../PeerConnectDisconnectTests.cpp | 31 +++++++++-------- ...onnectDisconnectWithCancelPendingTests.cpp | 17 ++++++---- Tests/Integration/PeerHandleTests.cpp | 13 +++----- Tests/Integration/PingTestsTests.cpp | 4 +-- .../ReliableOrderedConvertedTests.cpp | 28 +++++++--------- Tests/Integration/SecurityFunctionsTests.cpp | 10 +++--- .../Integration/SystemAddressAndGuidTests.cpp | 13 +++++--- Tests/Integration/TypedSendLiveTests.cpp | 15 ++++----- Tests/Support/TestHelpers.cpp | 20 ++++++++--- Tests/Support/TestHelpers.h | 8 +++++ 28 files changed, 231 insertions(+), 183 deletions(-) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 47561cffa..cd06cbbbf 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -4,8 +4,10 @@ # UnitTests (Tests/Unit) label "unit" — hermetic and # deterministic: no loopback networking, no wall-clock deadlines. # IntegrationTests (Tests/Integration) label "integration" — real UDP over -# loopback. RUN_SERIAL (several tests bind fixed ports) with a per-test -# timeout; CI retries transient timing misses (ctest --repeat until-pass:3). +# loopback on OS-assigned ports, so it runs in parallel; per-test timeout; +# CI retries transient timing misses (ctest --repeat until-pass:3). +# FuzzCorpus.* (Tests/Fuzz) label "unit" — replays the +# checked-in fuzz corpus through each harness. # # Under ctest every TEST() runs in its own process, so no state (sockets, # peers, GUIDs) can bleed between tests — the historical source of CI flakes. @@ -86,8 +88,10 @@ target_compile_definitions(IntegrationTests PRIVATE $<$:_DEBUG>) set_target_properties(IntegrationTests PROPERTIES FOLDER "Tests") mafianet_enable_warnings(IntegrationTests) +# Every integration test binds OS-assigned ports (TestHelpers::StartOnLoopback) +# and its own temp directories, so the suite runs in parallel under ctest -j. gtest_discover_tests(IntegrationTests - PROPERTIES LABELS integration RUN_SERIAL TRUE TIMEOUT 600 + PROPERTIES LABELS integration TIMEOUT 600 DISCOVERY_TIMEOUT 60 ) diff --git a/Tests/Integration/ComprehensiveConvertTests.cpp b/Tests/Integration/ComprehensiveConvertTests.cpp index dd2fc40ca..0b45bbfad 100644 --- a/Tests/Integration/ComprehensiveConvertTests.cpp +++ b/Tests/Integration/ComprehensiveConvertTests.cpp @@ -15,6 +15,7 @@ #include "mafianet/statistics.h" #include "CommonFunctions.h" +#include "TestHelpers.h" #include // For getenv #include // For strlen @@ -52,6 +53,7 @@ class ComprehensiveConvert : public ::testing::Test } RakPeerInterface *peers[NUM_PEERS]; + unsigned short ports[NUM_PEERS] = {}; // OS-assigned port each peer is bound to }; /* @@ -92,8 +94,11 @@ TEST_F(ComprehensiveConvert, DoesALittleBitOfEverything) peers[i]=RakPeerInterface::GetInstance(); peers[i]->SetMaximumIncomingConnections(CONNECTIONS_PER_SYSTEM); - SocketDescriptor socketDescriptor(60000+i, 0); - peers[i]->Startup(NUM_PEERS, &socketDescriptor, 1); + SocketDescriptor socketDescriptor(0, "127.0.0.1"); + StartupResult startResult = peers[i]->Startup(NUM_PEERS, &socketDescriptor, 1); + ASSERT_EQ(startResult, RAKNET_STARTED) << "Peer " << i << " failed to start (error " << startResult << ")"; + ports[i] = TestHelpers::BoundPort(peers[i]); + ASSERT_NE(ports[i], 0) << "Peer " << i << " has no bound port"; peers[i]->SetOfflinePingResponse("Offline Ping Data", (int)strlen("Offline Ping Data")+1); } @@ -104,10 +109,10 @@ TEST_F(ComprehensiveConvert, DoesALittleBitOfEverything) portAdd=randomMT()%NUM_PEERS; currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000+portAdd); + currentSystem.SetPortHostOrder(ports[portAdd]); if(!CommonFunctions::ConnectionStateMatchesOptions (peers[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { - ConnectionAttemptResult resultReturn = peers[i]->Connect("127.0.0.1", 60000+portAdd, 0, 0); + ConnectionAttemptResult resultReturn = peers[i]->Connect("127.0.0.1", ports[portAdd], 0, 0); ASSERT_TRUE(resultReturn==CONNECTION_ATTEMPT_STARTED || resultReturn==ALREADY_CONNECTED_TO_ENDPOINT) << "Connect function failed: Problem while calling connect (result " << resultReturn << ")"; @@ -124,17 +129,23 @@ TEST_F(ComprehensiveConvert, DoesALittleBitOfEverything) { // Initialize peerIndex=randomMT()%NUM_PEERS; - SocketDescriptor socketDescriptor(60000+peerIndex, 0); - peers[peerIndex]->Startup(NUM_PEERS, &socketDescriptor, 1); + // Peers are never shut down in this test, so this normally returns + // RAKNET_ALREADY_STARTED. Should a peer ever be down here, restart it on + // the port it was assigned before so the stored address stays valid. + SocketDescriptor socketDescriptor(ports[peerIndex], "127.0.0.1"); + StartupResult startResult = peers[peerIndex]->Startup(NUM_PEERS, &socketDescriptor, 1); + ASSERT_TRUE(startResult == RAKNET_STARTED || startResult == RAKNET_ALREADY_STARTED) + << "Peer " << peerIndex << " failed to (re)start on port " << ports[peerIndex] << " (error " << startResult << ")"; + ports[peerIndex] = TestHelpers::BoundPort(peers[peerIndex]); portAdd=randomMT()%NUM_PEERS; currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000+portAdd); + currentSystem.SetPortHostOrder(ports[portAdd]); if(!CommonFunctions::ConnectionStateMatchesOptions (peers[peerIndex],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { - ConnectionAttemptResult resultReturn = peers[peerIndex]->Connect("127.0.0.1", 60000+portAdd, 0, 0); + ConnectionAttemptResult resultReturn = peers[peerIndex]->Connect("127.0.0.1", ports[portAdd], 0, 0); ASSERT_TRUE(resultReturn==CONNECTION_ATTEMPT_STARTED || resultReturn==ALREADY_CONNECTED_TO_ENDPOINT) << "Connect function failed: Problem while calling connect (result " << resultReturn << ")"; @@ -147,10 +158,10 @@ TEST_F(ComprehensiveConvert, DoesALittleBitOfEverything) portAdd=randomMT()%NUM_PEERS; currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000+portAdd); + currentSystem.SetPortHostOrder(ports[portAdd]); if(!CommonFunctions::ConnectionStateMatchesOptions (peers[peerIndex],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { - ConnectionAttemptResult resultReturn = peers[peerIndex]->Connect("127.0.0.1", 60000+portAdd, 0, 0); + ConnectionAttemptResult resultReturn = peers[peerIndex]->Connect("127.0.0.1", ports[portAdd], 0, 0); ASSERT_TRUE(resultReturn==CONNECTION_ATTEMPT_STARTED || resultReturn==ALREADY_CONNECTED_TO_ENDPOINT) << "Connect function failed: Problem while calling connect (result " << resultReturn << ")"; } @@ -252,7 +263,7 @@ TEST_F(ComprehensiveConvert, DoesALittleBitOfEverything) { // Offline Ping peerIndex=randomMT()%NUM_PEERS; - peers[peerIndex]->Ping("127.0.0.1", 60000+(randomMT()%NUM_PEERS), (randomMT()%2)>0); + peers[peerIndex]->Ping("127.0.0.1", ports[randomMT()%NUM_PEERS], (randomMT()%2)>0); } else if (nextAction < .21f) { diff --git a/Tests/Integration/ConnectWithSocketTests.cpp b/Tests/Integration/ConnectWithSocketTests.cpp index a3807d2a6..63b4b7815 100644 --- a/Tests/Integration/ConnectWithSocketTests.cpp +++ b/Tests/Integration/ConnectWithSocketTests.cpp @@ -71,11 +71,13 @@ TEST_F(ConnectWithSocket, ConnectWithSocketFromGetSocketsAndGetSocket) DataStructures::List sockets; TestHelpers::StandardClientPrep(client,destroyList); TestHelpers::StandardServerPrep(server,destroyList); + const unsigned short serverPort=TestHelpers::BoundPort(server); + ASSERT_NE(serverPort,0) << "Server failed to start"; SystemAddress serverAddress; serverAddress.SetBinaryAddress("127.0.0.1"); - serverAddress.SetPortHostOrder(60000); + serverAddress.SetPortHostOrder(serverPort); char loopbackIp[]="127.0.0.1"; @@ -87,7 +89,7 @@ TEST_F(ConnectWithSocket, ConnectWithSocketFromGetSocketsAndGetSocket) ASSERT_TRUE(TestHelpers::WaitForTestPacket(server,5000)) << "Control test send didn't work"; // Disconnecting client - CommonFunctions::DisconnectAndWait(client,loopbackIp,60000); + CommonFunctions::DisconnectAndWait(client,loopbackIp,serverPort); RakNetSocket2 *theSocket; @@ -117,7 +119,7 @@ TEST_F(ConnectWithSocket, ConnectWithSocketFromGetSocketsAndGetSocket) ASSERT_TRUE(TestHelpers::WaitForTestPacket(server,5000)) << "Server did not recieve test packet from client"; // Disconnecting client - CommonFunctions::DisconnectAndWait(client,loopbackIp,60000); + CommonFunctions::DisconnectAndWait(client,loopbackIp,serverPort); // Testing ConnectWithSocket using socket from GetSocket theSocket=client->GetSocket(UNASSIGNED_SYSTEM_ADDRESS);//Get open Socket diff --git a/Tests/Integration/CrossConnectionConvertTests.cpp b/Tests/Integration/CrossConnectionConvertTests.cpp index e77a39040..1efc69b55 100644 --- a/Tests/Integration/CrossConnectionConvertTests.cpp +++ b/Tests/Integration/CrossConnectionConvertTests.cpp @@ -58,7 +58,6 @@ TEST_F(CrossConnectionConvert, SimultaneousConnectHandshakes) const bool isCI = (getenv("CI") != nullptr); const TimeMS testDurationMs = isCI ? 2000 : 10000; // 2 seconds in CI - static const unsigned short SERVER_PORT=1234; char serverIP[64]; strcpy(serverIP,"127.0.0.1"); @@ -70,14 +69,15 @@ TEST_F(CrossConnectionConvert, SimultaneousConnectHandshakes) server = RegisterPeer(RakPeerInterface::GetInstance()); client = RegisterPeer(RakPeerInterface::GetInstance()); - SocketDescriptor serverSd(SERVER_PORT, 0); - server->Startup(1, &serverSd, 1); + SocketDescriptor serverSd(0, "127.0.0.1"); + ASSERT_EQ(server->Startup(1, &serverSd, 1), RAKNET_STARTED) << "Server failed to start"; server->SetMaximumIncomingConnections(1); + const unsigned short serverPort=server->GetInternalID().GetPort(); SocketDescriptor clientSd(0, 0); - client->Startup(1, &clientSd, 1); + ASSERT_EQ(client->Startup(1, &clientSd, 1), RAKNET_STARTED) << "Client failed to start"; - client->Ping(serverIP,SERVER_PORT,false); + client->Ping(serverIP,serverPort,false); // PacketLogger pl; // pl.LogHeader(); @@ -163,7 +163,7 @@ TEST_F(CrossConnectionConvert, SimultaneousConnectHandshakes) connectionAttemptTime=0; server->Connect(clientIP,clientPort,0,0); - client->Connect(serverIP,SERVER_PORT,0,0); + client->Connect(serverIP,serverPort,0,0); connectionResultDeterminationTime=GetTimeMS()+2000; } @@ -174,7 +174,7 @@ TEST_F(CrossConnectionConvert, SimultaneousConnectHandshakes) SystemAddress sa; sa.SetBinaryAddress(serverIP); - sa.SetPortHostOrder(SERVER_PORT); + sa.SetPortHostOrder(serverPort); client->CancelConnectionAttempt(sa); sa.SetBinaryAddress(clientIP); @@ -190,7 +190,7 @@ TEST_F(CrossConnectionConvert, SimultaneousConnectHandshakes) } if (nextTestStartTime!=0 && GetTimeMS()>=nextTestStartTime) { - client->Ping(serverIP,SERVER_PORT,false); + client->Ping(serverIP,serverPort,false); nextTestStartTime=0; } RakSleep(0); diff --git a/Tests/Integration/DirectoryDeltaTransferStreamingLiveTests.cpp b/Tests/Integration/DirectoryDeltaTransferStreamingLiveTests.cpp index 20f8b6854..adb854e3c 100644 --- a/Tests/Integration/DirectoryDeltaTransferStreamingLiveTests.cpp +++ b/Tests/Integration/DirectoryDeltaTransferStreamingLiveTests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -96,8 +97,9 @@ namespace TEST(DirectoryDeltaTransferStreaming, LargeFilesAreWrittenChunkByChunk) { - const unsigned short port = 61211; - const std::filesystem::path root = std::filesystem::temp_directory_path() / "mafianet_ddt_streaming"; + // A per-run suffix keeps concurrent runs of this test from sharing a directory. + const std::filesystem::path root = std::filesystem::temp_directory_path() / + ("mafianet_ddt_streaming_" + std::to_string(std::random_device{}())); const std::filesystem::path source = root / "source"; const std::filesystem::path output = root / "output"; std::filesystem::remove_all(root); @@ -139,10 +141,11 @@ TEST(DirectoryDeltaTransferStreaming, LargeFilesAreWrittenChunkByChunk) const std::string outputDir = output.string() + "/"; clientDelta.SetApplicationDirectory(outputDir.c_str()); - SocketDescriptor serverSocket(port, "127.0.0.1"); + SocketDescriptor serverSocket(0, "127.0.0.1"); SocketDescriptor clientSocket(0, "127.0.0.1"); ASSERT_EQ(server->Startup(4, &serverSocket, 1), RAKNET_STARTED); ASSERT_EQ(client->Startup(1, &clientSocket, 1), RAKNET_STARTED); + const unsigned short port = server->GetInternalID().GetPort(); server->SetMaximumIncomingConnections(4); ASSERT_EQ(client->Connect("127.0.0.1", port, 0, 0), CONNECTION_ATTEMPT_STARTED); diff --git a/Tests/Integration/DisconnectReasonTests.cpp b/Tests/Integration/DisconnectReasonTests.cpp index 55245a55b..934ee9cd4 100644 --- a/Tests/Integration/DisconnectReasonTests.cpp +++ b/Tests/Integration/DisconnectReasonTests.cpp @@ -227,11 +227,7 @@ class DisconnectReason : public ::testing::Test server = RakPeerInterface::GetInstance(); client = RakPeerInterface::GetInstance(); - // Bind the server to an OS-assigned ephemeral port rather than a fixed one. - // Most tests in this suite share port 60000; running them all in one process - // (as CI does) means a prior test's socket can still be lingering on that port - // when this one starts, disrupting the fresh connection and intermittently - // swallowing the disconnect notification. An ephemeral port sidesteps that. + // Bind the server to an OS-assigned ephemeral port. SocketDescriptor sdServer(0, "127.0.0.1"); ASSERT_EQ(server->Startup(8, &sdServer, 1), RAKNET_STARTED) << "Server failed to start"; server->SetMaximumIncomingConnections(8); diff --git a/Tests/Integration/DroppedConnectionConvertTests.cpp b/Tests/Integration/DroppedConnectionConvertTests.cpp index 1c4bbef63..c6285ef21 100644 --- a/Tests/Integration/DroppedConnectionConvertTests.cpp +++ b/Tests/Integration/DroppedConnectionConvertTests.cpp @@ -77,22 +77,22 @@ TEST_F(DroppedConnectionConvert, LostConnectionsDetected) unsigned short numberOfSystems2; int sender; - // Used to refer to systems. We already know the IP - unsigned short serverPort = 20000; - serverID.FromStringExplicitPort("127.0.0.1", serverPort); - server = RegisterPeer(RakPeerInterface::GetInstance()); // server->InitializeSecurity(0,0,0,0); - SocketDescriptor socketDescriptor(serverPort,0); + SocketDescriptor socketDescriptor(0,"127.0.0.1"); StartupResult serverResult = server->Startup(NUMBER_OF_CLIENTS, &socketDescriptor, 1); ASSERT_EQ(serverResult, RAKNET_STARTED) << "Connect failed: Server failed to start (error " << serverResult << ")"; + + // Used to refer to systems. We already know the IP + const unsigned short serverPort = server->GetInternalID().GetPort(); + serverID.FromStringExplicitPort("127.0.0.1", serverPort); server->SetMaximumIncomingConnections(NUMBER_OF_CLIENTS); server->SetTimeoutTime(2000,UNASSIGNED_SYSTEM_ADDRESS); for (index=0; index < NUMBER_OF_CLIENTS; index++) { clients[index] = RegisterPeer(RakPeerInterface::GetInstance()); - SocketDescriptor socketDescriptor2(serverPort+1+index,0); + SocketDescriptor socketDescriptor2(0,0); StartupResult clientResult = clients[index]->Startup(1, &socketDescriptor2, 1); ASSERT_EQ(clientResult, RAKNET_STARTED) << "Connect failed: Client " << index << " failed to start (error " << clientResult << ")"; ASSERT_EQ(clients[index]->Connect("127.0.0.1", serverPort, 0, 0), CONNECTION_ATTEMPT_STARTED) << "Connect function failed."; diff --git a/Tests/Integration/EightPeerTests.cpp b/Tests/Integration/EightPeerTests.cpp index 0d8291442..10c7f08a0 100644 --- a/Tests/Integration/EightPeerTests.cpp +++ b/Tests/Integration/EightPeerTests.cpp @@ -14,6 +14,7 @@ #include "mafianet/sleep.h" #include "mafianet/time.h" #include "mafianet/get_time.h" +#include "TestHelpers.h" #include @@ -60,6 +61,7 @@ TEST_F(EightPeer, FullMeshReliableOrderedBroadcast) { const int peerNum= 8; RakPeerInterface *peerList[peerNum];//A list of 8 peers + unsigned short ports[peerNum];//OS-assigned port of each peer int connectionAmount[peerNum];//Counter for me to keep track of connection requests and accepts int recievedFromList[peerNum][peerNum];//Counter for me to keep track of packets received int lastNumberReceivedFromList[peerNum][peerNum];//Counter for me to keep track of last recieved sequence number @@ -80,9 +82,10 @@ TEST_F(EightPeer, FullMeshReliableOrderedBroadcast) lastNumberReceivedFromList[i][j]=0; } - SocketDescriptor sd(60000+i, 0); + SocketDescriptor sd(0, "127.0.0.1"); StartupResult result = peerList[i]->Startup(peerNum*2, &sd, 1); - ASSERT_EQ(result, RAKNET_STARTED) << "Peer " << i << " failed to start on port " << 60000+i << " (error " << result << ")"; + ASSERT_EQ(result, RAKNET_STARTED) << "Peer " << i << " failed to start (error " << result << ")"; + ports[i] = TestHelpers::BoundPort(peerList[i]); peerList[i]->SetMaximumIncomingConnections(peerNum); } @@ -95,7 +98,7 @@ TEST_F(EightPeer, FullMeshReliableOrderedBroadcast) { for (int j=i+1;jConnect("127.0.0.1", 60000+j, 0,0), CONNECTION_ATTEMPT_STARTED) << "Connect function returned failure. Problem while calling connect."; + ASSERT_EQ(peerList[i]->Connect("127.0.0.1", ports[j], 0,0), CONNECTION_ATTEMPT_STARTED) << "Connect function returned failure. Problem while calling connect."; } } diff --git a/Tests/Integration/FileListTransferWindowLiveTests.cpp b/Tests/Integration/FileListTransferWindowLiveTests.cpp index cd5c437ad..d418b3cba 100644 --- a/Tests/Integration/FileListTransferWindowLiveTests.cpp +++ b/Tests/Integration/FileListTransferWindowLiveTests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -98,9 +99,11 @@ namespace return bytes; } - void RunTransfer(unsigned int window, unsigned short port) + void RunTransfer(unsigned int window) { - const std::filesystem::path root = std::filesystem::temp_directory_path() / ("mafianet_flt_window_" + std::to_string(window)); + // A per-run suffix keeps concurrent runs of the same test from sharing a directory. + const std::filesystem::path root = std::filesystem::temp_directory_path() / + ("mafianet_flt_window_" + std::to_string(window) + "_" + std::to_string(std::random_device{}())); std::filesystem::remove_all(root); std::filesystem::create_directories(root); @@ -126,10 +129,11 @@ namespace serverTransfer.SetReferencePushWindow(window); EXPECT_EQ(serverTransfer.GetReferencePushWindow(), window); - SocketDescriptor serverSocket(port, "127.0.0.1"); + SocketDescriptor serverSocket(0, "127.0.0.1"); SocketDescriptor clientSocket(0, "127.0.0.1"); ASSERT_EQ(server->Startup(4, &serverSocket, 1), RAKNET_STARTED); ASSERT_EQ(client->Startup(1, &clientSocket, 1), RAKNET_STARTED); + const unsigned short port = server->GetInternalID().GetPort(); server->SetMaximumIncomingConnections(4); ASSERT_EQ(client->Connect("127.0.0.1", port, 0, 0), CONNECTION_ATTEMPT_STARTED); @@ -189,17 +193,17 @@ namespace TEST(FileListTransferWindow, UnwindowedTransferIsIntact) { - RunTransfer(1, 61201); + RunTransfer(1); } TEST(FileListTransferWindow, WindowOfFourDeliversTheSameBytes) { - RunTransfer(4, 61202); + RunTransfer(4); } TEST(FileListTransferWindow, WindowOfSixteenDeliversTheSameBytes) { - RunTransfer(16, 61203); + RunTransfer(16); } TEST(FileListTransferWindow, ZeroIsClampedToOne) diff --git a/Tests/Integration/LocalIsConnectedTests.cpp b/Tests/Integration/LocalIsConnectedTests.cpp index 883f331fe..8af7224c6 100644 --- a/Tests/Integration/LocalIsConnectedTests.cpp +++ b/Tests/Integration/LocalIsConnectedTests.cpp @@ -16,6 +16,7 @@ #include "mafianet/time.h" #include "mafianet/get_time.h" #include "CommonFunctions.h" +#include "TestHelpers.h" #include @@ -78,16 +79,17 @@ TEST_F(LocalIsConnected, ConnectionStateAndLocalAddressQueries) client=RakPeerInterface::GetInstance(); destroyList.Push(client,_FILE_AND_LINE_); + // The client keeps binding all interfaces (port 0): the GetLocalIP/IsLocalIP/ + // GetInternalID checks below are about its local addresses. SocketDescriptor clientSd; - client->Startup(1, &clientSd, 1); - SocketDescriptor serverSd(60000, 0); - server->Startup(1, &serverSd, 1); - server->SetMaximumIncomingConnections(1); + ASSERT_EQ(client->Startup(1, &clientSd, 1), RAKNET_STARTED); + const unsigned short serverPort = TestHelpers::StartOnLoopback(server, 1, 1); + ASSERT_NE(serverPort, 0) << "Server failed to start"; SystemAddress serverAddress; serverAddress.SetBinaryAddress("127.0.0.1"); - serverAddress.SetPortHostOrder(60000); + serverAddress.SetPortHostOrder(serverPort); TimeMS entryTime=GetTimeMS(); bool lastConnect=false; diff --git a/Tests/Integration/ManyClientsOneServerBlockingTests.cpp b/Tests/Integration/ManyClientsOneServerBlockingTests.cpp index 758153d1e..a6151800e 100644 --- a/Tests/Integration/ManyClientsOneServerBlockingTests.cpp +++ b/Tests/Integration/ManyClientsOneServerBlockingTests.cpp @@ -16,6 +16,7 @@ #include "mafianet/get_time.h" #include "CommonFunctions.h" +#include "TestHelpers.h" using namespace MafiaNet; @@ -43,7 +44,7 @@ class ManyClientsOneServerBlocking : public ::testing::Test { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); while (CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,false,true,true) ) { @@ -81,6 +82,7 @@ class ManyClientsOneServerBlocking : public ::testing::Test } DataStructures::List destroyList; + unsigned short serverPort = 0; }; /* @@ -137,10 +139,8 @@ TEST_F(ManyClientsOneServerBlocking, ClientsSurviveRepeatedDisconnectReconnectCy server=RakPeerInterface::GetInstance(); destroyList.Push(server,_FILE_AND_LINE_); - SocketDescriptor serverSd(60000, 0); - StartupResult serverResult = server->Startup(clientNum, &serverSd, 1); - ASSERT_EQ(serverResult, RAKNET_STARTED) << "Server failed to start (error " << serverResult << ")"; - server->SetMaximumIncomingConnections(clientNum); + serverPort = TestHelpers::StartOnLoopback(server, clientNum, clientNum); + ASSERT_NE(serverPort, 0) << "Server failed to start"; //Connect all the clients to the server @@ -148,7 +148,7 @@ TEST_F(ManyClientsOneServerBlocking, ClientsSurviveRepeatedDisconnectReconnectCy { //This fails the test, don't bother going on. - ASSERT_EQ(clientList[i]->Connect("127.0.0.1", 60000, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; + ASSERT_EQ(clientList[i]->Connect("127.0.0.1", serverPort, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; } @@ -185,12 +185,12 @@ TEST_F(ManyClientsOneServerBlocking, ClientsSurviveRepeatedDisconnectReconnectCy { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); if(!CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { //This fails the test, don't bother going on. - ASSERT_EQ(clientList[i]->Connect("127.0.0.1", 60000, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; + ASSERT_EQ(clientList[i]->Connect("127.0.0.1", serverPort, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; } } @@ -209,13 +209,13 @@ TEST_F(ManyClientsOneServerBlocking, ClientsSurviveRepeatedDisconnectReconnectCy { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); if(!CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { printf("Calling Connect() for client %i.\n",i); - if (clientList[i]->Connect("127.0.0.1", 60000, 0,0)!=CONNECTION_ATTEMPT_STARTED) + if (clientList[i]->Connect("127.0.0.1", serverPort, 0,0)!=CONNECTION_ATTEMPT_STARTED) { clientList[i]->GetSystemList(systemList,guidList);//Get connectionlist diff --git a/Tests/Integration/ManyClientsOneServerDeallocateBlockingTests.cpp b/Tests/Integration/ManyClientsOneServerDeallocateBlockingTests.cpp index 64db584c8..09fbfcb28 100644 --- a/Tests/Integration/ManyClientsOneServerDeallocateBlockingTests.cpp +++ b/Tests/Integration/ManyClientsOneServerDeallocateBlockingTests.cpp @@ -16,6 +16,7 @@ #include "mafianet/get_time.h" #include "CommonFunctions.h" +#include "TestHelpers.h" #include @@ -59,7 +60,7 @@ class ManyClientsOneServerDeallocateBlocking : public ::testing::Test { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); while (CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,false,true,true) ) { @@ -102,6 +103,7 @@ class ManyClientsOneServerDeallocateBlocking : public ::testing::Test static const int clientNum= 8; RakPeerInterface *clientList[clientNum];//A list of clients RakPeerInterface *server; + unsigned short serverPort = 0; }; /* @@ -192,10 +194,8 @@ TEST_F(ManyClientsOneServerDeallocateBlocking, ClientsReconnectAfterDeallocateAn } server=RakPeerInterface::GetInstance(); - SocketDescriptor serverSd(60000, 0); - StartupResult serverResult = server->Startup(clientNum, &serverSd, 1); - ASSERT_EQ(serverResult, RAKNET_STARTED) << "Server failed to start (error " << serverResult << ")"; - server->SetMaximumIncomingConnections(clientNum); + serverPort = TestHelpers::StartOnLoopback(server, clientNum, clientNum); + ASSERT_NE(serverPort, 0) << "Server failed to start"; const int timeoutTime=1000; server->SetTimeoutTime(timeoutTime,UNASSIGNED_SYSTEM_ADDRESS); @@ -209,7 +209,7 @@ TEST_F(ManyClientsOneServerDeallocateBlocking, ClientsReconnectAfterDeallocateAn { //This fails the test, don't bother going on. - ASSERT_EQ(clientList[i]->Connect("127.0.0.1", 60000, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed"; + ASSERT_EQ(clientList[i]->Connect("127.0.0.1", serverPort, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed"; } @@ -247,6 +247,7 @@ TEST_F(ManyClientsOneServerDeallocateBlocking, ClientsReconnectAfterDeallocateAn RakSleep(100); loopResult = clientList[i]->Startup(1, &clientSd2, 1); } + ASSERT_EQ(loopResult, RAKNET_STARTED) << "Client " << i << " failed to restart (error " << loopResult << ")"; } } @@ -259,12 +260,12 @@ TEST_F(ManyClientsOneServerDeallocateBlocking, ClientsReconnectAfterDeallocateAn { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); if(!CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { //This fails the test, don't bother going on. - ASSERT_EQ(clientList[i]->Connect("127.0.0.1", 60000, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed"; + ASSERT_EQ(clientList[i]->Connect("127.0.0.1", serverPort, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed"; } } @@ -285,13 +286,13 @@ TEST_F(ManyClientsOneServerDeallocateBlocking, ClientsReconnectAfterDeallocateAn { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); if(!CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { printf("Calling Connect() for client %i.\n",i); - if (clientList[i]->Connect("127.0.0.1", 60000, 0,0)!=CONNECTION_ATTEMPT_STARTED) + if (clientList[i]->Connect("127.0.0.1", serverPort, 0,0)!=CONNECTION_ATTEMPT_STARTED) { clientList[i]->GetSystemList(systemList,guidList);//Get connectionlist diff --git a/Tests/Integration/ManyClientsOneServerNonBlockingTests.cpp b/Tests/Integration/ManyClientsOneServerNonBlockingTests.cpp index da98d3c3e..625a620ff 100644 --- a/Tests/Integration/ManyClientsOneServerNonBlockingTests.cpp +++ b/Tests/Integration/ManyClientsOneServerNonBlockingTests.cpp @@ -16,6 +16,7 @@ #include "mafianet/get_time.h" #include "CommonFunctions.h" +#include "TestHelpers.h" using namespace MafiaNet; @@ -35,6 +36,7 @@ class ManyClientsOneServerNonBlocking : public ::testing::Test } DataStructures::List destroyList; + unsigned short serverPort = 0; }; /* @@ -91,10 +93,8 @@ TEST_F(ManyClientsOneServerNonBlocking, ClientsSurviveRapidDisconnectReconnectCy server=RakPeerInterface::GetInstance(); destroyList.Push(server,_FILE_AND_LINE_); - SocketDescriptor serverSd(60000, 0); - StartupResult serverResult = server->Startup(clientNum, &serverSd, 1); - ASSERT_EQ(serverResult, RAKNET_STARTED) << "Server failed to start (error " << serverResult << ")"; - server->SetMaximumIncomingConnections(clientNum); + serverPort = TestHelpers::StartOnLoopback(server, clientNum, clientNum); + ASSERT_NE(serverPort, 0) << "Server failed to start"; //Connect all the clients to the server @@ -102,7 +102,7 @@ TEST_F(ManyClientsOneServerNonBlocking, ClientsSurviveRapidDisconnectReconnectCy { //This fails the test, don't bother going on. - ASSERT_EQ(clientList[i]->Connect("127.0.0.1", 60000, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; + ASSERT_EQ(clientList[i]->Connect("127.0.0.1", serverPort, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; } @@ -139,12 +139,12 @@ TEST_F(ManyClientsOneServerNonBlocking, ClientsSurviveRapidDisconnectReconnectCy { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); if(!CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { //This fails the test, don't bother going on. - ASSERT_EQ(clientList[i]->Connect("127.0.0.1", 60000, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; + ASSERT_EQ(clientList[i]->Connect("127.0.0.1", serverPort, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed."; } } @@ -192,12 +192,12 @@ TEST_F(ManyClientsOneServerNonBlocking, ClientsSurviveRapidDisconnectReconnectCy { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000); + currentSystem.SetPortHostOrder(serverPort); if(!CommonFunctions::ConnectionStateMatchesOptions (clientList[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { - if (clientList[i]->Connect("127.0.0.1", 60000, 0,0)!=CONNECTION_ATTEMPT_STARTED) + if (clientList[i]->Connect("127.0.0.1", serverPort, 0,0)!=CONNECTION_ATTEMPT_STARTED) { clientList[i]->GetSystemList(systemList,guidList);//Get connectionlist diff --git a/Tests/Integration/MaximumConnectTests.cpp b/Tests/Integration/MaximumConnectTests.cpp index cba6b9e6a..b6a7e1d7f 100644 --- a/Tests/Integration/MaximumConnectTests.cpp +++ b/Tests/Integration/MaximumConnectTests.cpp @@ -14,6 +14,7 @@ #include "mafianet/sleep.h" #include "mafianet/time.h" #include "mafianet/get_time.h" +#include "TestHelpers.h" #include @@ -66,6 +67,7 @@ TEST_F(MaximumConnect, RefusesConnectionsBeyondMaximumIncoming) const int peerNum= 8; const int maxConnections=4;//Max allowed connections for test RakPeerInterface *peerList[peerNum];//A list of 8 peers + unsigned short ports[peerNum];//OS-assigned port of each peer Packet *packet; @@ -76,9 +78,10 @@ TEST_F(MaximumConnect, RefusesConnectionsBeyondMaximumIncoming) peerList[i]=RakPeerInterface::GetInstance(); destroyList.push_back(peerList[i]); - SocketDescriptor sd(60000+i, 0); + SocketDescriptor sd(0, "127.0.0.1"); StartupResult result = peerList[i]->Startup(maxConnections, &sd, 1); - ASSERT_EQ(result, RAKNET_STARTED) << "Peer " << i << " failed to start on port " << 60000+i << " (error " << result << ")"; + ASSERT_EQ(result, RAKNET_STARTED) << "Peer " << i << " failed to start (error " << result << ")"; + ports[i] = TestHelpers::BoundPort(peerList[i]); peerList[i]->SetMaximumIncomingConnections(maxConnections); connReturn=peerList[i]->GetMaximumIncomingConnections(); @@ -94,7 +97,7 @@ TEST_F(MaximumConnect, RefusesConnectionsBeyondMaximumIncoming) for (int j=i+1;jConnect("127.0.0.1", 60000+j, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; + ASSERT_EQ(peerList[i]->Connect("127.0.0.1", ports[j], 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; } diff --git a/Tests/Integration/MiscellaneousTestsTests.cpp b/Tests/Integration/MiscellaneousTestsTests.cpp index 0af55a118..c9350ade5 100644 --- a/Tests/Integration/MiscellaneousTestsTests.cpp +++ b/Tests/Integration/MiscellaneousTestsTests.cpp @@ -57,10 +57,12 @@ TEST_F(MiscellaneousTests, AdvertiseSystem) TestHelpers::StandardClientPrep(client, destroyList); TestHelpers::StandardServerPrep(server, destroyList); + const unsigned short serverPort = TestHelpers::BoundPort(server); + ASSERT_NE(serverPort, 0) << "Server failed to start"; printf("Testing AdvertiseSystem\n"); - client->AdvertiseSystem("127.0.0.1", 60000, 0, 0); + client->AdvertiseSystem("127.0.0.1", serverPort, 0, 0); ASSERT_TRUE(CommonFunctions::WaitForMessageWithID(server, ID_ADVERTISE_SYSTEM, 5000)) << "Did not recieve client advertise"; } diff --git a/Tests/Integration/OfflineMessagesConvertTests.cpp b/Tests/Integration/OfflineMessagesConvertTests.cpp index 06caad4b8..a44ee8382 100644 --- a/Tests/Integration/OfflineMessagesConvertTests.cpp +++ b/Tests/Integration/OfflineMessagesConvertTests.cpp @@ -76,11 +76,10 @@ TEST_F(OfflineMessagesConvert, AdvertiseSystemAndOfflinePing) nextTest = 0; - peer1->SetMaximumIncomingConnections(1); - SocketDescriptor socketDescriptor(60001, 0); - peer1->Startup(1, &socketDescriptor, 1); - socketDescriptor.port = 60002; - peer2->Startup(1, &socketDescriptor, 1); + const unsigned short peer1Port = TestHelpers::StartOnLoopback(peer1, 1, 1); + ASSERT_NE(peer1Port, 0) << "peer1 failed to start"; + const unsigned short peer2Port = TestHelpers::StartOnLoopback(peer2, 1); + ASSERT_NE(peer2Port, 0) << "peer2 failed to start"; char *pingResponseData = 0; unsigned int responseLen = 0; peer1->SetOfflinePingResponse("Offline Ping Data", (int) strlen("Offline Ping Data") + 1); @@ -91,7 +90,7 @@ TEST_F(OfflineMessagesConvert, AdvertiseSystemAndOfflinePing) // Wait for connection to complete RakSleep(300); - peer1->AdvertiseSystem("127.0.0.1", 60002, "hello world", (int) strlen("hello world") + 1); + peer1->AdvertiseSystem("127.0.0.1", peer2Port, "hello world", (int) strlen("hello world") + 1); TimeMS entryTime = GetTimeMS();//Loop entry time @@ -107,7 +106,7 @@ TEST_F(OfflineMessagesConvert, AdvertiseSystemAndOfflinePing) ASSERT_STREQ((const char *) (packet->data + 1), "hello world") << "Got Advertise system with unexpected data"; recievedProperOfflineData = true; - peer2->Ping("127.0.0.1", 60001, false); + peer2->Ping("127.0.0.1", peer1Port, false); nextTest++; } else if (packet->data[0] == ID_UNCONNECTED_PONG) diff --git a/Tests/Integration/PacketAndLowLevelTests.cpp b/Tests/Integration/PacketAndLowLevelTests.cpp index 6945bef76..20c1f5607 100644 --- a/Tests/Integration/PacketAndLowLevelTests.cpp +++ b/Tests/Integration/PacketAndLowLevelTests.cpp @@ -80,6 +80,8 @@ TEST_F(PacketAndLowLevelTests, LowLevelPacketFunctions) TestHelpers::StandardClientPrep(client, destroyList); TestHelpers::StandardServerPrep(server, destroyList); + ASSERT_TRUE(client->IsActive()) << "Client failed to start"; + ASSERT_NE(TestHelpers::BoundPort(server), 0) << "Server failed to start"; // Connecting to server ASSERT_TRUE(TestHelpers::WaitAndConnectTwoPeersLocally(client, server, 5000)) << "Client failed to connect to server"; @@ -132,11 +134,9 @@ TEST_F(PacketAndLowLevelTests, LowLevelPacketFunctions) pluginClient->AttachPlugin(myPlug); // safe: peer not started yet - SocketDescriptor pluginServerSd(60001, 0); - pluginServer->Startup(1, &pluginServerSd, 1); - pluginServer->SetMaximumIncomingConnections(1); + ASSERT_NE(TestHelpers::StartOnLoopback(pluginServer, 1, 1), 0) << "Plugin server failed to start"; SocketDescriptor pluginClientSd; - pluginClient->Startup(1, &pluginClientSd, 1); + ASSERT_EQ(pluginClient->Startup(1, &pluginClientSd, 1), RAKNET_STARTED); ASSERT_TRUE(TestHelpers::WaitAndConnectTwoPeersLocally(pluginClient, pluginServer, 5000)) << "Client failed to connect to server"; diff --git a/Tests/Integration/PeerBuilderTests.cpp b/Tests/Integration/PeerBuilderTests.cpp index 8a825a388..fd1397215 100644 --- a/Tests/Integration/PeerBuilderTests.cpp +++ b/Tests/Integration/PeerBuilderTests.cpp @@ -32,23 +32,21 @@ using namespace MafiaNet; /// an unresolvable target surfaces the ConnectionAttemptResult (PeerStage::Connect). /// - A client builder with no .connect() target still starts up (Startup only). -// A distinct port so a full-suite run does not collide with other tests. -static const unsigned short PEERBUILDER_TEST_PORT = 61016; - // Shared incoming password, exercised on both the server (SetIncomingPassword) // and the client (Connect password argument) sides. static const char *PEERBUILDER_PASSWORD = "hunter2"; // --- Server builder replaces the manual 3-call sequence; client connects. -// Then: startup failure surfaces the underlying StartupResult. Both phases -// bind PEERBUILDER_TEST_PORT, so they stay in one TEST (the first phase's -// peers must have freed the port before the clash phase reuses it). +// Then: startup failure surfaces the underlying StartupResult. Every server +// binds an OS-assigned port (.port(0)); the clash phase re-binds the port its +// holder was given. TEST(PeerBuilder, ServerClientConnectAndPortClash) { // --- 1. Server builder replaces the manual 3-call sequence; client connects --- { auto serverResult = Peer::server() - .port(PEERBUILDER_TEST_PORT) + .port(0) + .host("127.0.0.1") .max_connections(4) .incoming_password(PEERBUILDER_PASSWORD) .start(); @@ -58,10 +56,12 @@ TEST(PeerBuilder, ServerClientConnectAndPortClash) // The builder must have applied SetMaximumIncomingConnections, not just Startup. ASSERT_EQ(serverResult->get()->GetMaximumIncomingConnections(), 4) << "server incoming-connection cap not set by builder"; + const unsigned short serverPort = serverResult->get()->GetInternalID().GetPort(); + ASSERT_NE(serverPort, 0) << "server has no bound port"; auto clientResult = Peer::client() .max_connections(1) - .connect("127.0.0.1", PEERBUILDER_TEST_PORT, PEERBUILDER_PASSWORD) + .connect("127.0.0.1", serverPort, PEERBUILDER_PASSWORD) .start(); ASSERT_TRUE(clientResult) << "client builder start() failed: stage " << (int) clientResult.error().stage; @@ -96,21 +96,22 @@ TEST(PeerBuilder, ServerClientConnectAndPortClash) } ASSERT_TRUE(clientAccepted) << "client was not accepted within 10s"; ASSERT_TRUE(serverGotIncoming) << "server did not observe the incoming connection within 10s"; - // server/client are RAII Peers: they DestroyInstance here, freeing the - // port before the next sub-test reuses it. + // server/client are RAII Peers: they DestroyInstance here. } // --- 2. Startup failure surfaces the underlying StartupResult --- { - // Hold the port so the second server cannot bind it. + // Hold an OS-assigned port so the second server cannot bind it. auto holder = Peer::server() - .port(PEERBUILDER_TEST_PORT) + .port(0) .max_connections(1) .start(); ASSERT_TRUE(holder) << "port-holder server failed to start"; + const unsigned short heldPort = holder->get()->GetInternalID().GetPort(); + ASSERT_NE(heldPort, 0) << "port-holder server has no bound port"; auto clash = Peer::server() - .port(PEERBUILDER_TEST_PORT) + .port(heldPort) .max_connections(1) .start(); ASSERT_FALSE(clash) << "second server unexpectedly bound an in-use port"; @@ -127,10 +128,10 @@ TEST(PeerBuilder, ConnectFailureSurfacesConnectionAttemptResult) { // A syntactically invalid host fails address resolution synchronously // (getaddrinfo rejects it), so Connect returns CANNOT_RESOLVE_DOMAIN_NAME - // without any network round-trip. + // without any network round-trip, so the remote port is never used. auto badClient = Peer::client() .max_connections(1) - .connect("!!!not_a_valid_host!!!", PEERBUILDER_TEST_PORT) + .connect("!!!not_a_valid_host!!!", 1) .start(); ASSERT_FALSE(badClient) << "client unexpectedly connected to an invalid host"; ASSERT_EQ(badClient.error().stage, PeerStage::Connect) diff --git a/Tests/Integration/PeerConnectDisconnectTests.cpp b/Tests/Integration/PeerConnectDisconnectTests.cpp index 44f8c393b..8b3e52339 100644 --- a/Tests/Integration/PeerConnectDisconnectTests.cpp +++ b/Tests/Integration/PeerConnectDisconnectTests.cpp @@ -15,6 +15,7 @@ #include "mafianet/time.h" #include "mafianet/get_time.h" #include "CommonFunctions.h" +#include "TestHelpers.h" #include @@ -33,7 +34,7 @@ class PeerConnectDisconnect : public ::testing::Test RakPeerInterface::DestroyInstance(peer); } - static void WaitForConnectionRequestsToComplete(RakPeerInterface **peerList, int peerNum) + static void WaitForConnectionRequestsToComplete(RakPeerInterface **peerList, const unsigned short *ports, int peerNum) { SystemAddress currentSystem; bool msgWasPrinted=false; @@ -43,7 +44,7 @@ class PeerConnectDisconnect : public ::testing::Test for (int j=i+1;jStartup(maxConnections, &sd, 1); ASSERT_EQ(result, RAKNET_STARTED) << "Peer " << i << " failed to start (error " << result << ")"; + ports[i] = TestHelpers::BoundPort(peerList[i]); peerList[i]->SetMaximumIncomingConnections(maxConnections); } @@ -133,7 +136,7 @@ TEST_F(PeerConnectDisconnect, MeshReconnectsAfterRepeatedDisconnects) for (int j=i+1;jConnect("127.0.0.1", 60000+j, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; + ASSERT_EQ(peerList[i]->Connect("127.0.0.1", ports[j], 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; } @@ -175,22 +178,22 @@ TEST_F(PeerConnectDisconnect, MeshReconnectsAfterRepeatedDisconnects) { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000+j); + currentSystem.SetPortHostOrder(ports[j]); if(!CommonFunctions::ConnectionStateMatchesOptions (peerList[i],currentSystem,true,true,true,true) )//Are we connected or is there a pending operation ? { - ASSERT_EQ(peerList[i]->Connect("127.0.0.1", 60000+j, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; + ASSERT_EQ(peerList[i]->Connect("127.0.0.1", ports[j], 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; } } } - WaitAndPrintResults(peerList,peerNum); + WaitAndPrintResults(peerList,ports,peerNum); } - WaitAndPrintResults(peerList,peerNum); + WaitAndPrintResults(peerList,ports,peerNum); printf("Connecting peers\n"); @@ -213,10 +216,10 @@ TEST_F(PeerConnectDisconnect, MeshReconnectsAfterRepeatedDisconnects) for (int j=i+1;jConnect("127.0.0.1", 60000+j, 0,0); + ConnectionAttemptResult car=peerList[i]->Connect("127.0.0.1", ports[j], 0,0); // STARTED is success. ALREADY_CONNECTED_TO_ENDPOINT and // CONNECTION_ATTEMPT_ALREADY_IN_PROGRESS are benign races (the link came up // between the state check above and this call) and are expected while the @@ -226,13 +229,13 @@ TEST_F(PeerConnectDisconnect, MeshReconnectsAfterRepeatedDisconnects) ASSERT_TRUE(car==CONNECTION_ATTEMPT_STARTED || car==ALREADY_CONNECTED_TO_ENDPOINT || car==CONNECTION_ATTEMPT_ALREADY_IN_PROGRESS) - << "The connect function failed. Connect() from peer " << i << " to 127.0.0.1:" << 60000+j + << "The connect function failed. Connect() from peer " << i << " to 127.0.0.1:" << ports[j] << " (" << currentSystem.ToString() << ") failed with result " << (int)car << "."; } } } - WaitAndPrintResults(peerList,peerNum); + WaitAndPrintResults(peerList,ports,peerNum); allConnected = true; for (int i=0;i @@ -76,6 +77,7 @@ TEST_F(PeerConnectDisconnectWithCancelPending, MeshReconnectsWithCancelledPendin const int peerNum= 8; const int maxConnections=peerNum*3;//Max allowed connections for test set to times 3 to eliminate problem variables RakPeerInterface *peerList[peerNum];//A list of 8 peers + unsigned short ports[peerNum];//OS-assigned port of each peer SystemAddress currentSystem; @@ -87,9 +89,10 @@ TEST_F(PeerConnectDisconnectWithCancelPending, MeshReconnectsWithCancelledPendin peerList[i]=RakPeerInterface::GetInstance(); destroyList.push_back(peerList[i]); - SocketDescriptor sd(60000+i, 0); + SocketDescriptor sd(0, "127.0.0.1"); StartupResult result = peerList[i]->Startup(maxConnections, &sd, 1); ASSERT_EQ(result, RAKNET_STARTED) << "Peer " << i << " failed to start (error " << result << ")"; + ports[i] = TestHelpers::BoundPort(peerList[i]); peerList[i]->SetMaximumIncomingConnections(maxConnections); } @@ -102,7 +105,7 @@ TEST_F(PeerConnectDisconnectWithCancelPending, MeshReconnectsWithCancelledPendin for (int j=i+1;jConnect("127.0.0.1", 60000+j, 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; + ASSERT_EQ(peerList[i]->Connect("127.0.0.1", ports[j], 0,0), CONNECTION_ATTEMPT_STARTED) << "The connect function failed. Problem while calling connect."; } @@ -143,7 +146,7 @@ TEST_F(PeerConnectDisconnectWithCancelPending, MeshReconnectsWithCancelledPendin { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000+j); + currentSystem.SetPortHostOrder(ports[j]); peerList[i]->CancelConnectionAttempt(currentSystem); //Make sure a connection is not pending before trying to connect. @@ -160,11 +163,11 @@ TEST_F(PeerConnectDisconnectWithCancelPending, MeshReconnectsWithCancelledPendin for (int j=i+1;jConnect("127.0.0.1", 60000+j, 0,0)!=CONNECTION_ATTEMPT_STARTED) + if (peerList[i]->Connect("127.0.0.1", ports[j], 0,0)!=CONNECTION_ATTEMPT_STARTED) { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000+j); + currentSystem.SetPortHostOrder(ports[j]); peerList[i]->GetSystemList(systemList,guidList);//Get connectionlist @@ -224,11 +227,11 @@ TEST_F(PeerConnectDisconnectWithCancelPending, MeshReconnectsWithCancelledPendin { currentSystem.SetBinaryAddress("127.0.0.1"); - currentSystem.SetPortHostOrder(60000+j); + currentSystem.SetPortHostOrder(ports[j]); peerList[i]->CancelConnectionAttempt(currentSystem); //Make sure a connection is not pending before trying to connect. - if (peerList[i]->Connect("127.0.0.1", 60000+j, 0,0)!=CONNECTION_ATTEMPT_STARTED) + if (peerList[i]->Connect("127.0.0.1", ports[j], 0,0)!=CONNECTION_ATTEMPT_STARTED) { peerList[i]->GetSystemList(systemList,guidList);//Get connectionlist diff --git a/Tests/Integration/PeerHandleTests.cpp b/Tests/Integration/PeerHandleTests.cpp index f23ce700e..7d4451d0f 100644 --- a/Tests/Integration/PeerHandleTests.cpp +++ b/Tests/Integration/PeerHandleTests.cpp @@ -29,9 +29,6 @@ using namespace MafiaNet; /// - Peer::receive() yields an empty PacketPtr when nothing is queued, and the /// move-assignment in a receive loop frees each real packet without leaking. -// A distinct port so a full-suite run does not collide with other tests. -static const unsigned short PEERHANDLE_TEST_PORT = 61015; - // A user message id carried behind an ID_TIMESTAMP prefix, used to exercise the // timestamp-aware branch of PacketPtr::id(). static const unsigned char PEERHANDLE_TS_ID = (unsigned char) (ID_USER_PACKET_ENUM + 7); @@ -74,19 +71,19 @@ TEST(PeerHandle, LoopbackIdBranchesAndIncomingRange) Peer server; Peer client; - SocketDescriptor serverSd(PEERHANDLE_TEST_PORT, 0); - ASSERT_EQ(server->Startup(1, &serverSd, 1), RAKNET_STARTED) - << "server Startup failed (port " << PEERHANDLE_TEST_PORT << " in use?)"; + SocketDescriptor serverSd(0, "127.0.0.1"); + ASSERT_EQ(server->Startup(1, &serverSd, 1), RAKNET_STARTED) << "server Startup failed"; server->SetMaximumIncomingConnections(1); + const unsigned short serverPort = server->GetInternalID().GetPort(); SocketDescriptor clientSd; ASSERT_EQ(client->Startup(1, &clientSd, 1), RAKNET_STARTED) << "client Startup failed"; SystemAddress serverAddress; serverAddress.SetBinaryAddress("127.0.0.1"); - serverAddress.SetPortHostOrder(PEERHANDLE_TEST_PORT); + serverAddress.SetPortHostOrder(serverPort); - ASSERT_EQ(client->Connect("127.0.0.1", PEERHANDLE_TEST_PORT, 0, 0), CONNECTION_ATTEMPT_STARTED) + ASSERT_EQ(client->Connect("127.0.0.1", serverPort, 0, 0), CONNECTION_ATTEMPT_STARTED) << "Connect did not start"; // Wait for ID_CONNECTION_REQUEST_ACCEPTED on the client. This identifier is diff --git a/Tests/Integration/PingTestsTests.cpp b/Tests/Integration/PingTestsTests.cpp index fbaf3eb91..61980f4a0 100644 --- a/Tests/Integration/PingTestsTests.cpp +++ b/Tests/Integration/PingTestsTests.cpp @@ -159,8 +159,8 @@ TEST_F(PingTests, PingStatisticsAndOccasionalPing) receiver = RakPeerInterface::GetInstance(); destroyList.Push(receiver, _FILE_AND_LINE_); - SocketDescriptor receiverSd(0, "127.0.0.1"); // OS-assigned, per CLAUDE.md -- 60000 was fixed - receiver->Startup(2, &receiverSd, 1); + SocketDescriptor receiverSd(0, "127.0.0.1"); + ASSERT_EQ(receiver->Startup(2, &receiverSd, 1), RAKNET_STARTED) << "Receiver failed to start"; receiver->SetMaximumIncomingConnections(2); const unsigned short receiverPort = receiver->GetInternalID(UNASSIGNED_SYSTEM_ADDRESS).GetPort(); Packet *packet; diff --git a/Tests/Integration/ReliableOrderedConvertedTests.cpp b/Tests/Integration/ReliableOrderedConvertedTests.cpp index a3db4506f..4a52f1559 100644 --- a/Tests/Integration/ReliableOrderedConvertedTests.cpp +++ b/Tests/Integration/ReliableOrderedConvertedTests.cpp @@ -68,7 +68,7 @@ TEST_F(ReliableOrderedConverted, AllPacketsReceivedInOrder) char str[256]; char ip[32]; TimeMS sendInterval, nextSend, currentTime, quitTime; - unsigned short remotePort, localPort; + unsigned short remotePort; unsigned char streamNumberSender,streamNumberReceiver; BitStream bitStream; Packet *packet; @@ -81,6 +81,15 @@ TEST_F(ReliableOrderedConverted, AllPacketsReceivedInOrder) } + // The receiver is started first so the sender can connect to its OS-assigned port. + receiver = RegisterPeer(RakPeerInterface::GetInstance()); + + SocketDescriptor receiverSd(0, "127.0.0.1"); + StartupResult receiverResult = receiver->Startup(32, &receiverSd, 1); + ASSERT_EQ(receiverResult, RAKNET_STARTED) << "Receiver failed to start (error " << receiverResult << ")"; + remotePort = receiver->GetInternalID().GetPort(); + receiver->SetMaximumIncomingConnections(32); + sender = RegisterPeer(RakPeerInterface::GetInstance()); //sender->ApplyNetworkSimulator(.02, 100, 50); @@ -88,26 +97,11 @@ TEST_F(ReliableOrderedConverted, AllPacketsReceivedInOrder) strcpy(ip, "127.0.0.1"); - strcpy(str, "60000"); - remotePort=atoi(str); - strcpy(str, "0"); - localPort=atoi(str); - - SocketDescriptor senderSd(localPort, 0); + SocketDescriptor senderSd(0, 0); StartupResult senderResult = sender->Startup(1, &senderSd, 1); ASSERT_EQ(senderResult, RAKNET_STARTED) << "Sender failed to start (error " << senderResult << ")"; sender->Connect(ip, remotePort, 0, 0); - receiver = RegisterPeer(RakPeerInterface::GetInstance()); - - strcpy(str, "60000"); - localPort=atoi(str); - - SocketDescriptor receiverSd(localPort, 0); - StartupResult receiverResult = receiver->Startup(32, &receiverSd, 1); - ASSERT_EQ(receiverResult, RAKNET_STARTED) << "Receiver failed to start on port " << localPort << " (error " << receiverResult << ")"; - receiver->SetMaximumIncomingConnections(32); - // if (sender) // sender->ApplyNetworkSimulator(128000, 50, 100); // if (receiver) diff --git a/Tests/Integration/SecurityFunctionsTests.cpp b/Tests/Integration/SecurityFunctionsTests.cpp index 8c8251a9f..7b8afeeea 100644 --- a/Tests/Integration/SecurityFunctionsTests.cpp +++ b/Tests/Integration/SecurityFunctionsTests.cpp @@ -16,6 +16,7 @@ #include "mafianet/time.h" #include "mafianet/get_time.h" #include "CommonFunctions.h" +#include "TestHelpers.h" #include @@ -93,10 +94,9 @@ TEST_F(SecurityFunctions, PasswordAndBanList) client=RakPeerInterface::GetInstance(); SocketDescriptor clientSd; - client->Startup(1, &clientSd, 1); - SocketDescriptor serverSd(60000, 0); - server->Startup(1, &serverSd, 1); - server->SetMaximumIncomingConnections(1); + ASSERT_EQ(client->Startup(1, &clientSd, 1), RAKNET_STARTED); + const unsigned short serverPort = TestHelpers::StartOnLoopback(server, 1, 1); + ASSERT_NE(serverPort, 0) << "Server failed to start"; server->SetIncomingPassword(thePassword,(int)strlen(thePassword)); char returnedPass[22]; @@ -109,7 +109,7 @@ TEST_F(SecurityFunctions, PasswordAndBanList) SystemAddress serverAddress; serverAddress.SetBinaryAddress("127.0.0.1"); - serverAddress.SetPortHostOrder(60000); + serverAddress.SetPortHostOrder(serverPort); TimeMS entryTime=GetTimeMS(); // Testing if no password is rejected diff --git a/Tests/Integration/SystemAddressAndGuidTests.cpp b/Tests/Integration/SystemAddressAndGuidTests.cpp index ee9a07975..ae9687ecb 100644 --- a/Tests/Integration/SystemAddressAndGuidTests.cpp +++ b/Tests/Integration/SystemAddressAndGuidTests.cpp @@ -77,13 +77,18 @@ TEST_F(SystemAddressAndGuid, AddressAndGuidLookups) destroyList.Push(client, _FILE_AND_LINE_); ASSERT_FALSE(client->IsActive()) << "Client was active but shouldn't be yet"; - SocketDescriptor clientSd(60001, 0); - client->Startup(1, &clientSd, 1); + // All interfaces as before, but on an OS-assigned port. + SocketDescriptor clientSd(0, 0); + ASSERT_EQ(client->Startup(1, &clientSd, 1), RAKNET_STARTED); + const unsigned short clientPort = TestHelpers::BoundPort(client); + ASSERT_NE(clientPort, 0); ASSERT_TRUE(client->IsActive()) << "Client was not active but should be"; //Passed by reference for initializations TestHelpers::StandardServerPrep(server, destroyList); + const unsigned short serverPort = TestHelpers::BoundPort(server); + ASSERT_NE(serverPort, 0) << "Server failed to start"; ASSERT_TRUE(TestHelpers::WaitAndConnectTwoPeersLocally(client, server, 5000)) << "Could not connect the client"; @@ -124,7 +129,7 @@ TEST_F(SystemAddressAndGuid, AddressAndGuidLookups) printf("Test GetSystemListValues of the system and guid list\n"); SystemAddress serverAddress; serverAddress.SetBinaryAddress("127.0.0.1"); - serverAddress.SetPortHostOrder(60000); + serverAddress.SetPortHostOrder(serverPort); ASSERT_TRUE(systemList[0] == serverAddress) << "System address from list is wrong."; @@ -146,7 +151,7 @@ TEST_F(SystemAddressAndGuid, AddressAndGuidLookups) SystemAddress clientAddress; clientAddress.SetBinaryAddress("127.0.0.1"); - clientAddress.SetPortHostOrder(60001); + clientAddress.SetPortHostOrder(clientPort); printf("Test GetExternalID, automatic testing is not only required for this\nbecause of it's nature\nShould be supplemented by internet tests\n"); diff --git a/Tests/Integration/TypedSendLiveTests.cpp b/Tests/Integration/TypedSendLiveTests.cpp index 820f4a9ee..5638da96d 100644 --- a/Tests/Integration/TypedSendLiveTests.cpp +++ b/Tests/Integration/TypedSendLiveTests.cpp @@ -18,9 +18,6 @@ using namespace MafiaNet; namespace { - // A distinct port so a full-suite run does not collide with other tests. - const unsigned short TYPED_SEND_TEST_PORT = 61018; - // One typed message, registered identically on both ends so encode()/dispatch() // agree on its auto-assigned id (ID_USER_PACKET_ENUM). struct ChatMessage @@ -64,10 +61,10 @@ TEST_F(TypedSendLive, SendDeliversTypedMessagesAndBroadcastReachesAllPeers) RakSleep(2); Peer clientB; - SocketDescriptor serverSd(TYPED_SEND_TEST_PORT, 0); - ASSERT_EQ(server->Startup(2, &serverSd, 1), RAKNET_STARTED) - << "Server Startup failed (port " << TYPED_SEND_TEST_PORT << " in use?)"; + SocketDescriptor serverSd(0, "127.0.0.1"); + ASSERT_EQ(server->Startup(2, &serverSd, 1), RAKNET_STARTED) << "Server Startup failed"; server->SetMaximumIncomingConnections(2); + const unsigned short serverPort = server->GetInternalID().GetPort(); SocketDescriptor clientSd; ASSERT_TRUE(clientA->Startup(1, &clientSd, 1) == RAKNET_STARTED && clientB->Startup(1, &clientSd, 1) == RAKNET_STARTED) @@ -75,7 +72,7 @@ TEST_F(TypedSendLive, SendDeliversTypedMessagesAndBroadcastReachesAllPeers) SystemAddress serverAddress; serverAddress.SetBinaryAddress("127.0.0.1"); - serverAddress.SetPortHostOrder(TYPED_SEND_TEST_PORT); + serverAddress.SetPortHostOrder(serverPort); // Server dispatcher: counts typed ChatMessages and remembers the last one. int chatReceived = 0; @@ -95,8 +92,8 @@ TEST_F(TypedSendLive, SendDeliversTypedMessagesAndBroadcastReachesAllPeers) clientAD.on([&](const ChatMessage &m, const Sender &) { ++chatA; lastA = m; }); clientBD.on([&](const ChatMessage &m, const Sender &) { ++chatB; lastB = m; }); - ASSERT_TRUE(clientA->Connect("127.0.0.1", TYPED_SEND_TEST_PORT, 0, 0) == CONNECTION_ATTEMPT_STARTED && - clientB->Connect("127.0.0.1", TYPED_SEND_TEST_PORT, 0, 0) == CONNECTION_ATTEMPT_STARTED) + ASSERT_TRUE(clientA->Connect("127.0.0.1", serverPort, 0, 0) == CONNECTION_ATTEMPT_STARTED && + clientB->Connect("127.0.0.1", serverPort, 0, 0) == CONNECTION_ATTEMPT_STARTED) << "Connect did not start"; // Wait until the server reports two connected systems. diff --git a/Tests/Support/TestHelpers.cpp b/Tests/Support/TestHelpers.cpp index 0c2d492c3..9914e46a6 100644 --- a/Tests/Support/TestHelpers.cpp +++ b/Tests/Support/TestHelpers.cpp @@ -18,14 +18,24 @@ TestHelpers::~TestHelpers(void) { } -void TestHelpers::StandardServerPrep(RakPeerInterface *&server) +unsigned short TestHelpers::StartOnLoopback(RakPeerInterface *peer, unsigned int maxConnections, unsigned short maxIncoming) { + SocketDescriptor sd(0, "127.0.0.1"); + if (peer->Startup(maxConnections, &sd, 1) != RAKNET_STARTED) + return 0; + peer->SetMaximumIncomingConnections(maxIncoming); + return BoundPort(peer); +} - server=RakPeerInterface::GetInstance(); - SocketDescriptor serverSd(60000, 0); - server->Startup(1, &serverSd, 1); - server->SetMaximumIncomingConnections(1); +unsigned short TestHelpers::BoundPort(RakPeerInterface *peer) +{ + return peer->GetInternalID().GetPort(); +} +void TestHelpers::StandardServerPrep(RakPeerInterface *&server) +{ + server=RakPeerInterface::GetInstance(); + StartOnLoopback(server, 1, 1); } void TestHelpers::StandardClientPrep(RakPeerInterface *&client) diff --git a/Tests/Support/TestHelpers.h b/Tests/Support/TestHelpers.h index dd04f8054..1cb16e2e9 100644 --- a/Tests/Support/TestHelpers.h +++ b/Tests/Support/TestHelpers.h @@ -29,6 +29,14 @@ class TestHelpers TestHelpers(void); ~TestHelpers(void); + // Starts peer on 127.0.0.1 with a port the OS assigns, so tests can run in + // parallel. Returns that port (0 if Startup failed). Never bind a fixed + // port in a test. + static unsigned short StartOnLoopback(RakPeerInterface *peer, unsigned int maxConnections, unsigned short maxIncoming = 0); + // The port a started peer is bound to. + static unsigned short BoundPort(RakPeerInterface *peer); + + // Server on an OS-assigned port: read it with BoundPort(server). static void StandardServerPrep(RakPeerInterface *&server); static void StandardClientPrep(RakPeerInterface *&client); static void StandardServerPrep(RakPeerInterface *&server,DataStructures::List &destroyList); From eae5b22b366cf6d326d4bca4c8c03cf5f46ce911 Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:35:27 +0200 Subject: [PATCH 09/12] ci, docs: parallel test runs, Windows integration suite, new tooling docs - ctest runs with --parallel in every job (2 under sanitizers), now that no test binds a fixed port. - Windows runs the integration suite too; it was build-only because of the shared fixed ports. - building.rst and CLAUDE.md document the warning policy, fuzzing (and adding crash reproducers to the corpus), benchmarks, coverage, the simulated network as the default home for peer-level tests, and the ephemeral-port rule. --- .github/workflows/build.yml | 23 ++++++++--- CLAUDE.md | 29 ++++++++++++-- Dockerfile | 2 +- docs/getting-started/building.rst | 64 +++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 174f5bc14..361fb5285 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -118,6 +118,7 @@ jobs: run: | ctest --test-dir build -L unit \ --output-on-failure \ + --parallel 4 \ --timeout 600 \ --output-junit junit-unit.xml @@ -125,6 +126,7 @@ jobs: run: | ctest --test-dir build -L integration \ --output-on-failure \ + --parallel 4 \ --repeat until-pass:3 \ --timeout 600 \ --output-junit junit-integration.xml @@ -230,6 +232,7 @@ jobs: - name: Run integration tests run: | ctest --preset ${{ matrix.preset }} -L integration \ + --parallel 2 \ --repeat until-pass:3 \ --output-log build/${{ matrix.preset }}/integration.log \ --output-junit junit-integration.xml @@ -366,7 +369,7 @@ jobs: - name: Run tests run: | ctest --preset coverage -L unit --parallel 4 - ctest --preset coverage -L integration --repeat until-pass:3 + ctest --preset coverage -L integration --parallel 4 --repeat until-pass:3 - name: Report if: always() @@ -416,6 +419,7 @@ jobs: run: | ctest --test-dir build \ --output-on-failure \ + --parallel 4 \ --repeat until-pass:3 \ --timeout 600 \ --output-junit junit.xml @@ -428,9 +432,8 @@ jobs: path: build/junit.xml if-no-files-found: ignore - # Windows: Build everything, run the hermetic unit suite. The legacy - # integration suite (loopback networking) is not yet validated on Windows - # runners, so it stays build-only for now. + # Windows: build everything, run both suites. The integration suite became + # runnable here once every test moved to OS-assigned ports. windows: runs-on: windows-latest steps: @@ -449,13 +452,23 @@ jobs: run: | ctest --test-dir build -C Release -L unit ` --output-on-failure ` + --parallel 4 ` --timeout 600 ` --output-junit junit.xml + - name: Run integration tests + run: | + ctest --test-dir build -C Release -L integration ` + --output-on-failure ` + --parallel 4 ` + --repeat until-pass:3 ` + --timeout 600 ` + --output-junit junit-integration.xml + - name: Upload test results if: always() uses: actions/upload-artifact@v4 with: name: test-results-windows - path: build/junit.xml + path: build/junit*.xml if-no-files-found: ignore diff --git a/CLAUDE.md b/CLAUDE.md index cdfe8c4c1..2e6939bf8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,13 +67,31 @@ Available generators (run `cmake --help` for full list): | `MAFIANET_MINIMAL` | OFF | Compile out every optional plugin (core transport only); built by the `linux-minimal` CI job | | `MAFIANET_SANITIZER` | (empty) | `address+undefined` or `thread`; applied to every target. Prefer the `asan` / `tsan` presets | | `MAFIANET_USE_COMPILER_CACHE` | ON | Use sccache/ccache as compiler launcher when found on `PATH` | +| `MAFIANET_WARNINGS_AS_ERRORS` | OFF | `-Werror` / `/WX` on MafiaNet's own targets (on in the Linux/macOS CI jobs) | +| `MAFIANET_BUILD_FUZZERS` | OFF | libFuzzer harnesses in `Tests/Fuzz` (Clang); use the `fuzz` preset | +| `MAFIANET_BUILD_BENCHMARKS` | OFF | google/benchmark microbenchmarks in `Benchmarks/`; use the `bench` preset | +| `MAFIANET_COVERAGE` | OFF | `--coverage` instrumentation; use the `coverage` preset, report with gcovr | -Presets (`CMakePresets.json`, CMake 3.25+): `debug`, `release`, `ci`, `minimal`, `asan`, `tsan`. +Presets (`CMakePresets.json`, CMake 3.25+): `debug`, `release`, `ci`, `minimal`, `asan`, `tsan`, +`fuzz`, `bench`, `coverage`. `cmake --workflow --preset ` configures, builds and tests in one go; run sanitizer tests through `ctest --preset asan|tsan` so the sanitizer options and `ci/tsan.supp` are applied. Known by-design TSan races live in `ci/tsan.supp`; add an entry only for a race that is part of RakNet's lock-free getter model, never to silence a new bug. +**Warnings:** MafiaNet's own code builds with zero `-Wall -Wextra` warnings on Clang and GCC, and +CI enforces it (`MAFIANET_WARNINGS_AS_ERRORS`). Fix a new warning properly; suppress only with a +narrow, commented `[[maybe_unused]]` or pragma, never by loosening the flags. Warnings are applied +per target by `mafianet_enable_warnings()` (`cmake/MafiaNetHelpers.cmake`), never globally, so +fetched dependencies are not affected; call it for any new target. + +**Fuzzing:** `Tests/Fuzz/_fuzz.cpp` harnesses cover code that parses what remote peers send +(`bit_stream`, `reliability_layer`, `peer_datagram` — a whole server on the simulated network). +Each is also built as `_replay`, which replays `Tests/Fuzz/corpus//` as the unit test +`FuzzCorpus.` on every compiler. When a fuzzer finds a crash, fix it and add the minimized +reproducer to that corpus directory. Code that parses network input must not `RakAssert` on +malformed data: a Debug build would then abort on whatever a peer sends — reject and drop instead. + When adding or removing a plugin guarded by a `_RAKNET_SUPPORT_` flag, update `MAFIANET_OPTIONAL_FEATURES` in `Source/CMakeLists.txt` as well, or the CMake plugin selection silently drifts from `native_feature_includes.h`. @@ -133,7 +151,8 @@ Keep its message counts above `MMSG_BATCH_MAX` or that coverage silently disappe All tests are GoogleTest, built with `MAFIANET_BUILD_TESTS=ON` (requires `MAFIANET_BUILD_STATIC=ON`) and driven by CTest. Two binaries under `Tests/`: - **`UnitTests`** (`Tests/Unit/`, label `unit`): hermetic and deterministic — no loopback networking, no wall-clock timing. A started-but-unconnected peer with synchronous assertions is acceptable. New tests for pure logic go here. -- **`IntegrationTests`** (`Tests/Integration/`, label `integration`): real UDP over loopback. Discovered tests get `RUN_SERIAL` (several bind fixed ports) and a 600s timeout. Shared polling/connect helpers live in `Tests/Support/`. +- **`IntegrationTests`** (`Tests/Integration/`, label `integration`): real UDP over loopback, real threads. Every test binds OS-assigned ports, so the suite runs in parallel (`ctest -j`); 600s per-test timeout. Shared polling/connect helpers live in `Tests/Support/`. +- **`FuzzCorpus.*`** (label `unit`): replays the fuzz corpus through each harness (see Fuzzing above). Under ctest, every `TEST()` runs in its own process — never write tests that depend on state from a previous `TEST()`. Long sequential networking scenarios belong in a single `TEST_F`. @@ -153,7 +172,9 @@ For debugging, run a binary directly with a filter: `./build/Tests/IntegrationTe **Every change to library code in `Source/` needs tests, written in the new paradigm.** Do not create new test harnesses, `main()` functions, standalone test executables, or sample apps that act as tests — add GoogleTest cases to the existing suites. New `.cpp` files under `Tests/Unit/` and `Tests/Integration/` are picked up automatically by the build (glob); no CMake edits are needed unless you add a new directory. -**Default to Unit; escalate to Integration only when the behavior under test genuinely requires traffic.** Ask: can this be tested by calling the API synchronously? Serialization, containers, builders, dispatch tables, ID assignment, string/GUID utilities, replica-visibility queries — all Unit. Only reliability, connection lifecycle, and anything that must cross the network thread belongs in Integration. +**Default to Unit; escalate to Integration only when the behavior under test genuinely requires real sockets or threads.** Ask: can this be tested by calling the API synchronously? Serialization, containers, builders, dispatch tables, ID assignment, string/GUID utilities, replica-visibility queries — all Unit. + +**Peer-level behavior goes in Unit too, on the simulated network.** `Tests/Support/SimulatedNetwork.h` runs whole `RakPeer`s with no sockets, no threads and a virtual clock: connect handshakes, reliability under loss/jitter/reordering, MTU discovery, timeouts, disconnects, plugins. `net.AddPeer(...)`, `net.Connect(client, server)`, `net.SetLinkBothWays(a, b, rule)`, then `net.RunUntilReceived(peer, ID_..., maxSimMs)` — deterministic, milliseconds, no flakes (see `SimulatedPeerTests.cpp`). Integration is for what the simulation cannot show: the OS socket layer (batched recvmmsg/sendmmsg, socket errors, binding), the threaded update loop, and wall-clock behavior. **Unit tests** (`Tests/Unit/Tests.cpp`): - Hermetic and deterministic: no `Connect()`, no `RakSleep`-with-deadline polling, no dependence on wall-clock time or packet arrival. A started-but-unconnected peer with purely synchronous assertions is acceptable (see `GuidUtilTests.cpp`, `VirtualWorldTests.cpp`). @@ -162,7 +183,7 @@ For debugging, run a binary directly with a filter: `./build/Tests/IntegrationTe **Integration tests** (`Tests/Integration/Tests.cpp`): - One `TEST_F` per sequential scenario. Never split phases that share connection state into separate `TEST`s — each `TEST` runs in its own process under ctest. -- Ports: use OS-assigned ephemeral ports (`SocketDescriptor(0, "127.0.0.1")`, then `peer->GetInternalID().GetPort()`), never a new fixed port. (Some ported legacy tests still bind fixed ports; that is why the suite is `RUN_SERIAL` — do not add to the problem.) +- Ports: only OS-assigned ephemeral ports — `TestHelpers::StartOnLoopback(peer, maxConnections, maxIncoming)` returns the port, `TestHelpers::BoundPort(peer)` reads it — never a fixed port. The suite runs in parallel, so a fixed port (or a fixed temp directory) makes tests collide. Check every `Startup()` result. - Waiting: always poll for a condition with a deadline (`while (GetTimeMS() - start < N && !condition) { pump; RakSleep(30); }`), never assert immediately after a state change — packets surface asynchronously from each peer's network thread, and both sides of a handshake must be pumped until *each* has observed the event. - Never use a bare `RakSleep(N)` as a synchronization primitive; sleep only as the polling interval inside a deadline loop. - Cleanup must survive a failed `ASSERT_`: destroy peers in fixture `TearDown()` (`Shutdown(100)` then `DestroyInstance` for every peer created) or use RAII `Peer`/`PacketPtr` handles from `mafianet/peer_handle.h`. Never rely on code after the assertions to clean up. diff --git a/Dockerfile b/Dockerfile index 301e6b59e..8a033a8c0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,4 +28,4 @@ RUN cmake -B build -DMAFIANET_BUILD_SAMPLES=ON -DMAFIANET_BUILD_TESTS=ON \ # per-test timeout and bounded retries for transient timing misses on loaded # runners. JUnit XML is written to /results (mount it to collect the report). RUN mkdir -p /results -CMD ["ctest", "--test-dir", "build", "--output-on-failure", "--repeat", "until-pass:3", "--timeout", "600", "--output-junit", "/results/junit.xml"] +CMD ["ctest", "--test-dir", "build", "--output-on-failure", "--parallel", "4", "--repeat", "until-pass:3", "--timeout", "600", "--output-junit", "/results/junit.xml"] diff --git a/docs/getting-started/building.rst b/docs/getting-started/building.rst index 3d29b976d..460cf8d5a 100644 --- a/docs/getting-started/building.rst +++ b/docs/getting-started/building.rst @@ -68,6 +68,18 @@ Configure build options with CMake: * - ``MAFIANET_USE_COMPILER_CACHE`` - ON - Use ``sccache`` or ``ccache`` as the compiler launcher when one is on ``PATH``. An explicit ``CMAKE__COMPILER_LAUNCHER`` takes precedence. + * - ``MAFIANET_WARNINGS_AS_ERRORS`` + - OFF + - Treat warnings in MafiaNet's own code (``-Wall -Wextra``, ``/W4``) as errors. Fetched dependencies are never affected. + * - ``MAFIANET_BUILD_FUZZERS`` + - OFF + - Build the libFuzzer harnesses (Clang only). See `Fuzzing`_. + * - ``MAFIANET_BUILD_BENCHMARKS`` + - OFF + - Build the google/benchmark microbenchmarks. See `Benchmarks`_. + * - ``MAFIANET_COVERAGE`` + - OFF + - Instrument for line coverage (GCC/Clang). See `Coverage`_. Example with options: @@ -225,6 +237,12 @@ preset builds into ``build/``: - Release with ``MAFIANET_MINIMAL=ON``. * - ``asan`` / ``tsan`` - RelWithDebInfo and tests under AddressSanitizer + UBSan, or ThreadSanitizer. Not available on Windows. + * - ``fuzz`` + - Clang, RelWithDebInfo, the libFuzzer harnesses under ASan + UBSan. + * - ``bench`` + - Release, the microbenchmarks. + * - ``coverage`` + - Debug and tests with ``--coverage``. Workflow presets configure, build and test in one command: @@ -255,6 +273,52 @@ is advisory and exists to catch new races. On Linux hosts with On macOS, the ``.devcontainer`` gives a Linux environment for running the sanitizers and the Linux-only batched datagram paths. +Fuzzing +------- + +``Tests/Fuzz/`` holds libFuzzer harnesses for the code that parses what remote +peers send: ``bit_stream`` (BitStream deserialization), ``reliability_layer`` +(the datagram parser of a connected ReliabilityLayer) and ``peer_datagram`` +(a whole server peer on the simulated network, from offline messages through +a connected session). Build and run them with Clang: + +.. code-block:: bash + + cmake --workflow --preset fuzz + ./build/fuzz/Tests/Fuzz/peer_datagram_fuzz Tests/Fuzz/corpus/peer_datagram -max_total_time=300 + +Pass a scratch copy of the corpus directory if you do not want new inputs +written into the source tree. Every harness is also built as +``_replay`` with the tests, on every compiler, and replays +``Tests/Fuzz/corpus//`` as the unit test ``FuzzCorpus.``. When the +fuzzer finds a crash, fix it and add the minimized reproducer +(``-minimize_crash=1``) to that directory so every build keeps checking it. + +CI fuzzes each harness for one minute on every pull request, and for fifteen +minutes nightly with a corpus that persists between runs. + +Benchmarks +---------- + +.. code-block:: bash + + cmake --preset bench && cmake --build --preset bench + ./build/bench/Benchmarks/MafiaNetBenchmarks --benchmark_filter=ReliabilityLayer + +Numbers only mean something against a baseline on the same machine: run the +benchmarks on both revisions with ``--benchmark_out= +--benchmark_out_format=json`` and compare them with google/benchmark's +``tools/compare.py``. + +Coverage +-------- + +.. code-block:: bash + + cmake --preset coverage && cmake --build --preset coverage + ctest --preset coverage + gcovr --root . --filter Source/ build/coverage --html-details coverage/index.html + Running Tests ------------- From e0735494542725ec1e1102498f2d39f24011dbdd Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:48:19 +0200 Subject: [PATCH 10/12] test: avoid a GCC 13 -Wfree-nonheap-object false positive in the reliability fuzz harness Constructing the per-datagram std::vector from an offset range made GCC 13 at -O2 warn about freeing a pointer with a nonzero offset, which -Werror turned into a build failure in the Linux Release job. An exactly sized heap copy keeps the ASan coverage and avoids the warning. --- Tests/Fuzz/reliability_layer_fuzz.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Tests/Fuzz/reliability_layer_fuzz.cpp b/Tests/Fuzz/reliability_layer_fuzz.cpp index 87f88606e..c526062c2 100644 --- a/Tests/Fuzz/reliability_layer_fuzz.cpp +++ b/Tests/Fuzz/reliability_layer_fuzz.cpp @@ -13,7 +13,8 @@ #include #include -#include +#include +#include #include "mafianet/bit_stream.h" #include "mafianet/ds_list.h" @@ -61,14 +62,15 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) if (length > MAXIMUM_MTU_SIZE) length = MAXIMUM_MTU_SIZE; - // The layer's parser takes a mutable buffer; give it its own copy so a - // write past the datagram is caught by ASan instead of corrupting input. - std::vector datagram(data + offset, data + offset + length); - offset += length; - if (datagram.empty()) + // The layer's parser takes a mutable buffer; give it its own exactly + // sized copy so a read or write past the datagram is caught by ASan. + if (length == 0) continue; + std::unique_ptr datagram(new char[length]); + std::memcpy(datagram.get(), data + offset, length); + offset += length; - layer.HandleSocketReceiveFromConnectedPlayer(datagram.data(), (unsigned int) datagram.size(), + layer.HandleSocketReceiveFromConnectedPlayer(datagram.get(), (unsigned int) length, remote, handlers, MAXIMUM_MTU_SIZE, &socket, &rnr, now, updateBitStream); now += 10000; From b241f48782303cf7eb2ad84e048d89d4ec32e41e Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:06:09 +0200 Subject: [PATCH 11/12] fix: CI failures on CMake 3.22, MSVC and the coverage report - DOWNLOAD_EXTRACT_TIMESTAMP only exists from CMake 3.24; on 3.22 (the Ubuntu 22.04 job) it was read as part of URL_HASH and configure failed. Pass it only where supported. - SimulatedNetwork.cpp used std::back_inserter without , which MSVC does not pull in transitively. - Coverage counters were updated concurrently by the network threads and came out negative, which gcovr rejects: build with -fprofile-update=atomic, and have gcovr warn rather than abort if it still sees one. --- .github/workflows/build.yml | 1 + CMakeLists.txt | 4 +++- Tests/Support/SimulatedNetwork.cpp | 1 + cmake/FetchVoiceDependencies.cmake | 8 +++++++- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 361fb5285..6334b04de 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -376,6 +376,7 @@ jobs: run: | mkdir -p coverage-html gcovr --root . --filter 'Source/' build/coverage \ + --gcov-ignore-parse-errors=negative_hits.warn_once_per_file \ --html-details coverage-html/index.html \ --markdown-summary coverage-summary.md --print-summary cat coverage-summary.md >> "$GITHUB_STEP_SUMMARY" diff --git a/CMakeLists.txt b/CMakeLists.txt index 1e6fbc839..aa7f5eb74 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -103,7 +103,9 @@ if(MAFIANET_COVERAGE) if(MSVC) message(FATAL_ERROR "MAFIANET_COVERAGE is not supported with MSVC") endif() - add_compile_options(--coverage -O0) + # Atomic counters: the network threads update them concurrently, and + # non-atomic ones come out negative, which gcovr rejects. + add_compile_options(--coverage -O0 -fprofile-update=atomic) add_link_options(--coverage) endif() diff --git a/Tests/Support/SimulatedNetwork.cpp b/Tests/Support/SimulatedNetwork.cpp index 853920953..455716e93 100644 --- a/Tests/Support/SimulatedNetwork.cpp +++ b/Tests/Support/SimulatedNetwork.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include "mafianet/assert.h" #include "mafianet/message_identifiers.h" diff --git a/cmake/FetchVoiceDependencies.cmake b/cmake/FetchVoiceDependencies.cmake index 98bf69edb..97857ee72 100644 --- a/cmake/FetchVoiceDependencies.cmake +++ b/cmake/FetchVoiceDependencies.cmake @@ -41,11 +41,17 @@ FetchContent_Declare( # RNNoise model data (neural network weights) set(RNNOISE_MODEL_VERSION "0b50c45") +# DOWNLOAD_EXTRACT_TIMESTAMP (CMP0135) only exists from CMake 3.24; older +# versions would read it as part of URL_HASH. +set(_rnnoise_model_timestamp) +if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.24) + set(_rnnoise_model_timestamp DOWNLOAD_EXTRACT_TIMESTAMP TRUE) +endif() FetchContent_Declare( rnnoise_model URL https://media.xiph.org/rnnoise/models/rnnoise_data-${RNNOISE_MODEL_VERSION}.tar.gz URL_HASH SHA256=4ac81c5c0884ec4bd5907026aaae16209b7b76cd9d7f71af582094a2f98f4b43 - DOWNLOAD_EXTRACT_TIMESTAMP TRUE + ${_rnnoise_model_timestamp} ) # ----------------------------------------------------------------------------- From aeb2020e7f5cd676392594c73e9b600274db6d9d Mon Sep 17 00:00:00 2001 From: Segfault <5221072+Segfaultd@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:13:57 +0200 Subject: [PATCH 12/12] test: make GuidUtil's iteration count static for MSVC Dropping it from the lambda capture silenced Clang's unused-capture warning but MSVC requires a non-static local to be captured. A static constant needs no capture on either compiler. --- Tests/Unit/GuidUtilTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Unit/GuidUtilTests.cpp b/Tests/Unit/GuidUtilTests.cpp index 5706dfe42..25f6888a3 100644 --- a/Tests/Unit/GuidUtilTests.cpp +++ b/Tests/Unit/GuidUtilTests.cpp @@ -46,7 +46,7 @@ TEST(GuidUtil, UnassignedSentinelStringifiesToDocumentedLabel) TEST(GuidUtil, ToStringIsThreadSafe) { const unsigned threadCount = 16; - constexpr unsigned iterations = 20000; + static constexpr unsigned iterations = 20000; // static: MSVC would need it captured, Clang flags the capture std::atomic corrupted(false); std::vector threads;