Summary
Found by silent-failure-hunter during PR #33 review, validated with Perplexity against Suricata/Zeek practices.
Problem
In src/reassembly/segment.rs, inside the gap insertion loop (lines 155-178), when self.segments.len() >= max_segments, the code executes break and exits silently. Any remaining gap segments are dropped without being inserted, with no counter or logging.
After the loop, the return value is determined by whether had_gap was true — and it was, because gaps were computed before the loop. The function returns PartialOverlap, and the caller increments segments_inserted += 1, even though some (or all) gap bytes were silently discarded.
Impact
segments_inserted counter becomes unreliable (counts an insert that may be partial or empty)
buffered_bytes stays accurate (only counts actually inserted bytes), so no memory accounting drift
- Incomplete reassembly with no indication of why — someone analyzing PCAP output gets partial data with no visibility into the drop
Recommended Fix
Track the partial insertion and return an appropriate status:
let mut segments_exhausted = false;
for (gap_start, gap_end) in gaps {
if self.segments.len() >= max_segments {
segments_exhausted = true;
break;
}
// ... existing insertion logic ...
}
Options for the return value:
- Return
DepthExceeded (consistent with how the non-overlap path handles segment count limits at line 50-51)
- Add a dedicated
SegmentLimitReached variant to InsertResult
- At minimum, increment a counter so the drop is observable
Validation
Perplexity confirms: Suricata and Zeek track segment-limit drops separately from depth-limit drops. Both recommend distinct counters because they indicate different failure modes — segment limits suggest attack patterns (many small segments), depth limits suggest memory pressure.
Summary
Found by silent-failure-hunter during PR #33 review, validated with Perplexity against Suricata/Zeek practices.
Problem
In
src/reassembly/segment.rs, inside the gap insertion loop (lines 155-178), whenself.segments.len() >= max_segments, the code executesbreakand exits silently. Any remaining gap segments are dropped without being inserted, with no counter or logging.After the loop, the return value is determined by whether
had_gapwas true — and it was, because gaps were computed before the loop. The function returnsPartialOverlap, and the caller incrementssegments_inserted += 1, even though some (or all) gap bytes were silently discarded.Impact
segments_insertedcounter becomes unreliable (counts an insert that may be partial or empty)buffered_bytesstays accurate (only counts actually inserted bytes), so no memory accounting driftRecommended Fix
Track the partial insertion and return an appropriate status:
Options for the return value:
DepthExceeded(consistent with how the non-overlap path handles segment count limits at line 50-51)SegmentLimitReachedvariant toInsertResultValidation
Perplexity confirms: Suricata and Zeek track segment-limit drops separately from depth-limit drops. Both recommend distinct counters because they indicate different failure modes — segment limits suggest attack patterns (many small segments), depth limits suggest memory pressure.