Sync nuclear subtree from NUClear@2441bdf

Pull PR #190 review fixes: fragment validation, Discovery/Reliability
hardening, LogLevel/MSVC CI fixes, and related nuclearnet updates.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: TrentHouliston

src/nuclear/src/nuclearnet/Discovery.cpp

@@ -235,9 +235,10 @@ namespace network {
          * Get a specific peer's info.
          *
          * @param address The peer's address
-         * @return Pointer to peer info, or nullptr if not found
+         * @param out     Filled with peer info on success
+         * @return true if the peer was found
          */
-        const PeerInfo* get_peer(const sock_t& address) const;
+        bool get_peer(const sock_t& address, PeerInfo& out) const;
 
         /**
          * Evict all known peers, firing leave callbacks for any that were fully connected.

src/nuclear/src/nuclearnet/Discovery.hpp

@@ -1,104 +1,104 @@
 /*
-     * MIT License
-     *
-     * Copyright (c) 2025 NUClear Contributors
-     *
-     * This file is part of the NUClear codebase.
-     * See https://github.com/Fastcode/NUClear for further info.
-     *
-     * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
-     * documentation files (the "Software"), to deal in the Software without restriction, including without limitation
-     * the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and
-     * to permit persons to whom the Software is furnished to do so, subject to the following conditions:
-     *
-     * The above copyright notice and this permission notice shall be included in all copies or substantial portions of
-     * the Software.
-     *
-     * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO
-     * THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-     * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
-     * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
-     * IN THE SOFTWARE.
-     */
+ * MIT License
+ *
+ * Copyright (c) 2025 NUClear Contributors
+ *
+ * This file is part of the NUClear codebase.
+ * See https://github.com/Fastcode/NUClear for further info.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and
+ * to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of
+ * the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO
+ * THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
+ * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
 
 #ifndef NUCLEAR_NETWORK_FILE_DESCRIPTOR_HPP
 #define NUCLEAR_NETWORK_FILE_DESCRIPTOR_HPP
 
 #include "../util/platform.hpp"
 
-    namespace NUClear {
-    namespace network {
-
-        /**
-         * RAII wrapper for a file descriptor (socket).
-         * Automatically closes the file descriptor on destruction.
-         * Non-copyable, move-only.
-         */
-        class FileDescriptor {
-        public:
-            /// Construct with an invalid descriptor
-            FileDescriptor() = default;
-
-            /// Construct taking ownership of an existing file descriptor
-            explicit FileDescriptor(fd_t fd) : fd(fd) {}
+namespace NUClear {
+namespace network {
 
-            /// Destructor closes the descriptor if valid
-            ~FileDescriptor() {
+    /**
+     * RAII wrapper for a file descriptor (socket).
+     * Automatically closes the file descriptor on destruction.
+     * Non-copyable, move-only.
+     */
+    class FileDescriptor {
+    public:
+        /// Construct with an invalid descriptor
+        FileDescriptor() = default;
+
+        /// Construct taking ownership of an existing file descriptor
+        explicit FileDescriptor(fd_t fd) : fd(fd) {}
+
+        /// Destructor closes the descriptor if valid
+        ~FileDescriptor() {
+            reset();
+        }
+
+        // Non-copyable
+        FileDescriptor(const FileDescriptor&)            = delete;
+        FileDescriptor& operator=(const FileDescriptor&) = delete;
+
+        // Movable
+        FileDescriptor(FileDescriptor&& other) noexcept : fd(other.fd) {
+            other.fd = INVALID_SOCKET;
+        }
+        FileDescriptor& operator=(FileDescriptor&& other) noexcept {
+            if (this != &other) {
                 reset();
-            }
-
-            // Non-copyable
-            FileDescriptor(const FileDescriptor&)            = delete;
-            FileDescriptor& operator=(const FileDescriptor&) = delete;
-
-            // Movable
-            FileDescriptor(FileDescriptor&& other) noexcept : fd(other.fd) {
+                fd       = other.fd;
                 other.fd = INVALID_SOCKET;
             }
-            FileDescriptor& operator=(FileDescriptor&& other) noexcept {
-                if (this != &other) {
-                    reset();
-                    fd       = other.fd;
-                    other.fd = INVALID_SOCKET;
-                }
-                return *this;
-            }
-
-            /// Get the raw file descriptor
-            fd_t get() const {
-                return fd;
-            }
-
-            /// Check if the descriptor is valid
-            bool valid() const {
-                return fd != INVALID_SOCKET;
-            }
-
-            /// Implicit conversion to fd_t for use with system calls
-            operator fd_t() const {  // NOLINT(google-explicit-constructor)
-                return fd;
-            }
-
-            /// Release ownership without closing
-            fd_t release() {
-                const fd_t old = fd;
-                fd             = INVALID_SOCKET;
-                return old;
-            }
-
-            /// Close the current descriptor and take ownership of a new one
-            void reset(fd_t new_fd = INVALID_SOCKET) {
-                if (fd != INVALID_SOCKET) {
-                    ::close(fd);
-                }
-                fd = new_fd;
+            return *this;
+        }
+
+        /// Get the raw file descriptor
+        fd_t get() const {
+            return fd;
+        }
+
+        /// Check if the descriptor is valid
+        bool valid() const {
+            return fd != INVALID_SOCKET;
+        }
+
+        /// Implicit conversion to fd_t for use with system calls
+        operator fd_t() const {  // NOLINT(google-explicit-constructor)
+            return fd;
+        }
+
+        /// Release ownership without closing
+        fd_t release() {
+            const fd_t old = fd;
+            fd             = INVALID_SOCKET;
+            return old;
+        }
+
+        /// Close the current descriptor and take ownership of a new one
+        void reset(fd_t new_fd = INVALID_SOCKET) {
+            if (fd != INVALID_SOCKET) {
+                ::close(fd);
             }
+            fd = new_fd;
+        }
 
-        private:
-            fd_t fd{INVALID_SOCKET};
-        };
+    private:
+        fd_t fd{INVALID_SOCKET};
+    };
 
-    }  // namespace network
+}  // namespace network
 }  // namespace NUClear
 
 #endif  // NUCLEAR_NETWORK_FILE_DESCRIPTOR_HPP

src/nuclear/src/nuclearnet/FileDescriptor.hpp

@@ -38,13 +38,16 @@ namespace network {
                                  std::chrono::steady_clock::duration assembly_timeout)
         : packet_mtu(packet_mtu), max_assembly_size(max_assembly_size), assembly_timeout(assembly_timeout) {}
 
+    uint16_t Fragmentation::compute_fragment_count(const std::size_t payload_length, const uint16_t mtu) {
+        return payload_length == 0 ? 1 : static_cast<uint16_t>((payload_length + mtu - 1) / mtu);
+    }
+
     std::vector<Fragmentation::Fragment> Fragmentation::fragment(uint16_t packet_id,
                                                                   uint64_t hash,
                                                                   uint8_t flags,
                                                                   const std::vector<uint8_t>& payload) const {
         // Calculate how many fragments we need
-        const uint16_t packet_count =
-            payload.empty() ? 1 : static_cast<uint16_t>((payload.size() + packet_mtu - 1) / packet_mtu);
+        const uint16_t packet_count = compute_fragment_count(payload.size(), packet_mtu);
 
         std::vector<Fragment> fragments;
         fragments.reserve(packet_count);
@@ -82,7 +85,11 @@ namespace network {
             return false;
         }
 
-        // Enforce max assembly size check
+        if (data_length > packet_mtu) {
+            return false;
+        }
+
+        // Fast reject for absurd packet counts
         if (max_assembly_size > 0) {
             const std::size_t projected_size = static_cast<std::size_t>(packet_count) * packet_mtu;
             if (projected_size > max_assembly_size) {
@@ -94,14 +101,23 @@ namespace network {
 
         const std::lock_guard<std::mutex> lock(assembly_mutex);
 
-        auto& assembly          = assemblies[key];
-        assembly.hash           = hash;
-        assembly.flags          = flags;
-        assembly.packet_count   = packet_count;
-        assembly.last_update    = now;
+        auto& assembly        = assemblies[key];
+        assembly.hash         = hash;
+        assembly.flags        = flags;
+        assembly.packet_count = packet_count;
+        assembly.last_update  = now;
+
+        auto& frag = assembly.fragments[packet_no];
+        if (frag.empty()) {
+            if (max_assembly_size > 0 && assembly.accumulated_size + data_length > max_assembly_size) {
+                assemblies.erase(key);
+                return false;
+            }
+            assembly.accumulated_size += data_length;
+        }
 
         // Store this fragment
-        assembly.fragments[packet_no].assign(data, data + data_length);
+        frag.assign(data, data + data_length);
 
         // Check if we have all fragments
         if (assembly.fragments.size() == packet_count) {

src/nuclear/src/nuclearnet/Fragmentation.cpp

@@ -134,6 +134,11 @@ namespace network {
             return packet_mtu;
         }
 
+        /**
+         * Compute the number of fragments required for a payload.
+         */
+        static uint16_t compute_fragment_count(std::size_t payload_length, uint16_t packet_mtu);
+
     private:
         /// Key for an in-progress assembly: (source_key, packet_id)
         using AssemblyKey = std::pair<uint64_t, uint16_t>;
@@ -143,6 +148,7 @@ namespace network {
             uint64_t hash{0};
             uint8_t flags{0};
             uint16_t packet_count{0};
+            std::size_t accumulated_size{0};
             std::chrono::steady_clock::time_point last_update;
             std::map<uint16_t, std::vector<uint8_t>> fragments;
         };

src/nuclear/src/nuclearnet/Fragmentation.hpp

@@ -30,7 +30,7 @@ namespace network {
 
 namespace {
 
-    std::atomic<int> g_log_level{static_cast<int>(LogLevel::Off)};
+    std::atomic<std::uint8_t> g_log_level{static_cast<std::uint8_t>(LogLevel::Off)};
 
     const char* level_name(LogLevel level) {
         switch (level) {
@@ -46,7 +46,7 @@ namespace {
 }  // namespace
 
     void set_log_level(LogLevel level) {
-        g_log_level.store(static_cast<int>(level), std::memory_order_relaxed);
+        g_log_level.store(static_cast<std::uint8_t>(level), std::memory_order_relaxed);
     }
 
     LogLevel get_log_level() {
@@ -67,7 +67,7 @@ namespace {
     }
 
     std::string sock_str(const util::network::sock_t& address) {
-        const auto addr = address.address();
+        const auto addr = address.address(true);
         return addr.first + ":" + std::to_string(addr.second);
     }
 

src/nuclear/src/nuclearnet/Log.cpp

@@ -33,7 +33,7 @@
 namespace NUClear {
 namespace network {
 
-    enum class LogLevel : int {
+    enum class LogLevel : std::uint8_t {
         Off   = 0,
         Error = 1,
         Warn  = 2,
@@ -46,7 +46,7 @@ namespace network {
     LogLevel get_log_level();
 
     inline bool should_log(LogLevel level) {
-        return static_cast<int>(level) <= static_cast<int>(get_log_level());
+        return static_cast<std::uint8_t>(level) <= static_cast<std::uint8_t>(get_log_level());
     }
 
     void log(LogLevel level, const char* component, const std::string& message);