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

Hardening for 11.2.2: attempts file, parse timeout, worker handling, three fixes (#945)

Browse Source 
* Wrap the failure-report duplicate search in the output error

The aggregate and SMTP TLS saves turn a failed duplicate-check search
into ElasticsearchError or OpenSearchError; the failure-report saves
let the search library's exception escape instead.

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

* Refuse date strings too long to be dates

human_timestamp_to_datetime() had no length limit; a digit run of a
million characters took 12 s in dateutil. Add a 128 character limit
(ValueError) and map it to the matching Invalid*Report at the failure
Arrival-Date, the message Date header and the failure-report sample
Date in the Elasticsearch and OpenSearch saves.

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

* Skip failure sample headers with an empty name

The Elasticsearch and OpenSearch failure saves normalise each header
name; a name that is empty after that is not a valid field name, so
leave the header out and save the rest.

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

* Add a persisted store of unfinished parse attempts

A message whose parse kills the process (the out-of-memory killer, a
container memory cap) is never moved out of the reports folder, so every
later run parses it and dies again. parsedmarc.attempts keeps a small
JSON file counting, per message, the parse attempts that started but
never finished, so the mailbox loop can set such a message aside after
MAX_PARSE_ATTEMPTS. The key hashes the first 64 KiB of the message
because mailsuite's IMAP backend does not expose UIDVALIDITY, so a
reused UID must not inherit an old count. Writes are atomic and owner
only, and a failed write turns the store off instead of stopping a run.

Refs GHSA-qcc2-4x3v-v4g7, GHSA-q7gm-8cj9-94hr, GHSA-4272-4r8g-6c5h

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

* Set aside mailbox messages that keep killing the parse

A message whose parse ends the process stays in the reports folder, so
every one-shot run or service restart parsed it again and died before
reaching the reports behind it. The new [mailbox] attempts_file option
records each message in an AttemptStore before it is parsed and clears
the entry once the parse returns a report or raises ParserError, so an
entry left behind marks a parse that never finished; a message with
MAX_PARSE_ATTEMPTS of them is disposed of as invalid without parsing.
The entry is cleared before the save, so a failed save is never counted
as a crash, and only after a successful move, so a failed move is
retried.

With n_procs > 1 a dead run had a whole submission window in flight, so
counting every message submitted beside the culprit would set innocent
reports aside on the third run. A message with one unfinished attempt is
therefore parsed alone in the calling process after the rest of the
batch, and a second death counts against it only.

PARSEDMARC_MAILBOX_ATTEMPTS_FILE names the file directly, so the key
joins _DIRECT_FILE_KEYS; without it the variable was read as a
Docker-secret reference.

Refs GHSA-qcc2-4x3v-v4g7, GHSA-q7gm-8cj9-94hr, GHSA-4272-4r8g-6c5h

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

* Stop a parallel_map job that runs past a timeout

The parse limits added for GHSA-qcc2-4x3v-v4g7 bound every input known
to parse slowly, but nothing bounds a parse they do not cover: with
n_procs = 1 the rest of the mailbox waits behind it. parallel_map() now
takes parse_timeout and timeout_result. A job that runs that long gets
timeout_result(job, parse_timeout) in its place; since a
ProcessPoolExecutor cannot stop one job, every worker is killed, a new
pool is started and the other unfinished jobs are resubmitted.

A job's clock starts when its worker reports that it began, through a
queue the pool initializer hands each worker. Future.running() is not
enough: the executor marks one job more than it has workers as running,
so a job queued behind a slow neighbour would be charged for the wait.

ParseTimeoutError subclasses InvalidDMARCReport so the mbox and mailbox
loops treat a timed-out message as invalid without changes.

Refs GHSA-qcc2-4x3v-v4g7

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

* Add parse_timeout to the mbox, mailbox and watch functions

get_dmarc_reports_from_mbox(), get_dmarc_reports_from_mailbox() and
watch_inbox() take parse_timeout and pass it to parallel_map() with the
email job's timeout value. A timeout set with n_procs = 1 also takes the
parallel_map() branch, since only a parse in a worker process can be
stopped. The mailbox function and watch_inbox() reject an invalid value
before fetching or watching, as they do for max_unsaved_retries: raised
inside a watch check, the error would be retried on every check.

Refs GHSA-qcc2-4x3v-v4g7

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

* Add the [general] parse_timeout option

The CLI reads parse_timeout as seconds, rejects a negative or
non-numeric value as a ConfigurationError, defaults it to 0 (no limit),
and passes it to the report-file, mbox, mailbox and watch calls. The
watch call reads it from opts, so a SIGHUP reload takes a new value.
The default stays 0 because the parse limits already bound the known
inputs and a timeout routes every n_procs = 1 run through a worker
process; the documentation suggests 300 seconds.

Refs GHSA-qcc2-4x3v-v4g7

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

* Document the attempts_file option

Describe [mailbox] attempts_file in the option list and in a new usage
section on messages that crash parsedmarc, say in the Unsaved note that
the failed-save counts are the in-memory ones, keep the file in a state
directory in the systemd example and in a named volume in the Docker
Compose example, and add the Security changelog entry for the
retry-loop backstop the three advisories deferred.

Refs GHSA-qcc2-4x3v-v4g7, GHSA-q7gm-8cj9-94hr, GHSA-4272-4r8g-6c5h

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

* List parse_timeout and attempts_file as orchestration options

ParserConfig's docstring and the usage page's ParserConfig notes name the
keyword arguments that stay outside the dataclass. The two new options
are such arguments, so a library caller looking for them on ParserConfig
is told where they go.

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

* Bound a suspect message's parse by parse_timeout

With an attempts file, a message that was being parsed when an earlier
run died is parsed alone after the rest of its batch, so that a second
death is charged to it and not to the messages parsed beside it. That
parse ran in the main process, where parse_timeout cannot stop it, so the
one message most likely to be slow or fatal was the one the timeout did
not cover. With a timeout set, each suspect now gets a worker pool of its
own, one message per pool so a crash is still attributed to it alone,
and its result is taken the same way as the batch's. Without a timeout
the suspect is still parsed in this process.

Refs GHSA-qcc2-4x3v-v4g7

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

* Restore default signal handlers in parsing workers

Under the fork start method a worker inherits the parent's signal
handlers, and the CLI installs SIGTERM and SIGINT handlers that only set
a flag. When a worker dies, ProcessPoolExecutor terminates the others
with SIGTERM and joins them; a worker running the inherited handler
ignores that signal, so the teardown waited forever and the run hung
instead of failing. The pool initializer now resets both signals to
their defaults. The logging tests that call the initializer in the test
process put the process's own handlers back afterwards.

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

* Order the Unreleased changelog and tie its entries together

The three branches each added to the Unreleased entry. The GHSA-q7gm
bullet still promised a follow-up for a persisted failed-attempt marker,
which the attempts_file option now is, so it names the option. The
attempts_file bullet now says that a suspect message is parsed in a
worker of its own when parse_timeout is set, matching the code. The
#942 fix moves up beside the other fix already on master, and the
worker signal-handler fix gets its own line.

Refs GHSA-qcc2-4x3v-v4g7

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

* Ignore SIGINT in workers and end the batch when one dies

Resetting SIGINT to its default in the workers made a Ctrl-C, which
the terminal sends to the whole process group, kill every worker, so
the run ended in BrokenProcessPool where the CLI's handler meant to
let the batch finish. Workers now ignore SIGINT and keep the default
SIGTERM action, which ProcessPoolExecutor needs to tear down a broken
pool.

A worker that dies for another reason (the out-of-memory killer, a
crash) raised BrokenProcessPool out of parallel_map, which no caller
handled: the file and mbox paths ended in a traceback. parallel_map
now kills the remaining workers, closes the start queue and raises
ParserError from it, and the CLI file and mbox paths log it and exit 1
as the mailbox paths already did. The in-flight messages keep their
attempts-file entries, so the next run parses each alone.

The systemd example gains KillMode=mixed so stopping the service
signals only the main process and the batch in progress finishes.

Refs GHSA-qcc2-4x3v-v4g7

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

* State the parse_timeout bounds the code actually gives

A slow job running beside another that expires loses its partial run
and starts again, so P slow jobs cost one more round of
parse_timeout than the docstring said, plus the bounded joins of the
killed workers, during which no heartbeat is sent. The clocks are
also checked only while the parent waits for a result, so "at most
one second past the timeout" was not true while the caller is busy
between results; the docstring, usage.md and the changelog now say a
parse is stopped within one wait slice of the parent next waiting.

ParseTimeoutError's docstring says it is the value for every timed-out
input, SMTP TLS and unrecognised ones included. A new test closes the
results iterator mid-run and checks the workers are killed and the
start queue closed; the zero-timeout test now observes which pool ran
the job instead of patching _DeadlinePool.

Refs GHSA-qcc2-4x3v-v4g7

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

* Correct the attempts-file wording and test the parallel test mode

Suspect isolation happens whenever the mailbox takes the parallel path,
which parse_timeout selects even with n_procs = 1; the docstring,
comment, usage.md and changelog now say so. The set-aside log said a
message "has crashed parsedmarc {count-1} time(s)", which overstates
the crashes once a failed move has added attempts; it now reports the
unfinished attempts against the cap.

usage.md lists attempts_file among the _file keys read directly from
the environment and among the settings a SIGHUP reloads, and says that
a failed write clearing an entry after a good parse can lead to a
healthy message being set aside after two such failures. The test-mode
set-aside test now runs the parallel path too.

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

* Measure a date's length after its comments are removed

The length guard in human_timestamp_to_datetime() ran before the
RFC 5322 comments were stripped. RFC 5322 section 3.2.2 puts no limit
on a comment's length, so a valid Date header with a long comment was
refused, and through the sample Date path the whole report email with
it. The check now runs on the stripped value, which is linear to
produce; a long digit run survives stripping, so it is still refused
before dateutil sees it.

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

* Save parsed reports before exiting on a dead worker

A ParserError from parallel_map (a worker process that died) or from an
mbox made the CLI exit at once, discarding every report parsed so far;
report files already moved under archive_directory were then saved
nowhere. The CLI now stops reading input, saves what it gathered, and
exits with status 1.

Workers ignore SIGINT, so the plain pool path's executor shutdown on an
early exit (KeyboardInterrupt, closing the generator) waited for the
whole submission window. Any early exit now kills the workers, as the
timed path already did.

The Ctrl-C test ran under fork only, where a worker inherits the test's
own harmless handler, so it passed without the SIGINT line. It now runs
under every available start method and signals each worker once it has
started a job.

Refs GHSA-qcc2-4x3v-v4g7

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

* Stop counting a message that is already set aside

A message at the attempt limit that stayed in the folder (a failed move,
test mode) was recorded again on every later run, so the logged count
grew without bound. It is now set aside without being recorded, and the
log names the stored count and the limit.

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

* Describe worker death per mode and the systemd stop window

A dead worker ends a one-shot run with status 1 after saving, but in
watch mode the IMAP and Maildir watchers log it and check again, while
Graph and Gmail exit. KillMode=mixed lets the batch finish only within
TimeoutStopSec. Also say that queued messages keep attempt entries too,
and re-wrap a parse_timeout note line.

Refs GHSA-qcc2-4x3v-v4g7

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

* Clear the seen report IDs around the worker-death CLI tests

The new save-before-exit test parses a sample in the parent, which
records it in the shared dedup set, so a later test of the same sample
saw it as a duplicate and sent no results email.

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

* Save file reports when a mailbox fetch ends in a ParserError

A worker that died during the one-shot mailbox fetch reached the
ParserError handler, which exited before the reports from report files
and mbox inputs were saved; files already moved under archive_directory
were then kept nowhere. The handler now saves them first. A second test
covers the input-error path: no mailbox is opened and the parsed file
stays under archive_directory.

Refs GHSA-qcc2-4x3v-v4g7

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

* Say what a mailbox run keeps when a worker dies

Earlier batches are saved and the batch in progress stays in the
mailbox. Also say that a set-aside message is not recorded again, so its
entry ages out after 30 days, and rewrap two paragraphs.

Refs GHSA-qcc2-4x3v-v4g7

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

* Say which set-aside entries age out of the attempts file

The 30-day drop applies only to an entry that stays: a failed move, or
a message left in place in test mode. The sentence read as if every
set-aside message kept an entry, contradicting the line before it.

Refs GHSA-qcc2-4x3v-v4g7

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Carry the test attempts path in the message, not the environment

test_parallel_path_records_each_attempt_before_submitting_it passed the
attempts file's path to its worker job through os.environ. A forkserver
worker forks from a server started before the patch, so it never saw the
variable and the job raised KeyError (Python 3.14's Linux default). The
path now travels in a header line of the message itself, which reaches
the worker under every start method.

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

* Test the worker-side helpers and the save-failure exits

Codecov listed added lines no test reached: the timed pool's initializer
and job wrapper, which run only in workers; _kill_workers on an executor
without _processes; the plain path's heartbeat; the CLI's progress bar;
the mbox loop's stop after a file worker dies; and a failed save before
exiting, on the file/mbox path and the mailbox path.

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

* Read start tokens every round, not only while waiting

With parse_timeout set, the parent read the workers' start tokens only
inside its wait loop. When results were already done at harvest time it
never waited, so each finished job stayed in the pool's token map with
its whole input, and the map grew with the run past the submission
window. The tokens are now read at the top of every round, and close()
clears the map.

Refs GHSA-qcc2-4x3v-v4g7

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

* Kill the parsing workers on a second Ctrl-C

The second Ctrl-C ends parsedmarc with os._exit(130), which skips the
process pool's cleanup. Workers now ignore SIGINT, so under every start
method a running worker survived as an orphan and kept parsing, without
end when parse_timeout is off. The handler now kills the active child
processes first; Python runs it in the main thread between bytecodes,
so active_children() and Process.kill() are safe there.

Refs GHSA-qcc2-4x3v-v4g7

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

* Hand timed results back through files, not the pool's pipe

With parse_timeout set, a timeout kills every worker, possibly one that
is sending a large result. The executor's management thread then waits
for the rest of that message forever; the bounded join returns with the
thread alive, and because it is not a daemon the interpreter hangs at
exit. Reproduced under fork, forkserver and spawn with a 256-400 MiB
result. Workers of the timed pool now pickle each outcome, error values
included, to a file in a private directory the pool owns and return
None; every message left on a pipe is far below PIPE_BUF, which POSIX
writes atomically. The pool without a timeout never kills a running
worker and is unchanged.

Refs GHSA-qcc2-4x3v-v4g7

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

* Report an unwritable temporary directory as a ParserError

With parse_timeout set the results travel through a temporary
directory. If it could not be created, or a worker could not write its
result file, a raw OSError escaped parallel_map, which the CLI does not
handle. The pool now raises ParserError naming the cause; a worker that
cannot write its file returns the error text through the executor, still
a small message, and the parent raises the same ParserError.

Refs GHSA-qcc2-4x3v-v4g7

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

* Keep every hand-back failure a short ParserError

A result the worker could not pickle (PicklingError, TypeError,
RecursionError, MemoryError) escaped through the executor with a
traceback of any size, breaking the short-message guarantee the timed
pool relies on, and a result file that did not unpickle raised a raw
exception in the parent. The worker now catches any failure to write
its outcome and returns a short marker, its text cut to 200 characters
so the message stays below the 512-byte minimum PIPE_BUF; the parent
turns it, and any failure to load a result file, into a ParserError.
Also corrects the PIPE_BUF figure in the module docstring, names the
temporary directory as TMPDIR, and avoids a second 128 MiB object in a
test.

Refs GHSA-qcc2-4x3v-v4g7

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

* Cap the hand-back error text in bytes and survive a failing __str__

The pipe's atomic-write bound is in bytes, so a 200-character text of
multibyte characters could exceed it; the cap now applies to the UTF-8
encoding. An exception whose __str__ raises is named by its type alone,
so nothing in the worker's error path can raise through the executor. A
failed write's .tmp file is removed with the pool's result directory;
the docstring says so.

Refs GHSA-qcc2-4x3v-v4g7

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Match the pickler's message across Python versions in a test

The unpicklable-result test asserted "Can't get local object", which is
the wording of Python 3.12 and 3.13 only; 3.10, 3.11 and 3.14 say
"Can't pickle local object". Assert the part they share.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Cover the worker-side hand-back failures in the test process

The three failure branches of _run_timed_job and _take_outcome run in
worker processes in the pool tests, where coverage cannot see them;
call them directly here with an unwritable directory, an unpicklable
result and an unreadable result file.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Assert the unreadable-result message by its text, and format

The temporary-directory error carries the OSError's text, not its
class name, so the test asserts "Is a directory"; one long test line
is wrapped as ruff format wants.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Narrow the marker before unpacking it in two tests

pyright cannot narrow Optional through assertIsNotNone; a plain assert
does.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Sean WhalenandClaude Fable 5.1 authored and GitHub committed 2026-11-09 20:54:27 -05:00
1 parent 1c7f2785e5
commit e8292bef70
17 files changed
+4050 -128
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

tests/test_init.py
+640
View File
@@ -42,7 +42,9 @@ from lxml import etree # type: ignore[import-untyped]
import parsedmarc
import parsedmarc.constants as constants
from parsedmarc.attempts import MAX_PARSE_ATTEMPTS, attempt_key
from parsedmarc.mail import IMAPConnection, MaildirConnection, MSGraphConnection
from parsedmarc.parallel import _parse_report_email_job
from parsedmarc.types import (
AggregateReport,
FailureReport,
@@ -3500,6 +3502,14 @@ This is not a DMARC report."""
parsedmarc.parse_report_email(email_str, offline=True)
self.assertIn("not-a-real-date", str(ctx.exception))
def testOverlongDateHeaderIsInvalidDMARCReport(self):
"""A Date header too long to be a date is an InvalidDMARCReport
naming the header, where it was a bare ParserError"""
email_str = "From: a@b.c\nDate: " + "1" * 200 + "\nSubject: x\n\nbody"
with self.assertRaises(parsedmarc.InvalidDMARCReport) as ctx:
parsedmarc.parse_report_email(email_str, offline=True)
self.assertIn("Date header", str(ctx.exception))
def testFailureTextReportParses(self):
"""A valid legacy text/plain failure report parses to a failure
report (the success path that builds the synthetic feedback report
@@ -3768,6 +3778,21 @@ Test body"""
)
self.assertIsNone(report["version"])
def testOverlongArrivalDateIsInvalidFailureReport(self):
"""An Arrival-Date too long to be a date is an InvalidFailureReport
naming the field, not an "Unexpected error" from the date parser"""
report_str = self._make_feedback_report().replace(
"Arrival-Date: Thu, 1 Jan 2024 00:00:00 +0000",
"Arrival-Date: " + "1" * 200,
)
self.assertIn("Arrival-Date: 111", report_str)
with self.assertRaises(parsedmarc.InvalidFailureReport) as ctx:
parsedmarc.parse_failure_report(
report_str, self._make_sample(), self._default_msg_date(), offline=True
)
self.assertIn("Arrival-Date", str(ctx.exception))
self.assertNotIn("Unexpected error", str(ctx.exception))
def testMissingUserAgent(self):
"""Missing user_agent defaults to None"""
report_str = self._make_feedback_report()
@@ -4478,6 +4503,27 @@ class TestGetDmarcReportsFromMailboxValidation(unittest.TestCase):
self.assertIn("max_unsaved_retries", str(ctx.exception))
conn.watch.assert_not_called()
def test_invalid_parse_timeout_raises_before_fetching_or_watching(self):
"""Like max_unsaved_retries: raised inside a watch check, the
ValueError would be swallowed and retried on every check."""
for parse_timeout in (-1.0, float("nan"), float("inf")):
with self.subTest(parse_timeout=parse_timeout):
conn = MagicMock()
with self.assertRaises(ValueError) as ctx:
parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, parse_timeout=parse_timeout
)
self.assertIn("parse_timeout", str(ctx.exception))
conn.fetch_messages.assert_not_called()
with self.assertRaises(ValueError) as ctx:
parsedmarc.watch_inbox(
mailbox_connection=conn,
callback=lambda batch: None,
parse_timeout=parse_timeout,
)
self.assertIn("parse_timeout", str(ctx.exception))
conn.watch.assert_not_called()
class TestMigrateForensicArchiveFolderErrorHandling(unittest.TestCase):
"""The one migration scenario a real on-disk Maildir can't reproduce: a
@@ -4758,6 +4804,116 @@ class TestGetDmarcReportsFromMailboxContentErrors(unittest.TestCase):
self.assertGreater(exercised, 0)
_SLOW_MESSAGE = (
"From: reporter@example.net\r\nTo: dmarc@example.com\r\n"
"Subject: slow to parse\r\nX-Test-Slow: yes\r\n\r\nbody\r\n"
)
def _parallel_map_timing_out(calls):
"""An in-process stand-in for ``parsedmarc.parallel.parallel_map`` that
runs the real job function on every message except the one carrying
``X-Test-Slow``, for which it yields ``timeout_result``, as the real
``parallel_map`` does for a job that ran past ``parse_timeout``. Each
call's keyword arguments and ``n_procs`` are appended to ``calls``."""
def fake_parallel_map(func, jobs, n_procs, **kwargs):
calls.append(dict(kwargs, n_procs=n_procs))
for job in jobs:
content = job if isinstance(job, bytes) else job.encode()
if b"X-Test-Slow: yes" in content:
yield kwargs["timeout_result"](job, kwargs["parse_timeout"])
else:
yield func(job)
return fake_parallel_map
class TestParseTimeoutDisposal(unittest.TestCase):
"""A message whose parse ran past ``parse_timeout`` is handled like any
other invalid message: moved to the invalid folder by the mailbox loop,
skipped with a warning by the mbox loop. With a timeout set, ``n_procs``
1 takes the ``parallel_map`` branch, since only a worker process can be
stopped. ``parallel_map`` is replaced by a stand-in that times out the
marked message, because a real parse that slow needs an input the parse
limits do not already reject; the timeout itself is tested in
``tests/test_parallel.py``."""
def setUp(self):
parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear()
self.addCleanup(parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear)
with open("samples/aggregate/twilight.eml", encoding="utf-8") as f:
self.aggregate = f.read()
def test_mailbox_message_that_times_out_moves_to_invalid(self):
calls = []
conn = _InMemoryIMAPConnection({1: _SLOW_MESSAGE, 2: self.aggregate})
with (
patch("parsedmarc.parallel.parallel_map", _parallel_map_timing_out(calls)),
self.assertLogs("parsedmarc.log", level="WARNING") as logs,
):
result = parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, offline=True, n_procs=1, parse_timeout=1.0
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(conn.folders["INBOX"], {})
self.assertEqual(list(conn.folders["Archive/Invalid"]), [1])
self.assertEqual(list(conn.folders["Archive/Aggregate"]), [2])
self.assertTrue(
any(
"longer than parse_timeout (1 seconds)" in line for line in logs.output
),
logs.output,
)
self.assertEqual(len(calls), 1)
self.assertEqual(calls[0]["n_procs"], 1)
self.assertEqual(calls[0]["parse_timeout"], 1.0)
self.assertEqual(calls[0]["heartbeat"], conn.keepalive)
def test_mailbox_without_timeout_and_one_process_parses_in_process(self):
calls = []
conn = _InMemoryIMAPConnection({2: self.aggregate})
with patch("parsedmarc.parallel.parallel_map", _parallel_map_timing_out(calls)):
result = parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, offline=True, n_procs=1
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(calls, [])
def test_mbox_message_that_times_out_is_skipped_with_a_warning(self):
tmp = mkdtemp()
self.addCleanup(rmtree, tmp, ignore_errors=True)
path = os.path.join(tmp, "reports.mbox")
box = mailbox.mbox(path)
box.lock()
try:
box.add(mailbox.mboxMessage(_SLOW_MESSAGE))
box.add(mailbox.mboxMessage(self.aggregate))
box.flush()
finally:
box.unlock()
box.close()
calls = []
with (
patch("parsedmarc.parallel.parallel_map", _parallel_map_timing_out(calls)),
self.assertLogs("parsedmarc.log", level="WARNING") as logs,
):
result = parsedmarc.get_dmarc_reports_from_mbox(
path, offline=True, n_procs=1, parse_timeout=1.0
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertTrue(
any(
"longer than parse_timeout (1 seconds)" in line for line in logs.output
),
logs.output,
)
self.assertEqual(len(calls), 1)
self.assertEqual(calls[0]["n_procs"], 1)
self.assertEqual(calls[0]["parse_timeout"], 1.0)
class _FailingFetchMaildirConnection(MaildirConnection):
"""A MaildirConnection whose every fetch fails like a dropped network
connection would."""
@@ -5696,6 +5852,490 @@ class TestGetDmarcReportsFromMailboxMaildirSaveCallback(unittest.TestCase):
self.assertEqual(len(conn.fetch_messages("Archive/Aggregate")), 1)
# Read by the worker job below, which runs in another process and so
# cannot be handed the test's temporary path any other way.
# The header that carries the attempts file's path to the worker job inside
# the message itself, the one input that reaches a worker under every start
# method (a spawned or forkserver worker inherits no environment changes or
# patches made in the test process). The parser ignores an unknown header.
_ATTEMPTS_FILE_HEADER = "X-Test-Attempts-File"
def _with_attempts_file_header(content, path):
"""Return ``content`` with the ``_ATTEMPTS_FILE_HEADER`` line before it."""
return f"{_ATTEMPTS_FILE_HEADER}: {path}\n{content}"
def _job_requiring_recorded_attempt(msg_content, *, config):
"""A parallel parse job that parses only a message whose attempt is
already in the attempts file its ``_ATTEMPTS_FILE_HEADER`` names, and
rejects any other as invalid.
Module level so a process pool can pickle it under every start method.
"""
text = msg_content if isinstance(msg_content, str) else msg_content.decode()
first_line = text.split("\n", 1)[0]
path = first_line.removeprefix(f"{_ATTEMPTS_FILE_HEADER}: ")
with open(path, encoding="utf-8") as f:
attempts = json.load(f)["attempts"]
digest = attempt_key("", "", msg_content).rsplit("\x1f", 1)[1]
if not any(
key.endswith("\x1f" + digest) and entry["count"] == 1
for key, entry in attempts.items()
):
return parsedmarc.InvalidDMARCReport("no attempt recorded before the job")
return _parse_report_email_job(msg_content, config=config)
class _FailingInvalidMoveIMAPConnection(_InMemoryIMAPConnection):
"""The in-memory IMAP backend, refusing every move to the Invalid
folder."""
def move_message(self, message_id, folder_name):
if folder_name.endswith("/Invalid"):
raise RuntimeError("server said no")
super().move_message(message_id, folder_name)
class _ArrivalIMAPConnection(_InMemoryIMAPConnection):
"""The in-memory IMAP backend, delivering one more message right after
its first fetch_messages() call, so batch_size=0 recurses once."""
def __init__(self, messages, extra_id, extra_content):
super().__init__(messages)
self._extra = (extra_id, extra_content)
def fetch_messages(self, reports_folder, **kwargs):
result = super().fetch_messages(reports_folder, **kwargs)
if self._extra is not None:
extra_id, extra_content = self._extra
self._extra = None
self.folders["INBOX"][extra_id] = extra_content
return result
class _SingleCheckIMAPConnection(_InMemoryIMAPConnection):
"""The in-memory IMAP backend, whose watch() runs one check."""
def watch(self, check_callback, check_timeout, config_reloading=None):
del check_timeout, config_reloading
check_callback(self)
class TestCrashedMessageAttempts(unittest.TestCase):
"""attempts_file: a message whose parse never finished (the process
died) is counted in a JSON file, and once it has died
MAX_PARSE_ATTEMPTS times it is set aside as invalid without being
parsed again, so a one-shot run no longer dies on it forever
(GHSA-qcc2-4x3v-v4g7, GHSA-q7gm-8cj9-94hr, GHSA-4272-4r8g-6c5h). The
file is read and written for real; the process deaths are simulated by
seeding the file with the count earlier runs would have left."""
AGGREGATE = "samples/aggregate/twilight.eml"
SMTP_TLS = "samples/smtp_tls/google.com_smtp_tls_report.eml"
FAILURE = "samples/failure/dmarc_ruf_report_linkedin.eml"
def setUp(self):
self._tmp = mkdtemp()
self.addCleanup(rmtree, self._tmp, ignore_errors=True)
parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear()
parsedmarc._FAILED_SAVE_ATTEMPTS.clear()
self.addCleanup(parsedmarc._FAILED_SAVE_ATTEMPTS.clear)
self.path = os.path.join(self._tmp, "attempts.json")
self.samples = {}
for name in (self.AGGREGATE, self.SMTP_TLS, self.FAILURE):
with open(name, encoding="utf-8") as sample:
self.samples[name] = sample.read()
def _seed(self, message_id, content, count):
with open(self.path, "w", encoding="utf-8") as attempts_file:
json.dump(
{
"version": 1,
"attempts": {
attempt_key("INBOX", message_id, content): {
"count": count,
"last": datetime.now(timezone.utc).isoformat(),
}
},
},
attempts_file,
)
def _attempts(self):
with open(self.path, encoding="utf-8") as attempts_file:
return json.load(attempts_file)["attempts"]
def _run(self, conn, **kwargs):
parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear()
return parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, offline=True, attempts_file=self.path, **kwargs
)
def test_attempt_is_recorded_before_the_parse_and_cleared_after(self):
content = self.samples[self.AGGREGATE]
key = attempt_key("INBOX", 1, content)
seen_during_parse = []
real_parse = parsedmarc.parse_report_email
def parse_and_look(*args, **kwargs):
seen_during_parse.append(self._attempts().get(key))
return real_parse(*args, **kwargs)
conn = _InMemoryIMAPConnection({1: content})
with patch("parsedmarc.parse_report_email", parse_and_look):
result = self._run(conn)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(seen_during_parse), 1)
self.assertIsNotNone(seen_during_parse[0])
self.assertEqual(seen_during_parse[0]["count"], 1)
self.assertEqual(self._attempts(), {})
self.assertEqual(list(conn.folders["Archive/Aggregate"]), [1])
def test_unparseable_message_clears_its_attempt(self):
"""A ParserError is a finished parse: the message goes to Invalid
as before and its attempt is not left behind to count as a crash."""
junk = "From: noise@example.com\nSubject: not a report\n\nplain text\n"
conn = _InMemoryIMAPConnection({1: junk})
with self.assertLogs("parsedmarc.log", level="WARNING"):
self._run(conn)
self.assertEqual(list(conn.folders["Archive/Invalid"]), [1])
self.assertEqual(self._attempts(), {})
def test_message_at_the_cap_is_moved_to_invalid_without_parsing(self):
crashed = self.samples[self.AGGREGATE]
healthy = self.samples[self.SMTP_TLS]
for n_procs in (1, 2):
with self.subTest(n_procs=n_procs):
self._seed(1, crashed, count=2)
parsed = []
real_parse = parsedmarc.parse_report_email
def parse_and_note(content, *args, **kwargs):
parsed.append(content)
return real_parse(content, *args, **kwargs)
conn = _InMemoryIMAPConnection({1: crashed, 2: healthy})
with patch("parsedmarc.parse_report_email", parse_and_note):
with self.assertLogs("parsedmarc.log", level="ERROR") as cm:
result = self._run(conn, n_procs=n_procs)
self.assertEqual(result["aggregate_reports"], [])
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(conn.folders["INBOX"], {})
self.assertEqual(list(conn.folders["Archive/Invalid"]), [1])
self.assertEqual(list(conn.folders["Archive/SMTP-TLS"]), [2])
self.assertIn(
"Message UID 1 in INBOX has 2 unfinished parse attempts; "
"the limit is 2. Moving it to Archive/Invalid without "
"parsing it",
"\n".join(cm.output),
)
self.assertNotIn(crashed, parsed)
if n_procs == 1:
self.assertEqual(parsed, [healthy])
# Out of the folder, so the entry is gone.
self.assertEqual(self._attempts(), {})
def test_message_below_the_cap_is_parsed_again(self):
"""One unfinished attempt (a crash for any reason) is not enough:
the message is parsed again, and a successful parse clears it."""
content = self.samples[self.AGGREGATE]
self._seed(1, content, count=1)
conn = _InMemoryIMAPConnection({1: content})
result = self._run(conn)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(list(conn.folders["Archive/Aggregate"]), [1])
self.assertEqual(self._attempts(), {})
def test_message_at_the_cap_is_deleted_with_delete_invalid(self):
content = self.samples[self.AGGREGATE]
for n_procs in (1, 2):
with self.subTest(n_procs=n_procs):
self._seed(1, content, count=2)
conn = _InMemoryIMAPConnection(
{1: content, 2: self.samples[self.SMTP_TLS]}
)
with self.assertLogs("parsedmarc.log", level="ERROR") as cm:
self._run(conn, n_procs=n_procs, delete_invalid=True)
self.assertEqual(conn.folders["INBOX"], {})
self.assertEqual(conn.folders["Archive/Invalid"], {})
self.assertEqual(conn.folders["Archive/Aggregate"], {})
self.assertIn(
"has 2 unfinished parse attempts; the limit is 2. "
"Deleting it without parsing it",
"\n".join(cm.output),
)
self.assertEqual(self._attempts(), {})
def test_failed_save_is_not_counted_as_a_crash(self):
"""The attempt is cleared when the parse returns, before the save,
so a batch an output keeps rejecting stays in the folder for the
Unsaved retry logic and is never set aside as a crashed message."""
content = self.samples[self.AGGREGATE]
conn = _InMemoryIMAPConnection({1: content})
for _ in range(MAX_PARSE_ATTEMPTS + 2):
with self.assertLogs("parsedmarc.log", level="ERROR"):
self._run(
conn, save_callback=lambda batch: False, max_unsaved_retries=10
)
self.assertEqual(self._attempts(), {})
self.assertEqual(list(conn.folders["INBOX"]), [1])
self.assertEqual(conn.folders["Archive/Invalid"], {})
def test_test_mode_records_and_clears_but_moves_nothing(self):
"""In the serial path and the parallel one (n_procs 2), the message
at the cap is logged and left in place, not parsed or moved."""
crashed = self.samples[self.AGGREGATE]
healthy = self.samples[self.SMTP_TLS]
junk = "From: noise@example.com\nSubject: not a report\n\nplain text\n"
for n_procs in (1, 2):
with self.subTest(n_procs=n_procs):
self._seed(1, crashed, count=2)
conn = _InMemoryIMAPConnection({1: crashed, 2: healthy, 3: junk})
with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
result = self._run(conn, test=True, n_procs=n_procs)
self.assertEqual(result["aggregate_reports"], [])
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(sorted(conn.folders["INBOX"]), [1, 2, 3])
self.assertNotIn("Archive/Invalid", conn.folders)
self.assertIn(
"Message UID 1 in INBOX has 2 unfinished parse attempts; "
"the limit is 2. Leaving it in place without parsing it "
"(test mode)",
"\n".join(cm.output),
)
# The set-aside message is still in the folder, so its count
# stays; the parsed and the unparseable ones were recorded
# and cleared.
attempts = self._attempts()
key = attempt_key("INBOX", 1, crashed)
self.assertEqual(list(attempts), [key])
self.assertEqual(attempts[key]["count"], 2)
def test_set_aside_count_does_not_grow_on_later_runs(self):
"""A message left in place at the cap (test mode here, a failed move
likewise) is not recorded again, so every later run logs the same
count instead of one more each time, in both paths."""
crashed = self.samples[self.AGGREGATE]
key = attempt_key("INBOX", 1, crashed)
for n_procs in (1, 2):
with self.subTest(n_procs=n_procs):
self._seed(1, crashed, count=2)
for _ in range(3):
conn = _InMemoryIMAPConnection({1: crashed})
with self.assertLogs("parsedmarc.log", level="ERROR") as cm:
self._run(conn, test=True, n_procs=n_procs)
self.assertIn(
"has 2 unfinished parse attempts; the limit is 2.",
"\n".join(cm.output),
)
self.assertEqual(self._attempts()[key]["count"], 2)
def test_parallel_path_records_each_attempt_before_submitting_it(self):
conn = _InMemoryIMAPConnection(
{
1: _with_attempts_file_header(self.samples[self.AGGREGATE], self.path),
2: _with_attempts_file_header(self.samples[self.SMTP_TLS], self.path),
}
)
with patch(
"parsedmarc.parallel._parse_report_email_job",
_job_requiring_recorded_attempt,
):
result = self._run(conn, n_procs=2)
# The job rejects a message whose attempt was not yet on disk, which
# would have sent it to Invalid.
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertNotIn(1, conn.folders["Archive/Invalid"])
self.assertNotIn(2, conn.folders["Archive/Invalid"])
self.assertEqual(self._attempts(), {})
def test_parallel_path_parses_a_suspect_alone_in_this_process(self):
"""With n_procs > 1 a run that died had several messages in flight,
and only one of them killed it. A message with one unfinished
attempt is therefore not submitted to the pool beside others: it is
parsed in this process after them, its attempt recorded first, so a
second death counts against it alone and the messages parsed beside
it the first time are not set aside with it."""
suspect = self.samples[self.AGGREGATE]
others = {2: self.samples[self.SMTP_TLS], 3: self.samples[self.FAILURE]}
self._seed(1, suspect, count=1)
key = attempt_key("INBOX", 1, suspect)
parsed_here = []
real_parse = parsedmarc.parse_report_email
parent = os.getpid()
def parse_and_look(content, *args, **kwargs):
if os.getpid() == parent:
parsed_here.append((content, self._attempts().get(key)))
return real_parse(content, *args, **kwargs)
conn = _InMemoryIMAPConnection({1: suspect, **others})
with patch("parsedmarc.parse_report_email", parse_and_look):
with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
result = self._run(conn, n_procs=2)
self.assertEqual(len(parsed_here), 1)
self.assertEqual(parsed_here[0][0], suspect)
self.assertEqual(parsed_here[0][1]["count"], 2)
self.assertIn(
"Message UID 1 in INBOX was being parsed when parsedmarc last "
"stopped without finishing; parsing it on its own",
"\n".join(cm.output),
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(len(result["failure_reports"]), 1)
self.assertEqual(conn.folders["INBOX"], {})
self.assertEqual(self._attempts(), {})
def _recording_parallel_map(self, calls, jobs_seen):
"""``_parallel_map_timing_out`` that also notes each job it is
given, per call, as the call consumes it."""
fake = _parallel_map_timing_out(calls)
def recording(func, jobs, n_procs, **kwargs):
seen = []
jobs_seen.append(seen)
def noted():
for job in jobs:
seen.append(job)
yield job
return fake(func, noted(), n_procs, **kwargs)
return recording
def test_suspect_parse_is_bounded_by_parse_timeout(self):
"""With parse_timeout set, a suspect is parsed alone in a worker
pool of its own rather than in this process, where the timeout
could not stop it; one that times out goes to Invalid like any
other timed-out message and its attempt is cleared."""
others = {2: self.samples[self.AGGREGATE], 3: self.samples[self.SMTP_TLS]}
for n_procs in (1, 2):
with self.subTest(n_procs=n_procs):
self._seed(1, _SLOW_MESSAGE, count=1)
calls = []
jobs_seen = []
conn = _InMemoryIMAPConnection({1: _SLOW_MESSAGE, **others})
with (
patch(
"parsedmarc.parallel.parallel_map",
self._recording_parallel_map(calls, jobs_seen),
),
self.assertLogs("parsedmarc.log", level="WARNING") as cm,
):
result = self._run(conn, n_procs=n_procs, parse_timeout=1.0)
self.assertEqual(len(calls), 2)
self.assertNotIn(_SLOW_MESSAGE, jobs_seen[0])
self.assertEqual(jobs_seen[1], [_SLOW_MESSAGE])
self.assertEqual(calls[1]["n_procs"], 1)
self.assertEqual(calls[1]["parse_timeout"], 1.0)
self.assertEqual(calls[1]["heartbeat"], conn.keepalive)
self.assertIn(
"longer than parse_timeout (1 seconds)", "\n".join(cm.output)
)
self.assertEqual(list(conn.folders["Archive/Invalid"]), [1])
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(conn.folders["INBOX"], {})
self.assertEqual(self._attempts(), {})
def test_suspect_without_parse_timeout_is_parsed_in_this_process(self):
"""Without a timeout nothing would bound a worker any better than
this process, so the suspect is not given to parallel_map."""
suspect = self.samples[self.AGGREGATE]
self._seed(1, suspect, count=1)
calls = []
jobs_seen = []
conn = _InMemoryIMAPConnection({1: suspect, 2: self.samples[self.SMTP_TLS]})
with (
patch(
"parsedmarc.parallel.parallel_map",
self._recording_parallel_map(calls, jobs_seen),
),
self.assertLogs("parsedmarc.log", level="WARNING"),
):
result = self._run(conn, n_procs=2)
self.assertEqual(len(calls), 1)
self.assertNotIn(suspect, jobs_seen[0])
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(conn.folders["INBOX"], {})
self.assertEqual(self._attempts(), {})
def test_failed_disposal_keeps_the_entry(self):
"""The entry is cleared only once the message has left the folder:
a move the server refuses leaves the count in place, so the next
run sets the message aside again instead of parsing it."""
content = self.samples[self.AGGREGATE]
key = attempt_key("INBOX", 1, content)
for n_procs in (1, 2):
with self.subTest(n_procs=n_procs):
self._seed(1, content, count=2)
conn = _FailingInvalidMoveIMAPConnection(
{1: content, 2: self.samples[self.SMTP_TLS]}
)
with self.assertLogs("parsedmarc.log", level="ERROR") as cm:
self._run(conn, n_procs=n_procs)
self.assertEqual(list(conn.folders["INBOX"]), [1])
self.assertEqual(self._attempts()[key]["count"], 2)
self.assertIn(
"Error moving invalid message UID 1", "\n".join(cm.output)
)
def test_batch_size_zero_recursion_forwards_the_attempts_file(self):
"""A message that arrives during the run is processed by the
recursive re-check call, which must use the same attempts file."""
crashed = self.samples[self.SMTP_TLS]
self._seed(2, crashed, count=2)
conn = _ArrivalIMAPConnection(
{1: self.samples[self.AGGREGATE]}, extra_id=2, extra_content=crashed
)
with self.assertLogs("parsedmarc.log", level="ERROR"):
result = self._run(conn, batch_size=0)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(result["smtp_tls_reports"], [])
self.assertEqual(list(conn.folders["Archive/Invalid"]), [2])
def test_watch_inbox_forwards_the_attempts_file(self):
crashed = self.samples[self.AGGREGATE]
self._seed(1, crashed, count=2)
conn = _SingleCheckIMAPConnection({1: crashed})
conn.create_folder("Archive/Invalid")
with self.assertLogs("parsedmarc.log", level="ERROR"):
parsedmarc.watch_inbox(
conn,
lambda batch: None,
offline=True,
attempts_file=self.path,
)
self.assertEqual(list(conn.folders["Archive/Invalid"]), [1])
self.assertEqual(self._attempts(), {})
def test_no_attempts_file_keeps_no_record(self):
content = self.samples[self.AGGREGATE]
self._seed(1, content, count=5)
conn = _InMemoryIMAPConnection({1: content})
parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear()
result = parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, offline=True
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(list(conn.folders["Archive/Aggregate"]), [1])
# The file is neither consulted nor written.
self.assertEqual(self._attempts()[attempt_key("INBOX", 1, content)]["count"], 5)
class _MidRunArrivalMaildirConnection(MaildirConnection):
"""A MaildirConnection that delivers one extra message into the INBOX
right after its first fetch_messages() call returns, simulating mail