amorfo77/parsedmarc
1
0 Fork 0
mirror of https://github.com/domainaware/parsedmarc.git synced 2026-11-07 09:19:22 +00:00
Code Issues Projects Releases Wiki Activity

Fix DNS over HTTPS under dnspython 2.9 and the misleading pre-flight error (#935) (#936)

Browse Source 
* Fix DNS over HTTPS under dnspython 2.9 and the pre-flight error (#935)

dnspython 2.9 moved its [doh] extra from httpx to httpx2, and
dns.query.https() rejects a session of any other class before sending
anything (dnspython 2.9.1 dns/query.py:551: `if session and not
isinstance(session, httpx2.Client): raise ValueError("session parameter
must be an httpx2.Client")`; 2.8.0 checks httpx.Client at :566).
_get_doh_session() always built an httpx.Client, so every https://
nameserver failed and the resolver retried it until LifetimeTimeout.

The shared DoH client is now built from the Client class of the module
dnspython itself imported (dns.query.httpx2 on 2.9+, dns.query.httpx on
2.7/2.8), so it matches dnspython's isinstance check even when httpx2 is
installed beside dnspython 2.8. httpx2 keeps httpx's
`allow_env_proxies = trust_env and transport is None` (_client.py:658),
so the #880 proxy behavior carries over.

The nameservers pre-flight check called get_reverse_dns(), which turns
every DNS error into None, so the failure above was reported as "no PTR
record for 1.1.1.1". It now calls query_dns() directly and reports
"no PTR record" only for NXDOMAIN/NoAnswer, and the resolver's actual
error otherwise.

Reported by florianutz.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address review of the DNS over HTTPS fix (#935)

- Add the httpx2 and httpcore2 loggers to the dependency loggers so
  --debug shows DoH traffic under dnspython 2.9+.
- Report an invalid dns_test_address by name and value in the
  pre-flight check instead of "Text input is malformed.", without
  attempting a query.
- Rewrap an over-long docstring line in _nameservers_to_resolver_input.
- Soften the test docstring about the no-[doh]-extra fallback.
- Correct the pyproject comment on what the [doh] extra installs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Rewrap the dnspython [doh] comment in pyproject.toml (#935)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Sean WhalenandClaude Opus 5.5 authored and GitHub committed 2026-10-31 15:16:26 -04:00
1 parent 768bf75480
commit 70f96d0734
6 files changed
+324 -64
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

tests/test_cli.py
+127 -5
View File
@@ -27,6 +27,11 @@ from types import ModuleType, SimpleNamespace
from typing import cast
from unittest.mock import MagicMock, patch
import dns.message
import dns.rcode
import dns.rdata
import dns.rdataclass
import dns.rdatatype
import httpx
import opensearchpy
from azure.core.exceptions import ClientAuthenticationError
@@ -2923,6 +2928,11 @@ client_assertion = s3cret-signed-jwt-assertion
self._run_main(cfg_path, "--debug")
parsedmarc_logger = logging.getLogger("parsedmarc.log")
# dnspython 2.9+ logs DNS over HTTPS traffic under httpx2 and
# httpcore2 (httpx and httpcore on 2.7 and 2.8), so all four must
# follow --debug.
for wanted_name in ("httpx", "httpcore", "httpx2", "httpcore2"):
self.assertIn(wanted_name, parsedmarc.cli._DEPENDENCY_LOGGERS)
for name in parsedmarc.cli._DEPENDENCY_LOGGERS:
dep = logging.getLogger(name)
self.assertEqual(dep.level, logging.DEBUG, name)
@@ -7519,7 +7529,8 @@ class TestParseConfigGeneral(unittest.TestCase):
"""A nameservers list may mix DNS over HTTPS URLs and DNS over TLS
entries with plain IP addresses (issue #880); _parse_config splits
and strips them like any other list value, and hands them to the DNS
pre-flight check (mocked here so no network is needed)."""
pre-flight check (dns.query.https is mocked so no network is
needed)."""
from parsedmarc.cli import _parse_config
cp = _config_with(
@@ -7533,18 +7544,129 @@ class TestParseConfigGeneral(unittest.TestCase):
},
)
opts = _opts()
with patch(
"parsedmarc.cli.get_reverse_dns", return_value="one.one.one.one"
) as mock_reverse_dns:
queried: list[tuple[str, str]] = []
def responder(request, where, *args, **kwargs):
queried.append((where, request.question[0].to_text()))
response = dns.message.make_response(request)
rrset = response.find_rrset(
response.answer,
request.question[0].name,
dns.rdataclass.IN,
dns.rdatatype.PTR,
create=True,
)
rrset.add(dns.rdata.from_text("IN", "PTR", "one.one.one.one."), 300)
return response
with patch("dns.query.https", side_effect=responder):
_parse_config(cp, opts)
self.assertEqual(
opts.nameservers,
["https://cloudflare-dns.com/dns-query", "tls://9.9.9.9#dns.quad9.net"],
)
# The first (DoH) nameserver answered the pre-flight PTR query for
# dns_test_address, so the DoT one was never needed
self.assertEqual(
mock_reverse_dns.call_args.kwargs["nameservers"], opts.nameservers
queried,
[
(
"https://cloudflare-dns.com/dns-query",
"1.1.1.1.in-addr.arpa. IN PTR",
)
],
)
def test_general_nameservers_pre_flight_reports_the_resolver_error(self):
"""A nameserver that fails every query is reported with the
resolver's own error, not as a missing PTR record. Issue #935: under
dnspython 2.9 every DoH query raised "session parameter must be an
httpx2.Client", and the pre-flight check said "no PTR record for
1.1.1.1", because get_reverse_dns() turns every DNS error into
None."""
from parsedmarc.cli import ConfigurationError, _parse_config
cp = _config_with(
"general",
{
"dns_test_address": "1.1.1.1",
"dns_timeout": "0.2",
"nameservers": "https://dns.example/dns-query",
},
)
error = ValueError("session parameter must be an httpx2.Client")
with patch("dns.query.https", side_effect=error):
with self.assertRaises(ConfigurationError) as ctx:
_parse_config(cp, _opts())
message = str(ctx.exception)
self.assertIn("DNS pre-flight check failed", message)
self.assertIn("session parameter must be an httpx2.Client", message)
self.assertNotIn("no PTR record", message)
def test_general_nameservers_pre_flight_reports_a_missing_ptr_record(self):
"""When the nameserver answers but has no PTR record for
dns_test_address — NXDOMAIN, or NOERROR with an empty answer
section — the pre-flight error says so and names the nameservers."""
from parsedmarc.cli import ConfigurationError, _parse_config
def nxdomain(request, *args, **kwargs):
response = dns.message.make_response(request)
response.set_rcode(dns.rcode.NXDOMAIN)
return response
def no_answer(request, *args, **kwargs):
return dns.message.make_response(request)
for name, responder in (("NXDOMAIN", nxdomain), ("NoAnswer", no_answer)):
with self.subTest(name):
cp = _config_with(
"general",
{
"dns_test_address": "1.1.1.1",
"dns_timeout": "2.0",
"nameservers": "https://dns.example/dns-query",
},
)
with patch("dns.query.https", side_effect=responder):
with self.assertRaises(ConfigurationError) as ctx:
_parse_config(cp, _opts())
self.assertIn(
"DNS pre-flight check failed: no PTR record for 1.1.1.1 "
"from ['https://dns.example/dns-query']",
str(ctx.exception),
)
def test_general_nameservers_pre_flight_rejects_invalid_test_address(self):
"""An unparseable dns_test_address is reported by setting name and
value (dns.reversename.from_address raises dns.exception.SyntaxError,
"Text input is malformed.", for non-IP text), and no DNS query is
attempted."""
from parsedmarc.cli import ConfigurationError, _parse_config
for address in ("not-an-ip", "1.1.1"):
with self.subTest(address):
cp = _config_with(
"general",
{
"dns_test_address": address,
"dns_timeout": "2.0",
"nameservers": "https://dns.example/dns-query",
},
)
with (
patch("dns.query.https") as https,
patch("dns.query.udp") as udp,
):
with self.assertRaises(ConfigurationError) as ctx:
_parse_config(cp, _opts())
self.assertEqual(
f"DNS pre-flight check failed: dns_test_address {address!r} "
"is not a valid IP address",
str(ctx.exception),
)
https.assert_not_called()
udp.assert_not_called()
def test_general_nameservers_malformed_dot_entry_fails_pre_flight(self):
"""A malformed DNS over TLS nameservers entry raises a
ConfigurationError at startup, before any mailbox work begins. No