Skip to content

Header::parseDate(): route every parse failure to fallback_date - #629

Open
gabriel-pelicano-instar wants to merge 2 commits into
Webklex:masterfrom
gabriel-pelicano-instar:fix/parse-date-fallback-and-day-of-week
Open

Header::parseDate(): route every parse failure to fallback_date#629
gabriel-pelicano-instar wants to merge 2 commits into
Webklex:masterfrom
gabriel-pelicano-instar:fix/parse-date-fallback-and-day-of-week

Conversation

@gabriel-pelicano-instar

Copy link
Copy Markdown

Problem

Header::parseDate() runs its regex switch outside of any try, so the Carbon::createFromFormat() calls inside it escape the method whenever the textual month is not English.

A pt-BR mailer sending

Date: Seg, 08 Set 2026 10:00:00 -0300

matches

case preg_match('/([A-Z]{2,4}\,\ [0-9]{1,2}\ [A-Z]{2,3}\ [0-9]{4}\ ...)+$/i', $date) > 0:
    $date = Carbon::createFromFormat("d M Y H:i:s O", trim(implode(',', $array)));

and raises Carbon\Exceptions\InvalidFormatException: A textual month could not be found.

That exception is neither InvalidMessageDateException nor one of the four Query::make() catches under soft_fail, so:

  • fallback_date is never consulted — the option exists precisely for this;
  • soft_fail does not help;
  • Query::curate_messages() catches it as Exception and rethrows GetMessagesFailedException.

The result is that the whole fetched batch dies, not just the offending message. On an incremental sync the cursor never advances and the mailbox stops being read, cycle after cycle, until someone deletes the message by hand. Reproduced on v6.2.0 with Set, Dez and Fev; the pt-BR abbreviations that collide with English (Jan, Mar, Jun, Jul, Nov) pass by luck.

A second, quieter defect: the day-of-week is redundant in RFC 2822, but PHP reads an inconsistent one as a relative weekday. Mon, 08 Sep 2026 10:00:00 -0300 (08 Sep 2026 is a Tuesday) parses as 14 Sep 2026 — six days off, with no error at all. Any mailer with a wrong weekday silently moves every message it sends.

Changes

  1. The switch and the final Carbon::parse() now share one try, and the handler catches \Throwable. Every parse failure — including the ones raised inside the switch — reaches the existing fallback_date branch; without fallback_date it still surfaces as the documented InvalidMessageDateException instead of a raw Carbon exception. The switch body is only re-indented, so git diff -w shows the real change.
  2. The day-of-week token is dropped before the first Carbon::parse(), via preg_replace('/^[A-Za-z]{2,3}\.?\s*,\s*/', '', $date). The comma is required, so a leading month in Mar 08 2026 10:00:00 -0300 is left alone; and the strip applies to that attempt only, because most patterns in the switch are anchored on the day-of-week and still need the raw header.

Tests

Four cases added to tests/HeaderTest.php:

Header Before After
Seg, 08 Set 2026 10:00:00 -0300 (with fallback_date) InvalidFormatException escapes falls back
Seg, 08 Set 2026 10:00:00 -0300 (no fallback_date) InvalidFormatException InvalidMessageDateException
Mon, 08 Sep 2026 10:00:00 -0300 2026-09-14 2026-09-08
Mar 08 2026 10:00:00 -0300 2026-03-08 2026-03-08 (unchanged)

The existing suite is green, including the 39 fixture messages with their real Date: headers (807 assertions).

I also diffed the parse result of 16 headers before and after the change: the only differences are the four rows above plus Thu, 8 Sep 2026 10:00:00 +0000 (UTC), which was likewise off by two days for the same weekday reason. Di., 15 Feb. 2022 ... (MEZ), fr., 25 nov. 2022 ..., 2026.09.08-10.00.00, 08 Sep 2026 10:00:00 UT and the foo / 0 fallbacks are untouched.

