fix: address fragmentation v1 protocol violations and documentation gaps

lib-network/src/blockchain_sync/mod.rs

@@ -55,9 +55,14 @@ use std::time::Duration;
 #[deprecated(since = "0.1.0", note = "Use crate::mtu::BLE_CHUNK_SIZE instead")]
 pub const BLE_CHUNK_SIZE_COMPAT: usize = BLE_CHUNK_SIZE;
 
+/// Backward compatibility: Direct re-export of BLE_CHUNK_SIZE
+#[deprecated(since = "0.1.0", note = "Use crate::mtu::BLE_CHUNK_SIZE instead")]
+pub use crate::mtu::BLE_CHUNK_SIZE;
+
 #[deprecated(since = "0.1.0", note = "Use crate::mtu::BLUETOOTH_CLASSIC_CHUNK_SIZE instead")]
 pub const CLASSIC_CHUNK_SIZE: usize = BLUETOOTH_CLASSIC_CHUNK_SIZE;
 
+/// Backward compatibility: Direct re-export of WIFI_CHUNK_SIZE  
 #[deprecated(since = "0.1.0", note = "Use crate::mtu::WIFI_DIRECT_CHUNK_SIZE instead")]
 pub const WIFI_CHUNK_SIZE: usize = WIFI_DIRECT_CHUNK_SIZE;
 

lib-network/src/fragmentation.rs

@@ -83,6 +83,15 @@ impl FragmentHeader {
         let total_fragments = u16::from_le_bytes([bytes[4], bytes[5]]);
         let fragment_index = u16::from_le_bytes([bytes[6], bytes[7]]);
 
+        // Validate fragment_index is within bounds
+        if fragment_index >= total_fragments {
+            return Err(anyhow!(
+                "Invalid fragment: index {} >= total_fragments {}",
+                fragment_index,
+                total_fragments
+            ));
+        }
+
         Ok(Self {
             message_id,
             total_fragments,
@@ -137,6 +146,26 @@ impl Fragment {
 
 /// Fragment a message into chunks suitable for transmission
 ///
+/// This function splits a payload into multiple fragments, each with an 8-byte header
+/// containing sequence information for reassembly.
+///
+/// ## Message ID Generation - Collision Risk ⚠️
+///
+/// **WARNING**: The message_id is generated using a simple wrapping sum of payload bytes:
+/// ```ignore
+/// message_id = payload.iter().fold(0u32, |acc, &b| acc.wrapping_add(b as u32))
+/// ```
+///
+/// This algorithm is **NOT cryptographically secure** and can easily produce collisions:
+/// - Two different messages with the same byte sum will get the same message_id
+/// - Messages sent in the same session could have their fragments mixed during reassembly
+/// - Attackers could craft messages to intentionally cause collisions
+///
+/// **For production use, prefer `fragmentation_v2` which provides:**
+/// - Session-scoped identity `(session_id, message_seq, fragment_index)`
+/// - Sequential message_seq counters (zero collisions)
+/// - Proper isolation between concurrent sessions
+///
 /// ## Arguments
 ///
 /// - `payload`: The message data to fragment
@@ -160,6 +189,7 @@ pub fn fragment_message(payload: &[u8], chunk_size: usize) -> Vec<Fragment> {
     }
 
     // Generate unique message ID (use hash of payload for determinism)
+    // WARNING: Simple wrapping sum is NOT collision-resistant - see function docs
     let message_id = payload.iter().fold(0u32, |acc, &b| acc.wrapping_add(b as u32));
 
     let total_fragments = ((payload.len() + chunk_size - 1) / chunk_size) as u16;

lib-network/src/protocols/bluetooth/gatt.rs

@@ -126,8 +126,27 @@ pub fn fragment_data(data: &[u8], mtu: u16) -> Vec<Vec<u8>> {
 /// **REFACTORED**: Now uses centralized fragmentation with standardized 8-byte headers.
 /// Returns Vec of wire-format fragments ready for GATT transmission.
 /// 
+/// ## Wire Format Compatibility Warning
+/// 
+/// **BREAKING CHANGE**: The wire format has changed from 12-byte headers (old format)
+/// to 8-byte headers (new v1 format):
+/// 
+/// - **Old format** (12 bytes): `message_id: u64, total_fragments: u16, fragment_index: u16`
+/// - **New v1 format** (8 bytes): `message_id: u32, total_fragments: u16, fragment_index: u16`
+/// 
+/// This change is **NOT backward compatible**. Old clients will fail to parse fragments
+/// created by this function, and vice versa. For production deployments requiring 
+/// compatibility, use `fragment_large_message_v2` which includes protocol versioning.
+/// 
+/// ## Message ID Parameter
+/// 
+/// **Note**: The `message_id` parameter (u64) is currently **ignored**. Message IDs are
+/// generated internally as u32 by the centralized fragmentation logic. Callers should
+/// not rely on a specific `message_id` being preserved. This breaks backward compatibility
+/// with code that depends on specific message IDs.
+/// 
 /// **DEPRECATED**: Use `fragment_large_message_v2` with a GattSession for new code.
-pub fn fragment_large_message(message_id: u64, data: &[u8], mtu: u16) -> Vec<Vec<u8>> {
+pub fn fragment_large_message(_message_id: u64, data: &[u8], mtu: u16) -> Vec<Vec<u8>> {
     // ATT overhead is 3 bytes, leave room for our 8-byte header
     let chunk_size = (mtu as usize).saturating_sub(3 + 8).max(20);
 
@@ -205,18 +224,26 @@ impl FragmentReassembler {
         let result = self.inner.add_fragment(parsed)?;
 
         if let Some(ref data) = result {
-            info!("✅ Reassembled message from {} fragments ({} bytes)", 
-                self.inner.pending_count(), data.len());
+            info!("✅ Reassembled complete message ({} bytes)", data.len());
         }
 
         Ok(result)
     }
 
     /// Clear stale fragments older than timeout
-    pub fn cleanup_stale_fragments(&mut self, _message_id: u64) {
-        // Clear all pending (centralized reassembler doesn't track individual message cleanup)
-        self.inner.clear();
-        warn!("🗑️ Cleaned up all stale fragments");
+    /// 
+    /// **Note**: The centralized v1 reassembler does not support per-message cleanup.
+    /// This method only logs the request. To avoid disrupting reassembly of valid
+    /// in-progress messages, use v2 fragmentation with GattSession which provides
+    /// automatic timeout-based cleanup.
+    pub fn cleanup_stale_fragments(&mut self, message_id: u64) {
+        // Centralized reassembler currently does not support per-message cleanup.
+        // To avoid disrupting reassembly of other in-progress messages, we only log for now.
+        debug!(
+            "Requested cleanup of stale fragments for message_id {}, \
+             but centralized reassembler does not support targeted cleanup; no action taken",
+            message_id
+        );
     }
 }