From 4ef7c50a2dd9a0a018396152a06e4267b9295276 Mon Sep 17 00:00:00 2001 From: edef Date: Sat, 3 May 2025 18:20:07 +0000 Subject: [PATCH] tests(nix-daemon/framed): more thorough, failing tests This is mostly a WIP commit, to demonstrate bugs properly. See #120. The tests are marked `#[should_panic]`, since they are intended to fail. Change-Id: I39f1d66742e6629ccb889da8ef1199117b91b126 Reviewed-on: https://cl.snix.dev/c/snix/+/30490 Tested-by: besadii Reviewed-by: Florian Klink --- .../src/nix_daemon/framing/framed_read.rs | 183 ++++++++++++++++-- 1 file changed, 172 insertions(+), 11 deletions(-) diff --git a/snix/nix-compat/src/nix_daemon/framing/framed_read.rs b/snix/nix-compat/src/nix_daemon/framing/framed_read.rs index 320ab64af..309df82cf 100644 --- a/snix/nix-compat/src/nix_daemon/framing/framed_read.rs +++ b/snix/nix-compat/src/nix_daemon/framing/framed_read.rs @@ -10,7 +10,7 @@ use tokio::io::{AsyncRead, ReadBuf}; /// State machine for [`NixFramedReader`]. /// /// As the reader progresses it linearly cycles through the states. -#[derive(Debug)] +#[derive(Debug, PartialEq)] enum NixFramedReaderState { /// The reader always starts in this state. /// @@ -53,6 +53,15 @@ impl NixFramedReader { }, } } + + #[cfg(test)] + fn is_eof(&self) -> bool { + self.state + == NixFramedReaderState::ReadingSize { + buf: [0; 8], + filled: 8, + } + } } impl AsyncRead for NixFramedReader { @@ -136,15 +145,21 @@ impl AsyncRead for NixFramedReader { #[cfg(test)] mod nix_framed_tests { - use std::time::Duration; + use std::{ + cmp::min, + pin::Pin, + task::{self, Poll}, + time::Duration, + }; - use tokio::io::AsyncReadExt; + use tokio::io::{self, AsyncRead, AsyncReadExt, ReadBuf}; use tokio_test::io::Builder; use crate::nix_daemon::framing::NixFramedReader; #[tokio::test] - async fn read_hello_world_in_two_frames() { + #[should_panic] // broken + async fn read_unexpected_eof_after_frame() { let mut mock = Builder::new() // The client sends len .read(&5u64.to_le_bytes()) @@ -154,18 +169,58 @@ mod nix_framed_tests { // Send more data separately .read(&6u64.to_le_bytes()) .read(" world".as_bytes()) + // NOTE: no terminating zero .build(); let mut reader = NixFramedReader::new(&mut mock); - let mut result = String::new(); - reader - .read_to_string(&mut result) - .await - .expect("Could not read into result"); - assert_eq!("hello world", result); + let err = reader.read_to_string(&mut String::new()).await.unwrap_err(); + assert!(!reader.is_eof()); + assert_eq!(err.kind(), io::ErrorKind::UnexpectedEof); } + #[tokio::test] - async fn read_hello_world_in_two_frames_followed_by_zero_sized_frame() { + #[should_panic] // broken + async fn read_unexpected_eof_in_frame() { + let mut mock = Builder::new() + // The client sends len + .read(&5u64.to_le_bytes()) + // Immediately followed by the bytes + .read("hello".as_bytes()) + .wait(Duration::ZERO) + // Send more data separately + .read(&6u64.to_le_bytes()) + .read(" worl".as_bytes()) + // NOTE: we only sent five bytes of data before EOF + .build(); + + let mut reader = NixFramedReader::new(&mut mock); + let err = reader.read_to_string(&mut String::new()).await.unwrap_err(); + assert!(!reader.is_eof()); + assert_eq!(err.kind(), io::ErrorKind::UnexpectedEof); + } + + #[tokio::test] + #[should_panic] // broken + async fn read_unexpected_eof_in_length() { + let mut mock = Builder::new() + // The client sends len + .read(&5u64.to_le_bytes()) + // Immediately followed by the bytes + .read("hello".as_bytes()) + .wait(Duration::ZERO) + // Send a truncated length header + .read(&[0; 7]) + .build(); + + let mut reader = NixFramedReader::new(&mut mock); + let err = reader.read_to_string(&mut String::new()).await.unwrap_err(); + assert!(!reader.is_eof()); + assert_eq!(err.kind(), io::ErrorKind::UnexpectedEof); + } + + #[tokio::test] + #[should_panic] // broken + async fn read_hello_world_in_two_frames() { let mut mock = Builder::new() // The client sends len .read(&5u64.to_le_bytes()) @@ -185,5 +240,111 @@ mod nix_framed_tests { .await .expect("Could not read into result"); assert_eq!("hello world", result); + assert!(reader.is_eof()); + } + + struct SplitMock<'a>(&'a [u8]); + + impl AsyncRead for SplitMock<'_> { + fn poll_read( + mut self: Pin<&mut Self>, + _cx: &mut task::Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + let data = self.0; + + if data.is_empty() { + Poll::Pending + } else { + let n = min(buf.remaining(), data.len()); + buf.put_slice(&data[..n]); + self.0 = &data[n..]; + + Poll::Ready(Ok(())) + } + } + } + + /// Somewhat of a fuzz test, ensuring that we end up in identical states for the same input, + /// independent of how it is spread across read calls and poll cycles. + #[test] + #[should_panic] // broken + fn split_verif() { + let mut cx = task::Context::from_waker(task::Waker::noop()); + let mut input = make_framed(&[b"hello", b"world", b"!", b""]); + let framed_end = input.len(); + input.extend_from_slice(b"trailing data"); + + for end_point in 0..input.len() { + let input = &input[..end_point]; + + let unsplit_res = { + let mut dut = NixFramedReader::new(SplitMock(input)); + let mut data_buf = vec![0; input.len()]; + let mut read_buf = ReadBuf::new(&mut data_buf); + + for _ in 0..256 { + match Pin::new(&mut dut).poll_read(&mut cx, &mut read_buf) { + Poll::Ready(res) => res.unwrap(), + Poll::Pending => break, + } + } + + let len = read_buf.filled().len(); + data_buf.truncate(len); + + assert_eq!( + end_point >= framed_end, + dut.is_eof(), + "end_point = {end_point}, state = {:?}", + dut.state + ); + (dut.state, data_buf, dut.reader.0) + }; + + for split_point in 1..end_point.saturating_sub(1) { + let split_res = { + let mut dut = NixFramedReader::new(SplitMock(&[])); + let mut data_buf = vec![0; input.len()]; + let mut read_buf = ReadBuf::new(&mut data_buf); + + dut.reader.0 = &input[..split_point]; + for _ in 0..256 { + match Pin::new(&mut dut).poll_read(&mut cx, &mut read_buf) { + Poll::Ready(res) => res.unwrap(), + Poll::Pending => break, + } + } + + dut.reader.0 = &input[split_point - dut.reader.0.len()..]; + for _ in 0..256 { + match Pin::new(&mut dut).poll_read(&mut cx, &mut read_buf) { + Poll::Ready(res) => res.unwrap(), + Poll::Pending => break, + } + } + + let len = read_buf.filled().len(); + data_buf.truncate(len); + + (dut.state, data_buf, dut.reader.0) + }; + + assert_eq!(split_res, unsplit_res); + } + } + } + + /// Make framed data, given frame contents. Terminating frame is *not* implicitly included. + /// Include an empty slice explicitly. + fn make_framed(frames: &[&[u8]]) -> Vec { + let mut buf = vec![]; + + for &data in frames { + buf.extend_from_slice(&(data.len() as u64).to_le_bytes()); + buf.extend_from_slice(data); + } + + buf } }