diff --git a/.gitignore b/.gitignore index ea8c4bf..cb1ddd8 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /target +/dhat-heap.json diff --git a/src/codec.rs b/src/codec.rs index e8c8f03..0a6fa00 100644 --- a/src/codec.rs +++ b/src/codec.rs @@ -10,6 +10,17 @@ use crate::{ Packet, }; +/// MAVLink packet codec whose behavior is selected at compile time through +/// const-generic toggles. +/// +/// The toggles are, in order: +/// +/// * `ACCEPT_V1` -- accept MAVLink v1 frames. +/// * `ACCEPT_V2` -- accept MAVLink v2 frames. +/// * `DROP_INVALID_SYSID` -- reject frames whose system id equals zero. +/// * `DROP_INVALID_COMPID` -- reject frames whose component id equals zero. +/// * `SKIP_CRC_VALIDATION` -- skip **only** the CRC computation step. +/// * `DROP_INCOMPATIBLE` -- reject v2 frames with unsupported incompat flags. #[derive(Debug, Default)] pub struct MavlinkCodec< const ACCEPT_V1: bool, @@ -71,12 +82,6 @@ impl< trace!("Waitig for STX..."); if buf.is_empty() { - if ACCEPT_V2 { - // buf.reserve(V2Packet::MAX_PACKET_SIZE); - } else { - // buf.reserve(V1Packet::MAX_PACKET_SIZE); - } - trace!( "Not enough data, buf.len: {:?}, buf.capacity: {:?}", buf.len(), @@ -98,8 +103,6 @@ impl< // V1 Codec CodecState::WaitingV1PacketHeader if ACCEPT_V1 => { if buf.len() < V1Packet::HEADER_SIZE { - // buf.reserve(V1Packet::HEADER_SIZE); - trace!( "Not enough data, buf.len: {:?}, buf.capacity: {:?}", buf.len(), @@ -113,8 +116,6 @@ impl< } CodecState::ValidatingV1Packet { packet_size } if ACCEPT_V1 => { if buf.len() < packet_size { - // buf.reserve(V1Packet::MAX_PACKET_SIZE); - trace!( "Not enough data, buf.len: {:?}, buf.capacity: {:?}", buf.len(), @@ -186,22 +187,7 @@ impl< self.state = CodecState::CopyV1Packet { packet_size }; } CodecState::CopyV1Packet { packet_size } if ACCEPT_V1 => { - let buf_packet = if SKIP_CRC_VALIDATION { - // Copy the entire packet consuming the source buffer - let mut buf_packet = BytesMut::with_capacity(packet_size); - buf_packet[..packet_size].copy_from_slice(&buf[..packet_size]); - - // Since it is a non validated packet, there might be other packets within this buffer, so we can only discard this STX - buf.advance(V1Packet::STX_SIZE); - - buf_packet - } else { - let buf_packet = buf.split_to(packet_size); - // buf.reserve(V1Packet::MAX_PACKET_SIZE); - - buf_packet - }; - + let buf_packet = buf.split_to(packet_size); let packet = V1Packet { buffer: buf_packet.freeze(), }; @@ -212,8 +198,6 @@ impl< // V2 Codec CodecState::WaitingV2PacketHeader if ACCEPT_V2 => { if buf.len() < V2Packet::HEADER_SIZE { - // buf.reserve(V2Packet::HEADER_SIZE); - trace!( "Not enough data, buf.len: {:?}, buf.capacity: {:?}", buf.len(), @@ -225,7 +209,7 @@ impl< if DROP_INCOMPATIBLE { let incompat_flags = *v2::incompat_flags(buf); if incompat_flags & !MAVLINK_SUPPORTED_IFLAGS > 0 { - buf.advance(V1Packet::STX_SIZE); // Discard this STX + buf.advance(V2Packet::STX_SIZE); // Discard this STX self.state = CodecState::WaitingForStx; return Ok(Some(Err(DecoderError::Incompatible { incompat_flags }))); @@ -237,8 +221,6 @@ impl< } CodecState::ValidatingV2Packet { packet_size } if ACCEPT_V2 => { if buf.len() < packet_size { - // buf.reserve(V2Packet::MAX_PACKET_SIZE); - trace!( "Not enough data, buf.len: {:?}, buf.capacity: {:?}", buf.len(), @@ -310,21 +292,7 @@ impl< self.state = CodecState::CopyV2Packet { packet_size }; } CodecState::CopyV2Packet { packet_size } if ACCEPT_V2 => { - let buf_packet = if SKIP_CRC_VALIDATION { - // Copy the entire packet consuming the source buffer - let mut buf_packet = BytesMut::with_capacity(packet_size); - buf_packet[..packet_size].copy_from_slice(&buf[..packet_size]); - - // Since it is a non validated packet, there might be other packets within this buffer, so we can only discard this STX - buf.advance(V2Packet::STX_SIZE); - - buf_packet - } else { - let buf_packet = buf.split_to(packet_size); - // buf.reserve(V2Packet::MAX_PACKET_SIZE); - - buf_packet - }; + let buf_packet = buf.split_to(packet_size); let packet = V2Packet { buffer: buf_packet.freeze(), diff --git a/src/v2.rs b/src/v2.rs index da8214d..9c63b3b 100644 --- a/src/v2.rs +++ b/src/v2.rs @@ -175,9 +175,11 @@ pub(crate) fn packet_size>(buf: &T) -> usize { let header = V2Packet::HEADER_SIZE; let payload = *len(buf) as usize; let checksum = V2Packet::CHECKSUM_SIZE; - let signature = has_signature(buf) - .then_some(V2Packet::SIGNATURE_SIZE) - .unwrap_or_default(); + let signature = if has_signature(buf) { + V2Packet::SIGNATURE_SIZE + } else { + Default::default() + }; stx + header + payload + checksum + signature } diff --git a/tests/skip_crc_test.rs b/tests/skip_crc_test.rs new file mode 100644 index 0000000..24a9434 --- /dev/null +++ b/tests/skip_crc_test.rs @@ -0,0 +1,139 @@ +use bytes::BytesMut; +use dev_utils::{create_random_v1_raw_message, create_random_v2_raw_message}; +use mavlink_codec::{codec::MavlinkCodec, error::DecoderError, Packet}; +use rand::{rngs::StdRng, SeedableRng}; +use tokio_util::codec::Decoder; + +const SEED: u64 = 42; + +type SkipV1Codec = MavlinkCodec; +type SkipV2Codec = MavlinkCodec; +type StrictV1Codec = MavlinkCodec; +type StrictV2Codec = MavlinkCodec; + +fn corrupt_crc(buf: &mut [u8]) { + let len = buf.len(); + assert!(len >= 2, "packet must have at least two bytes to corrupt"); + buf[len - 2] = buf[len - 2].wrapping_add(1); + buf[len - 1] = buf[len - 1].wrapping_add(1); +} + +#[test] +fn skip_crc_v1_decodes_valid_packet() { + let mut rng: StdRng = SeedableRng::seed_from_u64(SEED); + let raw = create_random_v1_raw_message(&mut rng); + let total = raw.raw_bytes().len(); + let mut buf = BytesMut::from(raw.raw_bytes()); + + let mut codec = SkipV1Codec::default(); + let decoded = codec.decode(&mut buf).unwrap(); + + assert!( + matches!(decoded, Some(Ok(Packet::V1(_)))), + "expected Packet::V1, got {decoded:?}" + ); + assert!( + buf.is_empty(), + "F3: {} of {total} bytes remained after a single successful decode", + buf.len() + ); + assert!( + codec.decode(&mut buf).unwrap().is_none(), + "F3: decoder emitted a ghost packet from bytes that should already have been consumed" + ); +} + +#[test] +fn skip_crc_v2_decodes_valid_packet() { + let mut rng: StdRng = SeedableRng::seed_from_u64(SEED); + let raw = create_random_v2_raw_message(&mut rng); + let total = raw.raw_bytes().len(); + let mut buf = BytesMut::from(raw.raw_bytes()); + + let mut codec = SkipV2Codec::default(); + let decoded = codec.decode(&mut buf).unwrap(); + + assert!( + matches!(decoded, Some(Ok(Packet::V2(_)))), + "expected Packet::V2, got {decoded:?}" + ); + assert!( + buf.is_empty(), + "F3: {} of {total} bytes remained after a single successful decode", + buf.len() + ); + assert!( + codec.decode(&mut buf).unwrap().is_none(), + "F3: decoder emitted a ghost packet from bytes that should already have been consumed" + ); +} + +#[test] +fn skip_crc_v1_accepts_corrupted_crc() { + let mut rng: StdRng = SeedableRng::seed_from_u64(SEED); + let raw = create_random_v1_raw_message(&mut rng); + + let mut corrupted: Vec = raw.raw_bytes().to_vec(); + corrupt_crc(&mut corrupted); + + // Skip codec must accept despite the broken CRC. + { + let mut buf = BytesMut::from(corrupted.as_slice()); + let mut codec = SkipV1Codec::default(); + let decoded = codec.decode(&mut buf).unwrap(); + assert!( + matches!(decoded, Some(Ok(Packet::V1(_)))), + "skip-CRC codec must accept a packet with corrupted CRC, got {decoded:?}" + ); + assert!( + buf.is_empty(), + "F3: {} bytes remained after decoding a corrupted-CRC packet under SKIP_CRC_VALIDATION", + buf.len() + ); + } + + // Non-skip codec must reject with InvalidCRC, proving the toggle's scope. + { + let mut buf = BytesMut::from(corrupted.as_slice()); + let mut codec = StrictV1Codec::default(); + let decoded = codec.decode(&mut buf).unwrap(); + assert!( + matches!(decoded, Some(Err(DecoderError::InvalidCRC { .. }))), + "strict codec must reject corrupted CRC, got {decoded:?}" + ); + } +} + +#[test] +fn skip_crc_v2_accepts_corrupted_crc() { + let mut rng: StdRng = SeedableRng::seed_from_u64(SEED); + let raw = create_random_v2_raw_message(&mut rng); + + let mut corrupted: Vec = raw.raw_bytes().to_vec(); + corrupt_crc(&mut corrupted); + + { + let mut buf = BytesMut::from(corrupted.as_slice()); + let mut codec = SkipV2Codec::default(); + let decoded = codec.decode(&mut buf).unwrap(); + assert!( + matches!(decoded, Some(Ok(Packet::V2(_)))), + "skip-CRC codec must accept a packet with corrupted CRC, got {decoded:?}" + ); + assert!( + buf.is_empty(), + "F3: {} bytes remained after decoding a corrupted-CRC packet under SKIP_CRC_VALIDATION", + buf.len() + ); + } + + { + let mut buf = BytesMut::from(corrupted.as_slice()); + let mut codec = StrictV2Codec::default(); + let decoded = codec.decode(&mut buf).unwrap(); + assert!( + matches!(decoded, Some(Err(DecoderError::InvalidCRC { .. }))), + "strict codec must reject corrupted CRC, got {decoded:?}" + ); + } +}