mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-07-27 02:44:57 +00:00
Normalize aggregate result words to lowercase (#835)
* fix: normalize aggregate result words Normalize policy-evaluated and authentication result values so mixed-case reporter output does not create duplicate categories. Add a regression covering the existing uppercase sample. * Move changelog entry to the Unreleased section The entry landed in the released 10.2.4 section because the branch was cut before the Unreleased heading existed on master; merged master and moved it under Unreleased -> Bug fixes, matching the house entry style. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Apply review feedback: normalize disposition, share a guarded helper - policy_evaluated disposition gets the same lowercase normalization as dkim/spf and the auth result words; the RFC 7489 Appendix C / RFC 9990 disposition enum is lowercase (none/quarantine/reject), and mixed-case values split the Message Disposition categories in every dashboard the same way Pass/pass did. - All four normalization sites now share _normalize_result_word(), which guards with isinstance(str): xmltodict returns a dict for attribute-bearing elements, so the previously unguarded .lower() calls on auth results could raise AttributeError on malformed input that used to pass through. - The #520 fixture's disposition is now mixed-case (None) and the test asserts it parses as "none"; verified the assertion fails against the pre-fix parser. Test docstring cites the RFC authority per the project's testing standards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Sean Whalen
Claude Fable 5
parent
6acacd01d1
commit
e98e12acb0
@@ -10,6 +10,7 @@
|
||||
### Bug fixes
|
||||
|
||||
- **The Elasticsearch/OpenSearch aggregate dashboards' over-time charts (and the Grafana ES dashboard's summary pies and time series) bucketed on the multi-valued `date_range` field**; a date histogram counts a report once per value, double-counting any report whose begin and end dates fall in different buckets. All date histograms and time-range filters now use the single-valued `date_begin`, matching the report-begin semantics of the PostgreSQL (`begin_date`) and Splunk (`_time` = interval begin) dashboards.
|
||||
- **Aggregate-report policy and authentication result words are now normalized to lowercase** ([#288](https://github.com/domainaware/parsedmarc/issues/288)): reporters that emit mixed-case values such as `Pass` no longer create duplicate result categories in outputs and dashboards.
|
||||
|
||||
## 10.2.4
|
||||
|
||||
|
||||
+17
-7
@@ -181,6 +181,15 @@ def _text(value: Any) -> str | None:
|
||||
return value
|
||||
|
||||
|
||||
def _normalize_result_word(value: Any) -> Any:
|
||||
"""Lowercase a reporter-supplied enum word; RFC 7489 Appendix C and
|
||||
RFC 9990 define result/disposition types as lowercase tokens. Non-string
|
||||
values (e.g. xmltodict dicts from attribute-bearing elements) pass
|
||||
through unchanged.
|
||||
"""
|
||||
return value.lower() if isinstance(value, str) else value
|
||||
|
||||
|
||||
def _bucket_interval_by_day(
|
||||
begin: datetime,
|
||||
end: datetime,
|
||||
@@ -422,11 +431,12 @@ def _parse_report_record(
|
||||
"policy_override_reasons": [],
|
||||
}
|
||||
if "disposition" in policy_evaluated:
|
||||
new_policy_evaluated["disposition"] = policy_evaluated["disposition"]
|
||||
if "dkim" in policy_evaluated:
|
||||
new_policy_evaluated["dkim"] = policy_evaluated["dkim"]
|
||||
if "spf" in policy_evaluated:
|
||||
new_policy_evaluated["spf"] = policy_evaluated["spf"]
|
||||
new_policy_evaluated["disposition"] = _normalize_result_word(
|
||||
policy_evaluated["disposition"]
|
||||
)
|
||||
for key in ("dkim", "spf"):
|
||||
if key in policy_evaluated:
|
||||
new_policy_evaluated[key] = _normalize_result_word(policy_evaluated[key])
|
||||
reasons = []
|
||||
spf_aligned = (
|
||||
policy_evaluated["spf"] is not None
|
||||
@@ -505,7 +515,7 @@ def _parse_report_record(
|
||||
)
|
||||
new_result["selector"] = "none"
|
||||
if "result" in result and result["result"] is not None:
|
||||
new_result["result"] = result["result"]
|
||||
new_result["result"] = _normalize_result_word(result["result"])
|
||||
else:
|
||||
new_result["result"] = "none"
|
||||
new_result["human_result"] = _text(result.get("human_result"))
|
||||
@@ -521,7 +531,7 @@ def _parse_report_record(
|
||||
else:
|
||||
new_result["scope"] = "mfrom"
|
||||
if "result" in result and result["result"] is not None:
|
||||
new_result["result"] = result["result"]
|
||||
new_result["result"] = _normalize_result_word(result["result"])
|
||||
else:
|
||||
new_result["result"] = "none"
|
||||
new_result["human_result"] = _text(result.get("human_result"))
|
||||
|
||||
@@ -21,7 +21,7 @@
|
||||
<source_ip>23.104.41.189</source_ip>
|
||||
<count>1</count>
|
||||
<policy_evaluated>
|
||||
<disposition>none</disposition>
|
||||
<disposition>None</disposition>
|
||||
<dkim>Pass</dkim>
|
||||
<spf>Pass</spf>
|
||||
</policy_evaluated>
|
||||
|
||||
@@ -166,6 +166,25 @@ class Test(unittest.TestCase):
|
||||
)
|
||||
print("Passed!")
|
||||
|
||||
def testAggregateResultWordsAreLowercase(self):
|
||||
"""Reporter-supplied result words are lowercased at ingest; RFC 7489
|
||||
Appendix C and RFC 9990 define the result and disposition types as
|
||||
lowercase enum tokens (issue #288).
|
||||
"""
|
||||
result = parsedmarc.parse_report_file(
|
||||
"samples/aggregate_invalid/report_with_upper_cased_pass.xml",
|
||||
offline=True,
|
||||
)
|
||||
assert result["report_type"] == "aggregate"
|
||||
report = cast(AggregateReport, result["report"])
|
||||
record = report["records"][0]
|
||||
|
||||
self.assertEqual(record["policy_evaluated"]["dkim"], "pass")
|
||||
self.assertEqual(record["policy_evaluated"]["spf"], "pass")
|
||||
self.assertEqual(record["auth_results"]["dkim"][0]["result"], "pass")
|
||||
self.assertEqual(record["auth_results"]["spf"][0]["result"], "pass")
|
||||
self.assertEqual(record["policy_evaluated"]["disposition"], "none")
|
||||
|
||||
def testEmptySample(self):
|
||||
"""Test empty/unparasable report"""
|
||||
with self.assertRaises(parsedmarc.ParserError):
|
||||
|
||||
Reference in New Issue
Block a user