From 193db2118809ca39b0237e16934cdada4f55a32e Mon Sep 17 00:00:00 2001 From: plebhash Date: Wed, 8 Jan 2025 19:25:25 -0300 Subject: [PATCH 1/3] rename InterceptMessage.response_message to replacement_message.. this is a misleading name, as this is not really a response, but rather the new message to replace the intercepted one --- roles/tests-integration/lib/sniffer.rs | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/roles/tests-integration/lib/sniffer.rs b/roles/tests-integration/lib/sniffer.rs index 6926ccca5..181486456 100644 --- a/roles/tests-integration/lib/sniffer.rs +++ b/roles/tests-integration/lib/sniffer.rs @@ -47,10 +47,10 @@ enum SnifferError { /// In order to alter the messages sent between the roles, the [`Sniffer::intercept_messages`] /// field can be used. It will look for the [`InterceptMessage::expected_message_type`] in the /// specified [`InterceptMessage::direction`] and replace it with -/// [`InterceptMessage::response_message`]. +/// [`InterceptMessage::replacement_message`]. /// /// If `break_on` is set to `true`, the [`Sniffer`] will stop the communication after sending the -/// response message. +/// new message. /// /// Can be useful for testing purposes, as it allows to assert that the roles have sent specific /// messages in a specific order and to inspect the messages details. @@ -69,8 +69,8 @@ pub struct Sniffer { pub struct InterceptMessage { direction: MessageDirection, expected_message_type: MsgType, - response_message: PoolMessages<'static>, - response_message_type: MsgType, + replacement_message: PoolMessages<'static>, + replacement_message_type: MsgType, break_on: bool, } @@ -78,15 +78,15 @@ impl InterceptMessage { pub fn new( direction: MessageDirection, expected_message_type: MsgType, - response_message: PoolMessages<'static>, - response_message_type: MsgType, + replacement_message: PoolMessages<'static>, + replacement_message_type: MsgType, break_on: bool, ) -> Self { Self { direction, expected_message_type, - response_message, - response_message_type, + replacement_message, + replacement_message_type, break_on, } } @@ -234,15 +234,15 @@ impl Sniffer { let channel_msg = false; let frame = StandardEitherFrame::>::Sv2( Sv2Frame::from_message( - intercept_message.response_message.clone(), - intercept_message.response_message_type, + intercept_message.replacement_message.clone(), + intercept_message.replacement_message_type, extension_type, channel_msg, ) .expect("Failed to create the frame"), ); downstream_messages - .add_message(msg_type, intercept_message.response_message.clone()); + .add_message(msg_type, intercept_message.replacement_message.clone()); let _ = send.send(frame).await; if intercept_message.break_on { return Err(SnifferError::MessageInterrupted); @@ -276,15 +276,15 @@ impl Sniffer { let channel_msg = false; let frame = StandardEitherFrame::>::Sv2( Sv2Frame::from_message( - intercept_message.response_message.clone(), - intercept_message.response_message_type, + intercept_message.replacement_message.clone(), + intercept_message.replacement_message_type, extension_type, channel_msg, ) .expect("Failed to create the frame"), ); upstream_messages - .add_message(msg_type, intercept_message.response_message.clone()); + .add_message(msg_type, intercept_message.replacement_message.clone()); let _ = send.send(frame).await; if intercept_message.break_on { return Err(SnifferError::MessageInterrupted); From 98de4d793a41bcbebfdf17fdc260abffe1282ed7 Mon Sep 17 00:00:00 2001 From: plebhash Date: Thu, 9 Jan 2025 10:52:15 -0300 Subject: [PATCH 2/3] remove interrupt functionality from `Sniffer`.. it's not fully designed and motivation is not clear, we can re-design if needed --- roles/tests-integration/lib/sniffer.rs | 17 ----------------- .../tests/sniffer_integration.rs | 11 +++++------ 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/roles/tests-integration/lib/sniffer.rs b/roles/tests-integration/lib/sniffer.rs index 181486456..5ac716d9e 100644 --- a/roles/tests-integration/lib/sniffer.rs +++ b/roles/tests-integration/lib/sniffer.rs @@ -31,7 +31,6 @@ type MsgType = u8; enum SnifferError { DownstreamClosed, UpstreamClosed, - MessageInterrupted, } /// Allows to intercept messages sent between two roles. @@ -49,9 +48,6 @@ enum SnifferError { /// specified [`InterceptMessage::direction`] and replace it with /// [`InterceptMessage::replacement_message`]. /// -/// If `break_on` is set to `true`, the [`Sniffer`] will stop the communication after sending the -/// new message. -/// /// Can be useful for testing purposes, as it allows to assert that the roles have sent specific /// messages in a specific order and to inspect the messages details. #[derive(Debug, Clone)] @@ -71,7 +67,6 @@ pub struct InterceptMessage { expected_message_type: MsgType, replacement_message: PoolMessages<'static>, replacement_message_type: MsgType, - break_on: bool, } impl InterceptMessage { @@ -80,14 +75,12 @@ impl InterceptMessage { expected_message_type: MsgType, replacement_message: PoolMessages<'static>, replacement_message_type: MsgType, - break_on: bool, ) -> Self { Self { direction, expected_message_type, replacement_message, replacement_message_type, - break_on, } } } @@ -244,11 +237,6 @@ impl Sniffer { downstream_messages .add_message(msg_type, intercept_message.replacement_message.clone()); let _ = send.send(frame).await; - if intercept_message.break_on { - return Err(SnifferError::MessageInterrupted); - } else { - continue; - } } } @@ -286,11 +274,6 @@ impl Sniffer { upstream_messages .add_message(msg_type, intercept_message.replacement_message.clone()); let _ = send.send(frame).await; - if intercept_message.break_on { - return Err(SnifferError::MessageInterrupted); - } else { - continue; - } } } if send.send(frame).await.is_err() { diff --git a/roles/tests-integration/tests/sniffer_integration.rs b/roles/tests-integration/tests/sniffer_integration.rs index 84b3294b4..9f02a5683 100644 --- a/roles/tests-integration/tests/sniffer_integration.rs +++ b/roles/tests-integration/tests/sniffer_integration.rs @@ -11,9 +11,9 @@ use sniffer::{InterceptMessage, MessageDirection}; use std::convert::TryInto; #[tokio::test] -async fn test_sniffer_interrupter() { +async fn test_sniffer_intercept() { let (_tp, tp_addr) = start_template_provider(None).await; - let message = + let message_replacement = PoolMessages::Common(CommonMessages::SetupConnectionError(SetupConnectionError { flags: 0, error_code: "unsupported-feature-flags" @@ -22,15 +22,14 @@ async fn test_sniffer_interrupter() { .try_into() .unwrap(), })); - let interrupt_msgs = InterceptMessage::new( + let intercept = InterceptMessage::new( MessageDirection::ToDownstream, MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS, - message, + message_replacement, MESSAGE_TYPE_SETUP_CONNECTION_ERROR, - true, ); let (sniffer, sniffer_addr) = - start_sniffer("".to_string(), tp_addr, false, Some(vec![interrupt_msgs])).await; + start_sniffer("".to_string(), tp_addr, false, Some(vec![intercept])).await; let _ = start_pool(Some(sniffer_addr)).await; assert_common_message!(&sniffer.next_message_from_downstream(), SetupConnection); assert_common_message!(&sniffer.next_message_from_upstream(), SetupConnectionError); From 20ffef2fdd1add580c41ed53afd6ef87f39605c2 Mon Sep 17 00:00:00 2001 From: plebhash Date: Thu, 9 Jan 2025 11:05:45 -0300 Subject: [PATCH 3/3] expand `test_sniffer_intercept` to make test comprehensive.. we need 2 sniffers to assert the message was correctly replaced and fully delivered over the TCP connection --- .../tests/sniffer_integration.rs | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/roles/tests-integration/tests/sniffer_integration.rs b/roles/tests-integration/tests/sniffer_integration.rs index 9f02a5683..06427f0ec 100644 --- a/roles/tests-integration/tests/sniffer_integration.rs +++ b/roles/tests-integration/tests/sniffer_integration.rs @@ -10,6 +10,10 @@ use roles_logic_sv2::{ use sniffer::{InterceptMessage, MessageDirection}; use std::convert::TryInto; +// this test aims to assert that Sniffer is able to intercept and replace some message +// sniffer_a replaces a SetupConnectionSuccess from TP with a SetupConnectionError directed at Pool +// sniffer_b asserts that Pool is about to receive a SetupConnectionError +// TP -> sniffer_a -> sniffer_b -> Pool #[tokio::test] async fn test_sniffer_intercept() { let (_tp, tp_addr) = start_template_provider(None).await; @@ -28,11 +32,23 @@ async fn test_sniffer_intercept() { message_replacement, MESSAGE_TYPE_SETUP_CONNECTION_ERROR, ); - let (sniffer, sniffer_addr) = - start_sniffer("".to_string(), tp_addr, false, Some(vec![intercept])).await; - let _ = start_pool(Some(sniffer_addr)).await; - assert_common_message!(&sniffer.next_message_from_downstream(), SetupConnection); - assert_common_message!(&sniffer.next_message_from_upstream(), SetupConnectionError); + + // this sniffer will replace SetupConnectionSuccess with SetupConnectionError + let (_sniffer_a, sniffer_a_addr) = + start_sniffer("A".to_string(), tp_addr, false, Some(vec![intercept])).await; + + // this sniffer will assert SetupConnectionSuccess was correctly replaced with + // SetupConnectionError + let (sniffer_b, sniffer_b_addr) = + start_sniffer("B".to_string(), sniffer_a_addr, false, None).await; + + let _ = start_pool(Some(sniffer_b_addr)).await; + + // assert sniffer_a functionality of replacing messages work as expected (goal of this test) + assert_common_message!( + &sniffer_b.next_message_from_upstream(), + SetupConnectionError + ); } #[tokio::test]