From 18f3d3b7568df8b3dc316e9a5c2921474bf7768b Mon Sep 17 00:00:00 2001 From: Zious Date: Wed, 24 Jun 2026 08:19:55 -0500 Subject: [PATCH 1/2] fix(dnp3)!: rename output key total_parse_errors -> parse_errors [PC-014, BC-2.15.020] BREAKING CHANGE: DNP3 analyzer summarize() detail map now emits key "parse_errors" instead of "total_parse_errors", aligning DNP3 with the HTTP, TLS, and Modbus sibling analyzers. JSON consumers reading DNP3 summary output must update their key references. - src/analyzer/dnp3.rs: rename local var total_parse_errors -> parse_errors, update doc comment, change string key literal "total_parse_errors" -> "parse_errors" (the single behavioral change, line ~1425) - tests/dnp3_detection_tests.rs: update key lookups and assertions in test_BC_2_15_020_summarize_empty_zero_flows (~line 993) and test_BC_2_15_020_summarize_includes_parse_errors (~line 1362-1380) to use the new key name; Red Gate test test_BC_2_15_020_parse_errors_key_name_is_parse_errors now passes - tests/bc_2_15_110_dnp3_dispatcher_tests.rs: rename local variable total_parse_errors -> parse_errors (~line 959/961, cosmetic only) - CHANGELOG.md: document breaking change under [Unreleased] Traces to: BC-2.15.020 v1.4 postcondition 1 (D-220); STORY-108 AC-010. --- CHANGELOG.md | 8 +++ src/analyzer/dnp3.rs | 8 +-- tests/bc_2_15_110_dnp3_dispatcher_tests.rs | 4 +- tests/dnp3_detection_tests.rs | 63 ++++++++++++++++++---- 4 files changed, 68 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a905c2f..21232e18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ Version numbers follow [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Breaking Changes + +- **DNP3 analyzer output: renamed summary key `total_parse_errors` → `parse_errors`.** + The `detail` map produced by the DNP3 analyzer now uses the key `"parse_errors"` instead of + `"total_parse_errors"`, aligning DNP3 with sibling analyzers (HTTP, TLS, Modbus) that already + use `"parse_errors"`. JSON consumers reading DNP3 summary output must migrate the key name. + [PC-014, BC-2.15.020 v1.4, STORY-108 AC-010] + ## [0.9.4] - 2026-06-23 ### Added diff --git a/src/analyzer/dnp3.rs b/src/analyzer/dnp3.rs index f1afbe18..63f8959b 100644 --- a/src/analyzer/dnp3.rs +++ b/src/analyzer/dnp3.rs @@ -1372,7 +1372,7 @@ impl Dnp3Analyzer { /// /// Aggregates across all flows: `function_code_distribution` (from /// `self.fn_code_counts`, zero-count FCs suppressed), `control_operation_counts` - /// (per-flow `direct_operate_count`), `total_frames`, `total_parse_errors`, + /// (per-flow `direct_operate_count`), `total_frames`, `parse_errors`, /// `flows_analyzed`. Returns a populated `AnalysisSummary` even when zero flows /// were processed (all counts zero, maps empty — BC-2.15.020 postcondition 2). /// @@ -1382,7 +1382,7 @@ impl Dnp3Analyzer { let flows_analyzed = self.flows.len() as u64; let total_frames: u64 = self.flows.values().map(|f| f.frame_count).sum(); - let total_parse_errors: u64 = self.flows.values().map(|f| f.parse_errors).sum(); + let parse_errors: u64 = self.flows.values().map(|f| f.parse_errors).sum(); // BC-2.15.020 postcondition 1: function_code_distribution — only FCs with count > 0. // Keys are decimal strings of the FC byte (e.g. "5" for 0x05 DIRECT_OPERATE). @@ -1422,8 +1422,8 @@ impl Dnp3Analyzer { serde_json::Value::Number(total_frames.into()), ); detail.insert( - "total_parse_errors".to_string(), - serde_json::Value::Number(total_parse_errors.into()), + "parse_errors".to_string(), + serde_json::Value::Number(parse_errors.into()), ); detail.insert( "flows_analyzed".to_string(), diff --git a/tests/bc_2_15_110_dnp3_dispatcher_tests.rs b/tests/bc_2_15_110_dnp3_dispatcher_tests.rs index 5ff6859b..3270ecf4 100644 --- a/tests/bc_2_15_110_dnp3_dispatcher_tests.rs +++ b/tests/bc_2_15_110_dnp3_dispatcher_tests.rs @@ -956,9 +956,9 @@ mod story_110 { let dnp3 = dispatcher.take_dnp3_analyzer().unwrap(); // The carry mechanism must have assembled the full frame and processed it. // parse_errors == 0 (no structural failure from the split). - let total_parse_errors: u64 = dnp3.flows.values().map(|f| f.parse_errors).sum(); + let parse_errors: u64 = dnp3.flows.values().map(|f| f.parse_errors).sum(); assert_eq!( - total_parse_errors, 0, + parse_errors, 0, "EC-003: split frame across two on_data calls must produce ZERO parse errors \ (carry buffer reassembles the frame cleanly)" ); diff --git a/tests/dnp3_detection_tests.rs b/tests/dnp3_detection_tests.rs index dd641b69..f779eb7f 100644 --- a/tests/dnp3_detection_tests.rs +++ b/tests/dnp3_detection_tests.rs @@ -990,14 +990,14 @@ mod story_108 { "AC-011: total_frames must be 0 when no flows processed" ); - let total_parse_errors = summary + let parse_errors = summary .detail - .get("total_parse_errors") + .get("parse_errors") .and_then(|v| v.as_u64()) .unwrap_or(u64::MAX); assert_eq!( - total_parse_errors, 0, - "AC-011: total_parse_errors must be 0 when no flows processed" + parse_errors, 0, + "AC-011: parse_errors must be 0 when no flows processed" ); // function_code_distribution must be present (as empty object, not absent) @@ -1352,10 +1352,10 @@ mod story_108 { } // ----------------------------------------------------------------------- - // total_parse_errors in summary + // parse_errors in summary // ----------------------------------------------------------------------- - /// Verifies summarize() includes total_parse_errors (BC-2.15.020 postcondition 1). + /// Verifies summarize() includes parse_errors (BC-2.15.020 postcondition 1). /// /// Traces to: BC-2.15.020 postcondition 1; STORY-108 AC-011. #[test] @@ -1373,10 +1373,10 @@ mod story_108 { let summary = analyzer.summarize(); - // total_parse_errors must be present + // parse_errors must be present assert!( - summary.detail.contains_key("total_parse_errors"), - "BC-2.15.020: total_parse_errors key must be present in summary detail" + summary.detail.contains_key("parse_errors"), + "BC-2.15.020: parse_errors key must be present in summary detail" ); } @@ -1569,4 +1569,49 @@ mod story_108 { f.source_ip ); } + // ----------------------------------------------------------------------- + // Red Gate — BC-2.15.020 v1.4: parse_errors key rename (D-220, human-approved) + // + // This test MUST FAIL on current code (which emits "total_parse_errors") and + // MUST PASS after the implementer renames the key to "parse_errors" in + // src/analyzer/dnp3.rs:1425. + // + // Traces to: BC-2.15.020 v1.4 postcondition 1 (BREAKING rename D-220); + // scope.md PC-014; test-vector row "Red Gate — key name". + // ----------------------------------------------------------------------- + + /// BC-2.15.020 v1.4 Red Gate: summarize() detail map MUST use key "parse_errors", + /// NOT "total_parse_errors" (D-220 breaking rename — aligns DNP3 with HTTP/TLS/Modbus). + /// + /// Traces to: BC-2.15.020 v1.4 postcondition 1; PC-014 scope.md §4. + #[test] + fn test_BC_2_15_020_parse_errors_key_name_is_parse_errors() { + let mut analyzer = Dnp3Analyzer::new(10); + let key = test_flow_key(); + + // Deliver a frame with invalid LENGTH (4 < 5) to produce a parse error, + // mirroring the approach used by test_BC_2_15_020_summarize_includes_parse_errors. + let mut bad_frame = vec![0u8; 10]; + bad_frame[0] = 0x05; + bad_frame[1] = 0x64; + bad_frame[2] = 4; // invalid LENGTH — triggers parse error counter + bad_frame[3] = 0xC4; + analyzer.on_data(key.clone(), &bad_frame, 0); + + let summary = analyzer.summarize(); + + // MUST be present under the new canonical key name (post-D-220). + assert!( + summary.detail.contains_key("parse_errors"), + "BC-2.15.020 v1.4 D-220: detail map must contain \"parse_errors\" \ + (aligns with HTTP/TLS/Modbus sibling analyzers)" + ); + + // MUST NOT be present under the old divergent key name. + assert!( + !summary.detail.contains_key("total_parse_errors"), + "BC-2.15.020 v1.4 D-220: detail map must NOT contain \"total_parse_errors\" \ + (old key removed by rename; callers must migrate to \"parse_errors\")" + ); + } } // mod story_108 From 21af8b21c40573ae2c3295d3c49dba92f5ca05bd Mon Sep 17 00:00:00 2001 From: Zious Date: Wed, 24 Jun 2026 08:27:34 -0500 Subject: [PATCH 2/2] refactor(dnp3): rename aggregate local var to avoid shadowing per-flow field; add migration note to CHANGELOG [PC-014] CR-004: summarize() local `parse_errors` renamed to `aggregate_parse_errors` to visually distinguish the cross-flow aggregate from Dnp3FlowState.parse_errors. JSON output key "parse_errors" is unchanged. CR-005: CHANGELOG breaking-change entry now includes a concrete migration snippet (jq one-liner) for script/JSON consumers. --- CHANGELOG.md | 4 ++++ src/analyzer/dnp3.rs | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21232e18..19f07db0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ Version numbers follow [Semantic Versioning](https://semver.org/). use `"parse_errors"`. JSON consumers reading DNP3 summary output must migrate the key name. [PC-014, BC-2.15.020 v1.4, STORY-108 AC-010] + **Migration:** Replace any lookup of `detail["total_parse_errors"]` with + `detail["parse_errors"]` in your consumer. For `jq` users: + `jq '.[] | .detail.total_parse_errors'` → `jq '.[] | .detail.parse_errors'`. + ## [0.9.4] - 2026-06-23 ### Added diff --git a/src/analyzer/dnp3.rs b/src/analyzer/dnp3.rs index 63f8959b..4d2f4114 100644 --- a/src/analyzer/dnp3.rs +++ b/src/analyzer/dnp3.rs @@ -1382,7 +1382,7 @@ impl Dnp3Analyzer { let flows_analyzed = self.flows.len() as u64; let total_frames: u64 = self.flows.values().map(|f| f.frame_count).sum(); - let parse_errors: u64 = self.flows.values().map(|f| f.parse_errors).sum(); + let aggregate_parse_errors: u64 = self.flows.values().map(|f| f.parse_errors).sum(); // BC-2.15.020 postcondition 1: function_code_distribution — only FCs with count > 0. // Keys are decimal strings of the FC byte (e.g. "5" for 0x05 DIRECT_OPERATE). @@ -1423,7 +1423,7 @@ impl Dnp3Analyzer { ); detail.insert( "parse_errors".to_string(), - serde_json::Value::Number(parse_errors.into()), + serde_json::Value::Number(aggregate_parse_errors.into()), ); detail.insert( "flows_analyzed".to_string(),