`parseDate()` runs the regex `switch` outside of any `try`, so the
`Carbon::createFromFormat()` calls inside it escape the method whenever the
textual month is not English. A pt-BR mailer sending

    Date: Seg, 08 Set 2026 10:00:00 -0300

matches the `([A-Z]{2,4}\, [0-9]{1,2} [A-Z]{2,3} [0-9]{4} ...)` case and raises
`Carbon\Exceptions\InvalidFormatException: A textual month could not be found`.
That exception is neither `InvalidMessageDateException` nor one of the ones
`Query::make()` catches under `soft_fail`, so `fallback_date` is never
consulted and `Query::curate_messages()` turns it into
`GetMessagesFailedException`: the whole fetched batch dies, not just the one
message. On an incremental sync the cursor never advances and the mailbox stops
being read, cycle after cycle.

Two changes:

1. The `switch` and the final `Carbon::parse()` now share one `try`, and the
   handler catches `\Throwable`. Any parse failure — including the ones raised
   inside the `switch` — reaches the existing `fallback_date` branch, and
   without `fallback_date` it still surfaces as the documented
   `InvalidMessageDateException` instead of a raw Carbon exception.

2. The day-of-week token is dropped before the first `Carbon::parse()`. It is
   redundant in RFC 2822, and PHP reads an inconsistent one as a *relative*
   weekday: `Mon, 08 Sep 2026` (a Tuesday) parsed as 14 Sep 2026, six days off,
   silently. The strip requires the comma, so a leading month in
   `Mar 08 2026 10:00:00 -0300` is left alone, and the regex `switch` below
   still sees the raw header, because most of its patterns are anchored on the
   day-of-week.

Tests cover both, plus the two shapes that must not regress.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the previous commit, from an adversarial review of the same diff.

1. The strip now requires the tail to still look like a date
   (`(?=[0-9]{1,2}[\s.-]+[A-Za-z]{3})`). Without it, a truncated header such as
   `Seg, 10:00:00 -0300` became a bare time, which PHP parses as *today* at
   10:00: a plausible, wrong date, and one that never reaches `fallback_date`
   because nothing throws. That was a behaviour inversion introduced by the
   previous commit, not a pre-existing bug.

2. The strip moved into `stripDayOfWeek()` and is now applied to the second
   `Carbon::parse()` as well, when `$date` is still a string. Headers that only
   become parseable after the regex switch kept the relative-weekday shift:
   `Mon, 04 Jan 2018 10:12:47 UT` (a Thursday) came back as 08 Jan, before and
   after the first commit. The switch itself still sees the raw header.

Measured before/after across 25 headers, including every "known bad" shape the
package's own regexes name: the only differences are the intended ones. Full
suite green, fixtures included (39 tests, 807 assertions).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gabriel-pelicano-instar

Copy link
Copy Markdown
Author

Pushed a follow-up commit after an adversarial review of the diff:

  1. The strip now requires the tail to still look like a date ((?=[0-9]{1,2}[\s.-]+[A-Za-z]{3})). Without that lookahead, a truncated header such as Seg, 10:00:00 -0300 became a bare time, which PHP parses as today at 10:00 — a plausible but wrong date that never reaches fallback_date, because nothing throws. That was a regression introduced by my first commit, not a pre-existing bug.

  2. The strip is applied to the second Carbon::parse() as well (extracted into stripDayOfWeek(), guarded by is_string($date)). Headers that only become parseable after the regex switch kept the relative-weekday shift: Mon, 04 Jan 2018 10:12:47 UT (a Thursday) came back as 08 Jan both before and after my first commit. The switch itself still sees the raw header, since most of its patterns are anchored on the day-of-week.

Two more tests cover the two cases. Measured before/after across 25 headers, including every "known bad" shape the package's own regexes name — the only differences are the intended ones, and the full suite stays green (fixtures included: 39 tests, 807 assertions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant