Fix blank SMTP TLS failure-detail fields crashing saves; read RFC 8460 additional-information (11.0.3) (#916)

* Treat blank SMTP TLS failure-detail fields as absent; read RFC 8460 additional-information

Some reporters send optional failure-details fields as empty strings
(e.g. "sending-mta-ip": "" on sts-policy-fetch-error). The parser copied
them verbatim, so the PostgreSQL save failed on the INET columns
("invalid input syntax for type inet") and the Elasticsearch/OpenSearch
saves failed Ip() validation, discarding the whole report. Blank or
whitespace-only optional values are now omitted.

RFC 8460 section 4.4 names the URI key "additional-information"; the
parser only read "additional-info-uri" (the schema's value placeholder),
so conformant reports lost the URI. The RFC key is now read, with the old
key kept as a fallback.

Bump version to 11.0.3.

Closes #915

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Import parse_report_file directly in the ES/OpenSearch tests

Addresses github-code-quality's "Module is imported with 'import' and
'import from'" findings on tests/test_elastic.py and
tests/test_opensearch.py.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 11.0.3
+
+### Bug fixes
+
+- **An SMTP TLS (RFC 8460) failure report whose `failure-details` entry sends an empty string for an optional field — e.g. `"sending-mta-ip": ""` and `"receiving-ip": ""`, which real reporters send for results like `sts-policy-fetch-error` — no longer crashes the PostgreSQL save.** `_parse_smtp_tls_failure_details()` copied every optional field verbatim whenever the JSON key was present, so an empty string reached the PostgreSQL `sending_mta_ip`/`receiving_ip` `INET` columns and failed with `invalid input syntax for type inet: ""`, discarding the whole report. The same blank values would also have crashed the Elasticsearch and OpenSearch saves: those outputs' `sending_mta_ip`/`receiving_ip` fields are IP-typed (`Ip()` in `elastic.py`/`opensearch.py`), and `Document.save()`'s validation pass rejects an empty string for an `Ip()` field with a bare `ValueError` — `'' does not appear to be an IPv4 or IPv6 address` — raised before any network call, so the whole report was lost rather than indexed with an empty IP. A `None` or blank/whitespace-only value for any optional failure-detail field (`sending-mta-ip`, `receiving-ip`, `receiving-mx-hostname`, `receiving-mx-helo`, the additional-information URI, `failure-reason-code`) is now treated as absent, so it is stored as `NULL` rather than `''`. (Closes #915)
+- **The RFC 8460 §4.4 `additional-information` failure-details key is now read.** The JSON Report Schema names the key `additional-information` (its value is described by the placeholder `additional-info-uri`), but the parser only read the non-RFC key `additional-info-uri`, so a conformant reporter's URI was silently dropped — including in the RFC's own worked example, `samples/smtp_tls/rfc8460.json`. The non-RFC `additional-info-uri` key is still read as a fallback for backward compatibility, and is used when the RFC key is absent or blank.
+
 ## 11.0.2
 
 ### Changes

parsedmarc/__init__.py

@@ -656,6 +656,26 @@ def _parse_report_record(
     return new_record
 
 
+def _non_blank(value: Any) -> Any:
+    """Treat ``None`` or a blank/whitespace-only string as absent.
+
+    Some SMTP TLS (RFC 8460) reporters send an empty string for an optional
+    failure-details field (e.g. ``sending-mta-ip``/``receiving-ip`` on a
+    ``sts-policy-fetch-error``) instead of omitting the key. An empty string
+    is not a valid IP address, so passing it through crashed the PostgreSQL
+    save (``INET`` columns) and would also have crashed the Elasticsearch
+    and OpenSearch saves: their ``Ip()`` fields' validation
+    (``elasticsearch.dsl``/``opensearchpy``'s ``Document.save()``) rejects
+    an empty string with a bare ``ValueError`` (issue #915). Non-string,
+    non-blank values pass through unchanged.
+    """
+    if value is None:
+        return None
+    if isinstance(value, str) and value.strip() == "":
+        return None
+    return value
+
+
 def _parse_smtp_tls_failure_details(failure_details: dict[str, Any]):
     try:
         new_failure_details: dict[str, Any] = {
@@ -663,26 +683,28 @@ def _parse_smtp_tls_failure_details(failure_details: dict[str, Any]):
             "failed_session_count": failure_details["failed-session-count"],
         }
 
-        if "sending-mta-ip" in failure_details:
-            new_failure_details["sending_mta_ip"] = failure_details["sending-mta-ip"]
-        if "receiving-ip" in failure_details:
-            new_failure_details["receiving_ip"] = failure_details["receiving-ip"]
-        if "receiving-mx-hostname" in failure_details:
-            new_failure_details["receiving_mx_hostname"] = failure_details[
-                "receiving-mx-hostname"
-            ]
-        if "receiving-mx-helo" in failure_details:
-            new_failure_details["receiving_mx_helo"] = failure_details[
-                "receiving-mx-helo"
-            ]
-        if "additional-info-uri" in failure_details:
-            new_failure_details["additional_info_uri"] = failure_details[
-                "additional-info-uri"
-            ]
-        if "failure-reason-code" in failure_details:
-            new_failure_details["failure_reason_code"] = failure_details[
-                "failure-reason-code"
-            ]
+        optional_fields = [
+            ("sending-mta-ip", "sending_mta_ip"),
+            ("receiving-ip", "receiving_ip"),
+            ("receiving-mx-hostname", "receiving_mx_hostname"),
+            ("receiving-mx-helo", "receiving_mx_helo"),
+            ("failure-reason-code", "failure_reason_code"),
+        ]
+        for src_key, dest_key in optional_fields:
+            value = _non_blank(failure_details.get(src_key))
+            if value is not None:
+                new_failure_details[dest_key] = value
+
+        # RFC 8460 §4.4 names the JSON key "additional-information" (its
+        # value is described as an "additional-info-uri"). Some reporters
+        # instead send the non-RFC key "additional-info-uri" directly; fall
+        # back to it for backward compatibility when the RFC key is absent
+        # or blank.
+        additional_info_uri = _non_blank(failure_details.get("additional-information"))
+        if additional_info_uri is None:
+            additional_info_uri = _non_blank(failure_details.get("additional-info-uri"))
+        if additional_info_uri is not None:
+            new_failure_details["additional_info_uri"] = additional_info_uri
 
         return new_failure_details
 

parsedmarc/constants.py

@@ -1,4 +1,4 @@
-__version__ = "11.0.2"
+__version__ = "11.0.3"
 
 USER_AGENT = f"parsedmarc/{__version__}"
 

samples/smtp_tls/empty_failure_detail_fields.json

@@ -0,0 +1,38 @@
+{
+    "organization-name":"Reporter Example",
+    "date-range":{
+       "start-datetime":"2024-06-01T00:00:00Z",
+       "end-datetime":"2024-06-01T23:59:59Z"
+    },
+    "contact-info":"smtp-tls-reporting@reporter.example",
+    "report-id":"2024-06-01T00:00:00Z_example.com",
+    "policies":[
+       {
+          "policy":{
+             "policy-type":"sts",
+             "policy-string":[
+                "version: STSv1",
+                "mode: enforce",
+                "mx: example.com",
+                "max_age: 86400"
+             ],
+             "policy-domain":"example.com"
+          },
+          "summary":{
+             "total-successful-session-count":0,
+             "total-failure-session-count":1
+          },
+          "failure-details":[
+             {
+                "result-type":"sts-policy-fetch-error",
+                "sending-mta-ip":"",
+                "receiving-ip":"",
+                "receiving-mx-hostname":"",
+                "failed-session-count":1,
+                "additional-information":"",
+                "failure-reason-code":"no-policy-served"
+             }
+          ]
+       }
+    ]
+}

tests/test_elastic.py

@@ -8,10 +8,11 @@ queries, error wrapping — without needing a running Elasticsearch cluster.
 
 import time
 import unittest
+from typing import cast
 from unittest.mock import MagicMock, call, patch
 
 import parsedmarc.elastic as elastic_module
-from parsedmarc import InvalidFailureReport
+from parsedmarc import InvalidFailureReport, parse_report_file
 from parsedmarc.elastic import (
     AlreadySaved,
     ElasticsearchError,
@@ -1486,6 +1487,46 @@ class TestSaveSmtpTlsReport(unittest.TestCase):
             "https://reports.example.com/tls-help",
         )
 
+    def test_save_blank_failure_detail_fields_omitted_from_ip_fields(self):
+        """Blank optional failure-detail fields must not reach the Ip()
+        fields (issue #915).
+
+        samples/smtp_tls/empty_failure_detail_fields.json reproduces a
+        real reporter's shape: sending-mta-ip/receiving-ip sent as "" on
+        an sts-policy-fetch-error. Verified directly against
+        elasticsearch.dsl: Document.save() validates by calling
+        full_clean() before any network call, and an Ip() field's
+        validation calls Python's ipaddress.ip_address(""), which raises a
+        bare ValueError ("'' does not appear to be an IPv4 or IPv6
+        address") rather than a caught ValidationException -- so an empty
+        string here previously discarded the whole report instead of
+        indexing it with a blank IP.
+
+        save() is mocked (per this module's convention, so no real
+        cluster is needed), which also means it never runs full_clean()
+        itself; call it explicitly on the captured document so this test
+        exercises the same validation a real save() would, and fails with
+        that ValueError on unfixed parsedmarc/__init__.py.
+        """
+        result = parse_report_file(
+            "samples/smtp_tls/empty_failure_detail_fields.json", offline=True
+        )
+        report = cast(dict, result["report"])
+        with (
+            patch("parsedmarc.elastic.Search", return_value=_empty_search()),
+            patch("parsedmarc.elastic.Index"),
+            patch.object(
+                elastic_module._SMTPTLSReportDoc, "save", autospec=True
+            ) as mock_save,
+        ):
+            save_smtp_tls_report_to_elasticsearch(report)
+        doc = mock_save.call_args[0][0]
+        doc.full_clean()
+        failure_detail = doc.policies[0].failure_details[0]
+        self.assertIsNone(failure_detail.sending_mta_ip)
+        self.assertIsNone(failure_detail.receiving_ip)
+        self.assertEqual(failure_detail.failure_reason_code, "no-policy-served")
+
 
 class TestBackwardCompatAlias(unittest.TestCase):
     def test_save_forensic_alias_points_to_save_failure(self):

tests/test_init.py

@@ -963,6 +963,26 @@ class Test(unittest.TestCase):
                 )
             print("Passed!")
 
+    def testSmtpTlsRfc8460SampleReadsAdditionalInformation(self):
+        """samples/smtp_tls/rfc8460.json is RFC 8460 §4.4's own worked
+        example, whose second failure-details entry sets
+        "additional-information" (not the non-RFC "additional-info-uri").
+        Before the fix, the parser only read "additional-info-uri", so this
+        URI was silently dropped even from the RFC's own example."""
+        result = parsedmarc.parse_report_file(
+            "samples/smtp_tls/rfc8460.json", offline=True
+        )
+        report = cast(SMTPTLSReport, result["report"])
+        failure_details = report["policies"][0].get("failure_details", [])
+        starttls_detail = next(
+            d for d in failure_details if d["result_type"] == "starttls-not-supported"
+        )
+        self.assertEqual(
+            starttls_detail.get("additional_info_uri"),
+            "https://reports.company-x.example/report_info?"
+            "id=5065427c-23d3#StarttlsNotSupported",
+        )
+
     def testSmtpTlsCsvStripsNulFromFields(self):
         """A NUL character in an SMTP TLS report text field is stripped
         from CSV output instead of reaching the ``csv`` writer.
@@ -1487,6 +1507,81 @@ class Test(unittest.TestCase):
         self.assertEqual(result["additional_info_uri"], "https://example.com/info")
         self.assertEqual(result["failure_reason_code"], "TLS_ERROR")
 
+    def testParseSmtpTlsFailureDetailsBlankOptionalFieldsOmitted(self):
+        """Empty-string and whitespace-only optional fields are treated as
+        absent, not passed through.
+
+        Real-world reporters send "" for sending-mta-ip/receiving-ip on
+        results like sts-policy-fetch-error (see samples/smtp_tls/
+        empty_failure_detail_fields.json). An empty string is not a valid
+        IP address, so it previously reached PostgreSQL's INET columns
+        verbatim and crashed the PostgreSQL save, and would also have
+        crashed the Elasticsearch/OpenSearch saves' Ip() field validation
+        with a bare ValueError (issue #915).
+        """
+        details = {
+            "result-type": "sts-policy-fetch-error",
+            "failed-session-count": 1,
+            "sending-mta-ip": "",
+            "receiving-ip": "   ",
+            "receiving-mx-hostname": "",
+            "receiving-mx-helo": "\t",
+            "additional-information": "",
+            "additional-info-uri": "",
+            "failure-reason-code": "",
+        }
+        result = parsedmarc._parse_smtp_tls_failure_details(details)
+        self.assertEqual(result["result_type"], "sts-policy-fetch-error")
+        self.assertEqual(result["failed_session_count"], 1)
+        self.assertNotIn("sending_mta_ip", result)
+        self.assertNotIn("receiving_ip", result)
+        self.assertNotIn("receiving_mx_hostname", result)
+        self.assertNotIn("receiving_mx_helo", result)
+        self.assertNotIn("additional_info_uri", result)
+        self.assertNotIn("failure_reason_code", result)
+
+    def testParseSmtpTlsFailureDetailsAdditionalInformationKey(self):
+        """RFC 8460 §4.4's JSON key is "additional-information" (its value
+        is described as an "additional-info-uri"); it is read into the
+        parser's additional_info_uri field.
+        """
+        details = {
+            "result-type": "starttls-not-supported",
+            "failed-session-count": 1,
+            "additional-information": "https://example.com/rfc-key-info",
+        }
+        result = parsedmarc._parse_smtp_tls_failure_details(details)
+        self.assertEqual(
+            result["additional_info_uri"], "https://example.com/rfc-key-info"
+        )
+
+    def testParseSmtpTlsFailureDetailsAdditionalInformationWinsOverLegacy(self):
+        """When both the RFC key and the legacy key are present and
+        non-blank, the RFC key's value wins."""
+        details = {
+            "result-type": "starttls-not-supported",
+            "failed-session-count": 1,
+            "additional-information": "https://example.com/rfc-key-info",
+            "additional-info-uri": "https://example.com/legacy-key-info",
+        }
+        result = parsedmarc._parse_smtp_tls_failure_details(details)
+        self.assertEqual(
+            result["additional_info_uri"], "https://example.com/rfc-key-info"
+        )
+
+    def testParseSmtpTlsFailureDetailsAdditionalInformationBlankFallsBack(self):
+        """A blank RFC-key value falls back to a non-blank legacy value."""
+        details = {
+            "result-type": "starttls-not-supported",
+            "failed-session-count": 1,
+            "additional-information": "",
+            "additional-info-uri": "https://example.com/legacy-key-info",
+        }
+        result = parsedmarc._parse_smtp_tls_failure_details(details)
+        self.assertEqual(
+            result["additional_info_uri"], "https://example.com/legacy-key-info"
+        )
+
     def testParseSmtpTlsFailureDetailsMissingRequired(self):
         """Missing required field raises InvalidSMTPTLSReport"""
         with self.assertRaises(parsedmarc.InvalidSMTPTLSReport):

tests/test_opensearch.py

@@ -8,10 +8,11 @@ queries, error wrapping — without needing a running OpenSearch cluster.
 
 import time
 import unittest
+from typing import cast
 from unittest.mock import MagicMock, call, patch
 
 import parsedmarc.opensearch as opensearch_module
-from parsedmarc import InvalidFailureReport
+from parsedmarc import InvalidFailureReport, parse_report_file
 from parsedmarc.opensearch import (
     AlreadySaved,
     OpenSearchError,
@@ -1477,6 +1478,46 @@ class TestSaveSmtpTlsReport(unittest.TestCase):
             "https://reports.example.com/tls-help",
         )
 
+    def test_save_blank_failure_detail_fields_omitted_from_ip_fields(self):
+        """Blank optional failure-detail fields must not reach the Ip()
+        fields (issue #915).
+
+        samples/smtp_tls/empty_failure_detail_fields.json reproduces a
+        real reporter's shape: sending-mta-ip/receiving-ip sent as "" on
+        an sts-policy-fetch-error. Verified directly against opensearchpy:
+        Document.save() validates by calling full_clean() before any
+        network call, and an Ip() field's
+        validation calls Python's ipaddress.ip_address(""), which raises
+        a bare ValueError ("'' does not appear to be an IPv4 or IPv6
+        address") rather than a caught ValidationException -- so an empty
+        string here previously discarded the whole report instead of
+        indexing it with a blank IP.
+
+        save() is mocked (per this module's convention, so no real
+        cluster is needed), which also means it never runs full_clean()
+        itself; call it explicitly on the captured document so this test
+        exercises the same validation a real save() would, and fails with
+        that ValueError on unfixed parsedmarc/__init__.py.
+        """
+        result = parse_report_file(
+            "samples/smtp_tls/empty_failure_detail_fields.json", offline=True
+        )
+        report = cast(dict, result["report"])
+        with (
+            patch("parsedmarc.opensearch.Search", return_value=_empty_search()),
+            patch("parsedmarc.opensearch.Index"),
+            patch.object(
+                opensearch_module._SMTPTLSReportDoc, "save", autospec=True
+            ) as mock_save,
+        ):
+            save_smtp_tls_report_to_opensearch(report)
+        doc = mock_save.call_args[0][0]
+        doc.full_clean()
+        failure_detail = doc.policies[0].failure_details[0]
+        self.assertIsNone(failure_detail.sending_mta_ip)
+        self.assertIsNone(failure_detail.receiving_ip)
+        self.assertEqual(failure_detail.failure_reason_code, "no-policy-served")
+
 
 class TestBackwardCompatAlias(unittest.TestCase):
     def test_save_forensic_alias_points_to_save_failure(self):

tests/test_postgres.py

@@ -658,6 +658,46 @@ class TestPostgreSQLClientSave(unittest.TestCase):
         self.assertIn(100, policy_params)
         self.assertIn(2, policy_params)
 
+    def test_save_smtp_tls_report_blank_failure_detail_fields_become_null(self):
+        """Blank optional failure-detail fields reach PostgreSQL as None,
+        not "" (issue #915).
+
+        samples/smtp_tls/empty_failure_detail_fields.json reproduces a
+        real reporter's shape: sending-mta-ip/receiving-ip/
+        receiving-mx-hostname/additional-information sent as "" on an
+        sts-policy-fetch-error. "" is not a valid PostgreSQL INET value, so
+        before the parser fix in parsedmarc/__init__.py this reached
+        cur.execute() as an empty string and crashed the save with
+        "invalid input syntax for type inet: \"\"" against a real server.
+        This test goes through the real parser (parsedmarc.parse_report_file)
+        rather than a hand-built dict, so it fails on unfixed
+        parsedmarc/__init__.py even though this test file only mocks
+        psycopg.
+        """
+        client, mock_conn = _make_client()
+        cur = _mock_cursor(mock_conn, [(1,), (10,)])
+
+        result = parsedmarc.parse_report_file(
+            "samples/smtp_tls/empty_failure_detail_fields.json", offline=True
+        )
+        report = result["report"]
+
+        client.save_smtp_tls_report_to_postgresql(cast(dict, report))
+
+        sqls = _executed_sql(cur)
+        self.assertIn("smtp_tls_failure_detail", sqls[2])
+
+        detail_params = _named_params(cur.execute.call_args_list[2])
+        self.assertIsNone(detail_params["sending_mta_ip"])
+        self.assertIsNone(detail_params["receiving_ip"])
+        self.assertIsNone(detail_params["receiving_mx_hostname"])
+        self.assertIsNone(detail_params["additional_info_uri"])
+        # A non-blank optional field is preserved.
+        self.assertEqual(detail_params["failure_reason_code"], "no-policy-served")
+        # Required fields are preserved.
+        self.assertEqual(detail_params["result_type"], "sts-policy-fetch-error")
+        self.assertEqual(detail_params["failed_session_count"], 1)
+
     def test_save_smtp_tls_report_already_saved(self):
         """AlreadySaved is raised when ON CONFLICT returns no row."""
         client, mock_conn = _make_client()