Repository: fujiapple852/trippy (language: rust)

Issue:
range start index 4 out of range
**Describe the bug**
thread 'tracer-11332' panicked at crates\trippy-core\src\state.rs:636:59:
range start index 4 out of range for slice of length 2
stack backtrace:
   0:     0x7ff7b2badfb3 - <unknown>
   1:     0x7ff7b29e486a - <unknown>
   2:     0x7ff7b2b9d3e7 - <unknown>
   3:     0x7ff7b2baf9a5 - <unknown>
   4:     0x7ff7b2baf7dd - <unknown>
   5:     0x7ff7b2baf4ed - <unknown>
   6:     0x7ff7b2bafbb1 - <unknown>
   7:     0x7ff7b2bafa89 - <unknown>
   8:     0x7ff7b2baf9df - <unknown>
   9:     0x7ff7b2baf9ce - <unknown>
  10:     0x7ff7b2e9b5d1 - <unknown>
  11:     0x7ff7b2e9b53e - <unknown>
  12:     0x7ff7b2c575d8 - <unknown>
  13:     0x7ff7b2c3e6ce - <unknown>
  14:     0x7ff7b2c4bf93 - <unknown>
  15:     0x7ff7b2c5356e - <unknown>
  16:     0x7ff7b2c530d2 - <unknown>
  17:     0x7ff7b2bad54d - <unknown>
  18:     0x7ffaf1ce7034 - BaseThreadInitThunk
  19:     0x7ffaf28a2651 - RtlUserThreadStart
**To Reproduce**

<!-- A description of the steps to reproduce the behavior including the full `trip` or `trip.exe` command line. -->
trip -6 --udp -R dublin  --tui-custom-columns holsravbwdtC -m pretty -C5 -S5000 -P33437 -i20msec -T200msec -f4 -t 5 (some ipv6prefix)::1
Bug +/- stable with same parameters on some ports&ips. But disappears with  (some ipv6prefix)::2 or changing -S or -P.
Key problem parameter(s) -f and -t (slicing)



Discussion / hints:
Thank you very much for reporting this. Can you do `trip -V` and give me the version of trippy you are running please?
I got the same issue with version 0.12.1.

Simply `trip intel.com -f 20` sometimes panick with `range start index 19 out of range for slice of length 4`.
Thank you @justin0u0. Appreciate the information. If it happens again and it's possible for you to capture the panic message that would also be appreciated but no worries what you provided so far already helps.
Hi @c-git, this is what I got with `RUST_BACKTRACE=full trip intel.com -f 20`. Hope this help!

```
thread 'tracer-24894' panicked at crates/trippy-core/src/state.rs:636:59:
range start index 19 out of range for slice of length 4
stack backtrace:
   0:     0x5ee62beb8c29 - <unknown>
   1:     0x5ee62bd0fa80 - <unknown>
   2:     0x5ee62bea87ac - <unknown>
   3:     0x5ee62beb89be - <unknown>
   4:     0x5ee62be98c00 - <unknown>
   5:     0x5ee62be98743 - <unknown>
   6:     0x5ee62be9947e - <unknown>
   7:     0x5ee62beb90b0 - <unknown>
   8:     0x5ee62beb9006 - <unknown>
   9:     0x5ee62be98f7f - <unknown>
  10:     0x5ee62bc62544 - <unknown>
  11:     0x5ee62bc62ad1 - <unknown>
  12:     0x5ee62bf51c5a - <unknown>
  13:     0x5ee62bf393c2 - <unknown>
  14:     0x5ee62bf55491 - <unknown>
  15:     0x5ee62bf560a4 - <unknown>
  16:     0x5ee62bf56312 - <unknown>
  17:     0x5ee62be9c0d5 - <unknown>
  18:     0x7d7db0894ac3 - start_thread
                               at ./nptl/pthread_create.c:442:8
  19:     0x7d7db0926850 - __GI___clone3
                               at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
  20:                0x0 - <unknown>
```
Yes it does, thank you.

Fix patch:
diff --git a/crates/trippy-core/Cargo.toml b/crates/trippy-core/Cargo.toml
index b2d1b7c8..25176e06 100644
--- a/crates/trippy-core/Cargo.toml
+++ b/crates/trippy-core/Cargo.toml
@@ -43,7 +43,7 @@ serde = { workspace = true, default-features = false, features = ["derive"] }
 test-case.workspace = true
 tokio-util.workspace = true
 tokio = { workspace = true, features = ["full"] }
