Skip to content

Commit d74656e

Browse files
committed
fix: deep code review pass 22 - README JSON, non_exhaustive, test assertions
- Update README JSON example to match actual serialization output (sys_up_time/first/last are u32 not Duration, field name is flowsets not sets) - Add #[non_exhaustive] to 5 public types: NetflowPacket, ParseResult, TemplateEvent, TemplateProtocol, ScopingInfo - Fix test_error_sample_size_bounded vacuous if-let (now panics on wrong variant) - Tighten scoped parser test assertions from >= 2 to == 2 - Update match arms in examples/tests/doctests for non_exhaustive compatibility
1 parent 14fc860 commit d74656e

10 files changed

Lines changed: 49 additions & 36 deletions

README.md

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -82,39 +82,39 @@ println!("{}", json!(result.packets).to_string());
8282
{
8383
"V5": {
8484
"header": {
85+
"version": 5,
8586
"count": 1,
86-
"engine_id": 7,
87-
"engine_type": 6,
88-
"flow_sequence": 33752069,
89-
"sampling_interval": 2057,
90-
"sys_up_time": { "nanos": 672000000, "secs": 50332 },
91-
"unix_nsecs": 134807553,
87+
"sys_up_time": 50332672,
9288
"unix_secs": 83887623,
93-
"version": 5
89+
"unix_nsecs": 134807553,
90+
"flow_sequence": 33752069,
91+
"engine_type": 6,
92+
"engine_id": 7,
93+
"sampling_interval": 2057
9494
},
95-
"sets": [
95+
"flowsets": [
9696
{
97-
"d_octets": 66051,
98-
"d_pkts": 101124105,
97+
"src_addr": "0.1.2.3",
9998
"dst_addr": "4.5.6.7",
100-
"dst_as": 515,
101-
"dst_mask": 5,
102-
"dst_port": 1029,
103-
"first": { "nanos": 87000000, "secs": 67438 },
104-
"input": 515,
105-
"last": { "nanos": 553000000, "secs": 134807 },
10699
"next_hop": "8.9.0.1",
100+
"input": 515,
107101
"output": 1029,
102+
"d_pkts": 101124105,
103+
"d_octets": 66051,
104+
"first": 67438087,
105+
"last": 134807553,
106+
"src_port": 515,
107+
"dst_port": 1029,
108108
"pad1": 6,
109-
"pad2": 1543,
109+
"tcp_flags": 7,
110110
"protocol_number": 8,
111111
"protocol_type": "Egp",
112-
"src_addr": "0.1.2.3",
112+
"tos": 9,
113113
"src_as": 1,
114+
"dst_as": 515,
114115
"src_mask": 4,
115-
"src_port": 515,
116-
"tcp_flags": 7,
117-
"tos": 9
116+
"dst_mask": 5,
117+
"pad2": 1543
118118
}
119119
]
120120
}

examples/netflow_pcap.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,8 @@ fn main() {
7575
for data in pcap_data.iter() {
7676
for result in parser.iter_packets(data) {
7777
match result {
78-
Ok(packet) => match packet {
79-
NetflowPacket::V5(ref _v5) => parsed_packets.push(packet),
80-
NetflowPacket::V7(ref _v7) => parsed_packets.push(packet),
81-
NetflowPacket::V9(ref _v9) => parsed_packets.push(packet),
82-
NetflowPacket::IPFix(ref ipfix) => {
78+
Ok(packet) => match &packet {
79+
NetflowPacket::IPFix(ipfix) => {
8380
let has_no_template = ipfix
8481
.flowsets
8582
.iter()
@@ -90,6 +87,7 @@ fn main() {
9087
parsed_packets.push(packet);
9188
}
9289
}
90+
_ => parsed_packets.push(packet),
9391
},
9492
Err(e) => println!("Error parsing packet: {:?}", e),
9593
}

examples/template_hooks.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ fn demo_basic_hooks() {
5252
template_id,
5353
protocol,
5454
} => println!(" ❌ Missing template {:?} ({:?})", template_id, protocol),
55+
_ => {}
5556
}
5657
Ok(())
5758
})
@@ -195,6 +196,7 @@ fn demo_logging_hooks() {
195196
"[ERROR] Missing template {:?} for protocol {:?}",
196197
template_id, protocol
197198
),
199+
_ => "[DEBUG] Unknown event".to_string(),
198200
};
199201
log_clone.lock().unwrap().push(log_entry);
200202
Ok(())

src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ pub use variable_versions::v9::lookup::V9Field;
6666
pub use variable_versions::v9::{V9FieldPair, V9FlowRecord};
6767

6868
/// Enum of supported Netflow Versions
69+
#[non_exhaustive]
6970
#[derive(Debug, PartialEq, Clone, Serialize)]
7071
pub enum NetflowPacket {
7172
/// Version 5
@@ -127,6 +128,7 @@ impl NetflowPacket {
127128
/// eprintln!("Parsing stopped: {}", error);
128129
/// }
129130
/// ```
131+
#[non_exhaustive]
130132
#[derive(Debug, Clone, Serialize)]
131133
#[must_use = "parsing results should not be discarded; check .packets and .error"]
132134
pub struct ParseResult {

src/scoped_parser.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ impl<K: Hash + Eq> RouterScopedParser<K> {
371371
}
372372

373373
/// Information extracted from NetFlow packet headers for RFC-compliant scoping.
374+
#[non_exhaustive]
374375
#[derive(Debug, Clone, PartialEq, Eq)]
375376
pub enum ScopingInfo {
376377
/// NetFlow v9 packet with source ID
@@ -443,6 +444,7 @@ pub struct V9SourceKey {
443444
/// ScopingInfo::Unknown => {
444445
/// println!("Invalid or truncated packet");
445446
/// }
447+
/// _ => {}
446448
/// }
447449
/// ```
448450
pub fn extract_scoping_info(data: &[u8]) -> ScopingInfo {

src/template_events.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use std::sync::Arc;
3838

3939
/// Protocol type for template events.
40+
#[non_exhaustive]
4041
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize)]
4142
pub enum TemplateProtocol {
4243
/// NetFlow v9 template
@@ -49,6 +50,7 @@ pub enum TemplateProtocol {
4950
///
5051
/// These events are emitted during template cache operations and allow
5152
/// users to monitor and react to template state changes.
53+
#[non_exhaustive]
5254
#[derive(Debug, Clone)]
5355
pub enum TemplateEvent {
5456
/// A new template was learned and added to the cache.

tests/cache_collision.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ fn test_auto_scoped_parser_template_isolation() {
3939
assert_eq!(result2_data.packets.len(), 1);
4040

4141
// Source2 should have a miss, source1 should not
42-
assert!(parser.source_count() >= 2);
42+
assert_eq!(parser.source_count(), 2);
4343
}
4444

4545
// Verify that a single NetflowParser produces consistent results across repeated V5 parses

tests/memory_bounds.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,19 @@ fn test_error_sample_size_bounded() {
6464
let garbage = vec![0xFFu8; 256];
6565
let result = parser.parse_bytes(&garbage);
6666

67-
if let Some(netflow_parser::NetflowError::UnsupportedVersion { sample, .. }) = result.error
68-
{
69-
assert!(
70-
sample.len() <= max_sample,
71-
"error sample {} bytes exceeds max {}",
72-
sample.len(),
73-
max_sample
74-
);
67+
match result.error {
68+
Some(netflow_parser::NetflowError::UnsupportedVersion { sample, .. }) => {
69+
assert!(
70+
sample.len() <= max_sample,
71+
"error sample {} bytes exceeds max {}",
72+
sample.len(),
73+
max_sample
74+
);
75+
}
76+
other => panic!(
77+
"expected UnsupportedVersion error, got {:?}",
78+
other
79+
),
7580
}
7681
}
7782

tests/scoped_parser.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ fn test_auto_scoped_parser_multiple_sources() {
4747
let _ = parser.parse_from_source(source2, &v5_packet);
4848

4949
// V5 packets create sources
50-
assert!(parser.source_count() >= 2);
50+
assert_eq!(parser.source_count(), 2);
5151
}
5252

5353
// Verify that RouterScopedParser works with String keys for per-router parsing

tests/template_hooks.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ fn test_hook_with_logging() {
211211
template_id,
212212
protocol,
213213
} => format!("Missing template {:?} ({:?})", template_id, protocol),
214+
_ => "Unknown event".to_string(),
214215
};
215216
log_clone.lock().unwrap().push(msg);
216217
Ok(())
@@ -256,6 +257,7 @@ fn test_hooks_fire_during_parsing() {
256257
TemplateEvent::MissingTemplate { .. } => "MissingTemplate",
257258
TemplateEvent::Evicted { .. } => "Evicted",
258259
TemplateEvent::Expired { .. } => "Expired",
260+
_ => "Unknown",
259261
};
260262
events_clone.lock().unwrap().push(name.to_string());
261263
Ok(())

0 commit comments

Comments
 (0)