From ab95489f200b83515a85343f12223196d5a87bdf Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Tue, 5 May 2026 00:04:12 +0700 Subject: [PATCH] test(client): add integration tests for packet length and server fill Verify that the client correctly handles custom packet lengths and server fill negotiation to ensure protocol compliance and support for larger MTUs. Add validation to reject empty server fill strings during configuration. --- crates/irtt-cli/src/lib.rs | 11 + crates/irtt-client/src/client.rs | 5 + crates/irtt-client/src/client/tests/config.rs | 12 + crates/irtt-client/tests/large_packets.rs | 140 ++++++++++++ crates/irtt-client/tests/server_fill.rs | 214 ++++++++++++++++++ crates/irtt-client/tests/support/mod.rs | 4 +- 6 files changed, 384 insertions(+), 2 deletions(-) create mode 100644 crates/irtt-client/tests/large_packets.rs create mode 100644 crates/irtt-client/tests/server_fill.rs diff --git a/crates/irtt-cli/src/lib.rs b/crates/irtt-cli/src/lib.rs index 07f5959..dbfe112 100644 --- a/crates/irtt-cli/src/lib.rs +++ b/crates/irtt-cli/src/lib.rs @@ -838,6 +838,17 @@ mod tests { } } + #[test] + fn maps_length_option() { + let args = parse(&["--length", "1472", "127.0.0.1:2112"]).unwrap(); + assert_eq!(args.length, 1472); + assert_eq!(args.to_client_config().length, 1472); + + let args = parse(&["127.0.0.1:2112"]).unwrap(); + assert_eq!(args.length, 0); + assert_eq!(args.to_client_config().length, 0); + } + #[test] fn maps_loose_negotiation() { let args = parse(&["127.0.0.1:2112"]).unwrap(); diff --git a/crates/irtt-client/src/client.rs b/crates/irtt-client/src/client.rs index 276dd8b..251a88b 100644 --- a/crates/irtt-client/src/client.rs +++ b/crates/irtt-client/src/client.rs @@ -775,6 +775,11 @@ fn validate_protocol_config(config: &ClientConfig) -> Result<(), ClientError> { if let Some(fill) = &config.server_fill { let len = fill.len(); + if len == 0 { + return Err(ClientError::InvalidConfig { + reason: "server_fill must not be empty".to_owned(), + }); + } if len > MAX_SERVER_FILL_BYTES { return Err(ClientError::InvalidConfig { reason: format!("server_fill must be <= {MAX_SERVER_FILL_BYTES} bytes, got {len}"), diff --git a/crates/irtt-client/src/client/tests/config.rs b/crates/irtt-client/src/client/tests/config.rs index 790fa72..777f9f6 100644 --- a/crates/irtt-client/src/client/tests/config.rs +++ b/crates/irtt-client/src/client/tests/config.rs @@ -80,6 +80,18 @@ fn params_from_config_rejects_invalid_dscp_codepoint() { )); } +#[test] +fn params_from_config_rejects_empty_server_fill() { + let config = ClientConfig { + server_fill: Some("".to_owned()), + ..ClientConfig::default() + }; + assert!(matches!( + params_from_config(&config), + Err(ClientError::InvalidConfig { .. }) + )); +} + #[test] fn params_from_config_rejects_oversized_server_fill() { let config = ClientConfig { diff --git a/crates/irtt-client/tests/large_packets.rs b/crates/irtt-client/tests/large_packets.rs new file mode 100644 index 0000000..d3cc650 --- /dev/null +++ b/crates/irtt-client/tests/large_packets.rs @@ -0,0 +1,140 @@ +mod support; + +use irtt_client::{Client, ClientConfig, ClientEvent, ClientTimestamp, NegotiationPolicy}; +use irtt_proto::{echo_header_len, echo_packet_len, Params, TimestampFields}; + +use support::{config_for_params, default_params, run_one_probe, BackendPeer, ServerObservation}; + +fn params_with_length(length: i64) -> Params { + Params { + length, + ..default_params() + } +} + +fn assert_echo_size(run: &support::OneProbeRun, expected: usize) { + match run.observations[1] { + ServerObservation::Echo { len, .. } => assert_eq!(len, expected), + ref other => panic!("expected Echo observation, got {other:?}"), + } +} + +fn assert_reply(event: &ClientEvent) { + assert!(matches!(event, ClientEvent::EchoReply { .. })); +} + +fn assert_sent_bytes(event: &ClientEvent, expected: usize) { + match event { + ClientEvent::EchoSent { bytes, .. } => assert_eq!(*bytes, expected), + other => panic!("expected EchoSent, got {other:?}"), + } +} + +fn assert_reply_bytes(event: &ClientEvent, expected: usize) { + match event { + ClientEvent::EchoReply { bytes, .. } => assert_eq!(*bytes, expected), + other => panic!("expected EchoReply, got {other:?}"), + } +} + +// ─── Config/client validation ─── + +#[test] +fn length_zero_baseline() { + let params = params_with_length(0); + let expected = echo_packet_len(false, ¶ms); + let header = echo_header_len(false, ¶ms); + assert_eq!(expected, header); +} + +#[test] +fn length_one_accepted() { + let params = params_with_length(1); + let expected = echo_packet_len(false, ¶ms); + assert!(expected >= 1); +} + +#[test] +fn length_1472_accepted() { + let params = params_with_length(1472); + let expected = echo_packet_len(false, ¶ms); + assert_eq!(expected, 1472); +} + +#[test] +fn length_4096_accepted() { + let params = params_with_length(4096); + let expected = echo_packet_len(false, ¶ms); + assert_eq!(expected, 4096); +} + +// ─── Echo request size: FakeServer observes exact packet size ─── + +#[test] +fn echo_request_size_matches_echo_packet_len_for_zero_length() { + let params = params_with_length(0); + let run = run_one_probe(params.clone(), TimestampFields::default()); + let expected = echo_packet_len(false, ¶ms); + assert_echo_size(&run, expected); + assert_sent_bytes(&run.sent, expected); +} + +#[test] +fn echo_request_size_matches_echo_packet_len_above_header() { + for length in [16, 60, 128, 1472, 4096] { + let params = params_with_length(length); + let run = run_one_probe(params.clone(), TimestampFields::default()); + let expected = echo_packet_len(false, ¶ms); + assert_eq!( + expected, + echo_header_len(false, ¶ms).max(length as usize) + ); + assert_echo_size(&run, expected); + assert_sent_bytes(&run.sent, expected); + } +} + +// ─── Echo reply handling ─── + +#[test] +fn echo_reply_decoded_for_large_length() { + let params = params_with_length(4096); + let run = run_one_probe(params.clone(), TimestampFields::default()); + let expected = echo_packet_len(false, ¶ms); + assert_reply(&run.reply); + assert_reply_bytes(&run.reply, expected); +} + +#[test] +fn echo_reply_bytes_consistent_with_echo_packet_len_for_various_lengths() { + for length in [0, 1472, 4096] { + let params = params_with_length(length); + let run = run_one_probe(params.clone(), TimestampFields::default()); + let expected = echo_packet_len(false, ¶ms); + assert_reply_bytes(&run.reply, expected); + } +} + +// ─── Backend-neutral smoke ─── + +#[test] +fn backend_large_packet_smoke() { + let params = params_with_length(1472); + let peer = BackendPeer::start_open_echo(params.clone(), None); + let mut client = Client::connect(ClientConfig { + negotiation_policy: NegotiationPolicy::Loose, + length: 1472, + ..config_for_params(peer.addr(), ¶ms) + }) + .unwrap(); + + let outcome = client.open(ClientTimestamp::now()).unwrap(); + assert!(matches!(outcome, irtt_client::OpenOutcome::Started { .. })); + + client.send_probe().unwrap(); + let events = client.recv_once().unwrap(); + assert_eq!(events.len(), 1); + assert!(matches!(events[0], ClientEvent::EchoReply { .. })); + + client.close(ClientTimestamp::now()).unwrap(); +} diff --git a/crates/irtt-client/tests/server_fill.rs b/crates/irtt-client/tests/server_fill.rs new file mode 100644 index 0000000..7a453fa --- /dev/null +++ b/crates/irtt-client/tests/server_fill.rs @@ -0,0 +1,214 @@ +mod support; + +use irtt_client::{ + Client, ClientConfig, ClientError, ClientEvent, ClientTimestamp, NegotiationPolicy, +}; +use irtt_proto::{echo_packet_len, Params, TimestampFields}; + +use support::{ + config_for_params, default_params, run_one_probe, run_one_probe_with_config, server_fill, + start_open_server, OneProbeRun, ServerObservation, +}; + +fn open_observation_params(run: &OneProbeRun) -> &Params { + match &run.observations[0] { + ServerObservation::Open { params, .. } => params, + other => panic!("expected Open observation, got {other:?}"), + } +} + +fn echo_observation(run: &OneProbeRun) -> (usize, bool, u64, u32) { + match run.observations[1] { + ServerObservation::Echo { + len, + hmac, + token, + sequence, + } => (len, hmac, token, sequence), + ref other => panic!("expected Echo observation, got {other:?}"), + } +} + +// ─── Config mapping ─── + +#[test] +fn server_fill_none_produces_no_fill_in_open_params() { + let params = default_params(); + let run = run_one_probe(params, TimestampFields::default()); + assert!(open_observation_params(&run).server_fill.is_none()); +} + +#[test] +fn server_fill_short_value_maps_into_open_params() { + let mut params = default_params(); + params.server_fill = server_fill("abc"); + let run = run_one_probe(params, TimestampFields::default()); + assert_eq!( + open_observation_params(&run) + .server_fill + .as_ref() + .map(|f| f.value.as_str()), + Some("abc") + ); +} + +#[test] +fn server_fill_32_byte_value_maps_into_open_params() { + let mut params = default_params(); + let fill = "0123456789abcdef0123456789abcdef"; + params.server_fill = server_fill(fill); + let run = run_one_probe(params, TimestampFields::default()); + assert_eq!( + open_observation_params(&run) + .server_fill + .as_ref() + .map(|f| f.value.as_str()), + Some(fill) + ); +} + +#[test] +fn server_fill_over_32_bytes_rejected_at_config_boundary() { + let config = ClientConfig { + server_fill: Some("0123456789abcdef0123456789abcdefx".to_owned()), + ..ClientConfig::default() + }; + assert!(matches!( + Client::connect(config), + Err(ClientError::InvalidConfig { .. }) + )); +} + +#[test] +fn server_fill_empty_rejected_at_config_boundary() { + let config = ClientConfig { + server_fill: Some("".to_owned()), + ..ClientConfig::default() + }; + assert!(matches!( + Client::connect(config), + Err(ClientError::InvalidConfig { .. }) + )); +} + +// ─── Open negotiation ─── + +#[test] +fn server_fill_negotiated_params_reflect_accepted_value() { + let mut params = default_params(); + params.server_fill = server_fill("rand"); + let run = run_one_probe(params.clone(), TimestampFields::default()); + assert_eq!(run.negotiated.params.server_fill, params.server_fill); +} + +#[test] +fn server_fill_strict_rejects_removed_fill() { + let mut requested = default_params(); + requested.server_fill = server_fill("rand"); + + let mut returned = requested.clone(); + returned.server_fill = None; + + let server = start_open_server(returned, None); + let mut client = Client::connect(config_for_params(server.addr, &requested)).unwrap(); + assert!(matches!( + client.open(ClientTimestamp::now()), + Err(ClientError::NegotiationRejected { .. }) + )); + server.join(); +} + +#[test] +fn server_fill_strict_rejects_different_fill_value() { + let mut requested = default_params(); + requested.server_fill = server_fill("abc"); + + let mut returned = requested.clone(); + returned.server_fill = server_fill("xyz"); + + let server = start_open_server(returned, None); + let mut client = Client::connect(config_for_params(server.addr, &requested)).unwrap(); + assert!(matches!( + client.open(ClientTimestamp::now()), + Err(ClientError::NegotiationRejected { .. }) + )); + server.join(); +} + +#[test] +fn server_fill_strict_rejects_unexpected_fill_from_server() { + let requested = default_params(); + + let mut returned = requested.clone(); + returned.server_fill = server_fill("unexpected"); + + let server = start_open_server(returned, None); + let mut client = Client::connect(config_for_params(server.addr, &requested)).unwrap(); + assert!(matches!( + client.open(ClientTimestamp::now()), + Err(ClientError::NegotiationRejected { .. }) + )); + server.join(); +} + +#[test] +fn server_fill_loose_allows_server_to_remove_fill() { + let mut requested = default_params(); + requested.server_fill = server_fill("rand"); + + let mut returned = requested.clone(); + returned.server_fill = None; + + let requested_for_config = requested.clone(); + let run = run_one_probe_with_config(returned, TimestampFields::default(), None, |addr| { + ClientConfig { + negotiation_policy: NegotiationPolicy::Loose, + ..config_for_params(addr, &requested_for_config) + } + }); + + assert_eq!( + open_observation_params(&run).server_fill, + requested.server_fill + ); + assert_eq!(run.negotiated.params.server_fill, None); +} + +// ─── Echo behavior ─── + +#[test] +fn server_fill_echo_reply_decoded_with_fill_configured() { + let mut params = default_params(); + params.server_fill = server_fill("rand"); + let run = run_one_probe(params, TimestampFields::default()); + assert!(matches!(run.reply, ClientEvent::EchoReply { .. })); +} + +// server_fill is a negotiation/config parameter only; it does not affect +// client-observable reply payload layout. The client sends server_fill in +// the open request and expects the server to use it for reply payload +// content, but the client's decode/echo layout logic is independent of +// server_fill. Only the length parameter expands the packet beyond the +// protocol header. +#[test] +fn server_fill_does_not_affect_client_side_packet_layout() { + let mut params_with_fill = default_params(); + params_with_fill.server_fill = server_fill("rand"); + + let params_without_fill = default_params(); + + let run_with = run_one_probe(params_with_fill, TimestampFields::default()); + let run_without = run_one_probe(params_without_fill, TimestampFields::default()); + + let (echo_len_with, _, _, _) = echo_observation(&run_with); + let (echo_len_without, _, _, _) = echo_observation(&run_without); + assert_eq!(echo_len_with, echo_len_without); + assert_eq!( + echo_packet_len(false, &run_with.negotiated.params), + echo_len_with + ); + assert_eq!( + echo_packet_len(false, &run_without.negotiated.params), + echo_len_without + ); +} diff --git a/crates/irtt-client/tests/support/mod.rs b/crates/irtt-client/tests/support/mod.rs index 9e1ce0c..16d1f3b 100644 --- a/crates/irtt-client/tests/support/mod.rs +++ b/crates/irtt-client/tests/support/mod.rs @@ -413,13 +413,13 @@ where } fn recv_request(socket: &UdpSocket) -> (Vec, SocketAddr) { - let mut buf = [0_u8; 2048]; + let mut buf = [0_u8; 8192]; let (size, peer) = socket.recv_from(&mut buf).unwrap(); (buf[..size].to_vec(), peer) } fn recv_request_timeout(socket: &UdpSocket) -> Option> { - let mut buf = [0_u8; 2048]; + let mut buf = [0_u8; 8192]; match socket.recv_from(&mut buf) { Ok((size, _)) => Some(buf[..size].to_vec()), Err(_) => None,