-toml.workspace = true
+toml = { workspace = true, default-features = false, features = ["parse"] }
 tracing-subscriber = { workspace = true, default-features = false, features = ["env-filter", "fmt"] }
 
 # see https://github.com/meh/rust-tun/pull/74
diff --git a/crates/trippy-core/src/state.rs b/crates/trippy-core/src/state.rs
index 1303c1b9..6b296fd8 100644
--- a/crates/trippy-core/src/state.rs
+++ b/crates/trippy-core/src/state.rs
@@ -528,7 +528,7 @@ impl FlowState {
 mod state_updater {
     use crate::state::FlowState;
     use crate::types::Checksum;
-    use crate::{NatStatus, ProbeStatus, Round};
+    use crate::{NatStatus, ProbeStatus, Round, TimeToLive};
     use std::time::Duration;
 
     /// Update the state of a `FlowState` from a `Round`.
@@ -632,16 +632,9 @@ mod state_updater {
                     hop.last_sequence = awaited.sequence.0;
                     if self.forward_loss {
                         hop.total_backward_lost += 1;
-                    } else {
-                        let remaining = &self.round.probes[index..];
-                        let all_awaited = remaining
-                            .iter()
-                            .skip(1)
-                            .all(|p| matches!(p, ProbeStatus::Awaited(_) | ProbeStatus::Skipped));
-                        if all_awaited {
-                            hop.total_forward_lost += 1;
-                            self.forward_loss = true;
-                        }
+                    } else if is_forward_loss(self.round.probes, awaited.ttl) {
+                        hop.total_forward_lost += 1;
+                        self.forward_loss = true;
                     }
                 }
                 ProbeStatus::Failed(failed) => {
@@ -665,6 +658,28 @@ mod state_updater {
         }
     }
 
+    /// Determine if forward loss has occurred at a given time-to-live.
+    ///
+    /// This is determined by checking if all probes after the awaited probe are all also awaited.
+    fn is_forward_loss(probes: &[ProbeStatus], awaited_ttl: TimeToLive) -> bool {
+        // Skip all probes that have a ttl less than or equal to the awaited ttl. What remains
+        // are the probes we are interested in.
+        let mut remaining = probes
+            .iter()
+            .skip_while(|p| match p {
+                ProbeStatus::Awaited(a) => a.ttl <= awaited_ttl,
+                ProbeStatus::Complete(c) => c.ttl <= awaited_ttl,
+                ProbeStatus::Failed(f) => f.ttl <= awaited_ttl,
+                ProbeStatus::NotSent | ProbeStatus::Skipped => true,
+            })
+            .peekable();
+        let is_empty = remaining.peek().is_none();
+        let all_awaited =
+            remaining.all(|p| matches!(p, ProbeStatus::Awaited(_) | ProbeStatus::Skipped));
+        // If there is at least one probe remaining and all are awaited then we have forward loss.
+        !is_empty && all_awaited
+    }
+
     /// Determine the NAT detection status.
     ///
     /// Returns a tuple of the NAT detection status and the checksum to use for the next hop.
@@ -699,8 +714,81 @@ mod state_updater {
     #[cfg(test)]
     mod tests {
         use super::*;
+        use crate::probe::ProbeFailed;
+        use crate::{
+            Flags, IcmpPacketType, Port, Probe, ProbeComplete, RoundId, Sequence, TimeToLive,
+            TraceId,
+        };
+        use std::net::{IpAddr, Ipv4Addr};
+        use std::time::SystemTime;
         use test_case::test_case;
 
+        #[test_case(false, &[], 1; "no forward loss no probes ttl 1")]
+        #[test_case(true, &[('a', 1), ('a', 2)], 1; "forward loss AA ttl 1")]
+        #[test_case(false, &[('a', 1), ('c', 2)], 1; "no forward loss AC ttl 1")]
+        #[test_case(false, &[('a', 1), ('f', 2)], 1; "no forward loss AF ttl 1")]
+        #[test_case(false, &[('a', 1), ('n', 2)], 1; "no forward loss AN ttl 1")]
+        #[test_case(false, &[('a', 1), ('c', 2), ('a', 3), ('a', 4)], 1; "no forward loss ACAA ttl 1")]
+        #[test_case(true, &[('a', 1), ('c', 2), ('a', 3), ('a', 4)], 3; "forward loss ACAA ttl 3")]
+        #[test_case(false, &[('a', 1), ('c', 2), ('a', 3), ('a', 4)], 4; "no forward loss ACAA ttl 4")]
+        #[test_case(false, &[('a', 1), ('f', 2), ('n', 3), ('a', 4)], 4; "no forward loss AFAN ttl 1")]
+        #[test_case(true, &[('a', 4), ('a', 5)], 4; "forward loss AA non-default minimum ttl 4")]
+        #[test_case(false, &[('a', 4), ('c', 5)], 4; "no forward loss AC non-default minimum ttl 4")]
+        #[test_case(false, &[('a', 4), ('c', 5), ('a', 6), ('a', 7)], 4; "no forward loss ACAA non-default minimum ttl 4")]
+        #[test_case(true, &[('a', 4), ('c', 5), ('a', 6), ('a', 7)], 6; "forward loss ACAA non-default minimum ttl 6")]
+        fn test_is_forward_loss(expected: bool, probes: &[(char, u8)], awaited_ttl: u8) {
+            assert!(awaited_ttl > 0);
+            let probes = probes
+                .iter()
+                .map(|(typ, ttl)| {
+                    assert!(matches!(typ, 'n' | 's' | 'f' | 'a' | 'c'));
+                    if *ttl == awaited_ttl {
+                        assert!(matches!(typ, 'a'));
+                    }
+                    match typ {
+                        'n' => ProbeStatus::NotSent,
+                        's' => ProbeStatus::Skipped,
+                        'f' => ProbeStatus::Failed(ProbeFailed {
+                            sequence: Sequence::default(),
+                            identifier: TraceId::default(),
+                            src_port: Port::default(),
+                            dest_port: Port::default(),
+                            ttl: TimeToLive(*ttl),
+                            round: RoundId::default(),
+                            sent: SystemTime::now(),
+                        }),
+                        'a' => ProbeStatus::Awaited(Probe {
+                            sequence: Sequence::default(),
+                            identifier: TraceId::default(),
+                            src_port: Port::default(),
+                            dest_port: Port::default(),
+                            ttl: TimeToLive(*ttl),
+                            round: RoundId(0),
+                            sent: SystemTime::now(),
+                            flags: Flags::empty(),
+                        }),
+                        'c' => ProbeStatus::Complete(ProbeComplete {
+                            sequence: Sequence::default(),
+                            identifier: TraceId::default(),
+                            src_port: Port::default(),
+                            dest_port: Port::default(),
+                            ttl: TimeToLive(*ttl),
+                            round: RoundId::default(),
+                            sent: SystemTime::now(),
+                            host: IpAddr::V4(Ipv4Addr::UNSPECIFIED),
+                            received: SystemTime::now(),
+                            icmp_packet_type: IcmpPacketType::NotApplicable,
+                            expected_udp_checksum: None,
+                            actual_udp_checksum: None,
+                            extensions: None,
+                        }),
+                        _ => unreachable!(),
+                    }
+                })
+                .collect::<Vec<_>>();
+            assert_eq!(is_forward_loss(&probes, TimeToLive(awaited_ttl)), expected);
+        }
+
         #[test_case(123, 123, None => (NatStatus::NotDetected, 123); "first hop matching checksum")]
         #[test_case(123, 321, None => (NatStatus::Detected, 321); "first hop non-matching checksum")]
         #[test_case(123, 123, Some(123) => (NatStatus::NotDetected, 123); "non-first hop matching checksum match previous")]
@@ -903,6 +991,7 @@ mod tests {
     #[test_case(file!("nat.toml"))]
     #[test_case(file!("minimal.toml"))]
     #[test_case(file!("floss_bloss.toml"))]
+    #[test_case(file!("non_default_minimum_ttl.toml"))]
     fn test_scenario(scenario: Scenario) {
         let mut trace = State::new(StateConfig {
             max_flows: 1,


Test patch:
diff --git a/crates/trippy-core/tests/resources/state/non_default_minimum_ttl.toml b/crates/trippy-core/tests/resources/state/non_default_minimum_ttl.toml
new file mode 100644
index 00000000..a1c33985
--- /dev/null
+++ b/crates/trippy-core/tests/resources/state/non_default_minimum_ttl.toml
@@ -0,0 +1,21 @@
+largest_ttl = 5
+
+[[rounds]]
+probes = [
+  "4 A 333 10.1.0.1 0 12340 80 0 0",
+  "5 A 777 10.1.0.2 1 12340 80 0 0",
+]
+
+[[expected.hops]]
+ttl = 4
+total_sent = 1
+total_recv = 0
+total_forward_loss = 1
+total_backward_loss = 0
+
+[[expected.hops]]
+ttl = 5
+total_sent = 1
+total_recv = 0
+total_forward_loss = 0
+total_backward_loss = 1