From 86eb11e395abac300c486847de13be8668395337 Mon Sep 17 00:00:00 2001 From: Matthew J Mucklo Date: Tue, 21 Jul 2026 23:37:38 -0700 Subject: [PATCH 1/2] Tighten rfc5322 dot placement, fix empty quoted local-part, test length toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three correctness/config improvements surfaced by differential testing against a reference validator and the dominicsayers/isemail gold-standard corpus. - rfc5322() now enforces dot-atom local-part structure (§3.2.3): leading, trailing, and consecutive dots are rejected, matching the real obs-local-part ABNF (§4.4, non-empty words). The permissive behavior stays available via rfc2822() or withAllowObsLocalPart(true). Obs-dot testspec cases moved from `normal` to `relaxed` mode accordingly. - Empty quoted local-part (""@domain) is now recognized as quoted, so rejectEmptyQuotedLocalPart (default false) actually controls its acceptance. The closing-quote handler records the quote explicitly; a display-name quote resets the flag so the real local-part stays unquoted. - Documented + tested that enforceLengthLimits(false) disables the 64-octet local-part limit (withLengthLimits() for custom limits) — already supported. 91 -> 94 tests, PHPStan level 8 / Psalm / cs all clean. --- CHANGELOG.md | 9 ++++++ src/Parse.php | 18 ++++++++++-- src/ParseOptions.php | 11 +++++--- tests/ParseTest.php | 65 +++++++++++++++++++++++++++++++++++++++++++- tests/testspec.yml | 22 ++++++++------- 5 files changed, 107 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71d1ab3..796d483 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Fixed +- **Empty quoted local-part** (`""@domain`): the `rejectEmptyQuotedLocalPart` option (default `false`) now actually takes effect. Previously an empty quoted local-part was rejected as `incomplete_address` by the state machine before the option was consulted, because an empty quote leaves `quote_temp` empty and the `@` handler used content-emptiness as the "was quoted" signal. The closing-quote handler now records the quote explicitly (and a display-name quote resets it so the real local-part stays unquoted). + +### Changed +- **`rfc5322()` dot-atom enforcement** (behavior change): the `rfc5322()` preset now rejects a leading, trailing, or consecutive dot in the local part (`.a@`, `a.@`, `a..b@`), enforcing dot-atom per §3.2.3. This matches the actual obs-local-part ABNF (§4.4: `word *("." word)`, words non-empty). The previous permissive behavior remains available via `rfc2822()` or `ParseOptions::rfc5322()->withAllowObsLocalPart(true)`. + +### Notes +- The RFC 5321 §4.5.3.1 length limits (64-octet local part, etc.) can be disabled wholesale with `->withEnforceLengthLimits(false)`, or customized via `->withLengthLimits(new LengthLimits(...))`. Now covered by tests. + ## [3.4.0] Performance release. The two main-loop optimizations below compound to roughly **25–30% faster parsing on typical mixed inputs**, and more on longer and batch inputs where the O(n²)→O(n) change dominates. No API or behavior changes — all existing callers are unaffected. diff --git a/src/Parse.php b/src/Parse.php index 5be9a0f..956b624 100644 --- a/src/Parse.php +++ b/src/Parse.php @@ -461,6 +461,10 @@ public function parse(string $emails, bool $multiple = true, string $encoding = $subState = self::STATE_LOCAL_PART; $emailAddress['special_char_in_substate'] = null; $emailAddress['in_angle_addr'] = true; + // Any quote before `<` was the display name, not the local part; + // clear the quoted flag the closing-quote handler set so the real + // local-part inside the angle-addr starts unquoted. + $emailAddress['local_part_quoted'] = false; $this->handleQuote($emailAddress); } } elseif ('>' == $curChar) { @@ -749,8 +753,14 @@ public function parse(string $emails, bool $multiple = true, string $encoding = // Odd number of backslashes = this quote is escaped $emailAddress['quote_temp'] .= $curChar; } else { - // Even backslashes (or zero) = this is the real closing quote + // Even backslashes (or zero) = this is the real closing quote. + // Record that a quote was seen so an *empty* quoted local-part + // (`""@domain`) is still recognised as quoted — quote_temp is + // empty in that case, so the '@' handler below can't tell. A + // display-name quote self-corrects: the real local-part resets + // this flag from address_temp_quoted when '@' is reached. $state = self::STATE_ADDRESS; + $emailAddress['local_part_quoted'] = true; } } else { $emailAddress['quote_temp'] .= $curChar; @@ -848,8 +858,10 @@ public function parse(string $emails, bool $multiple = true, string $encoding = } } - // Did we find no email addresses at all? - if (!$emailAddress['invalid'] && !count($emailAddresses) && (!$emailAddress['original_address'] || !$emailAddress['local_part_parsed'])) { + // Did we find no email addresses at all? An empty local-part only counts as + // "no address" when it is unquoted; `""@domain` is a legitimately-empty quoted + // local-part whose acceptance is decided later by rejectEmptyQuotedLocalPart. + if (!$emailAddress['invalid'] && !count($emailAddresses) && (!$emailAddress['original_address'] || (!$emailAddress['local_part_parsed'] && !$emailAddress['local_part_quoted']))) { $success = false; $reason = 'No email addresses found'; if (!$multiple) { diff --git a/src/ParseOptions.php b/src/ParseOptions.php index eba2524..cae6b02 100644 --- a/src/ParseOptions.php +++ b/src/ParseOptions.php @@ -138,15 +138,18 @@ public static function rfc6531(): self /** * RFC 5322 addr-spec — recommended default for new code. * - * Follows RFC 5322 §3.4.1 including obs-local-part (§4.4): permissive dot - * placement. Generators MUST NOT produce obs-local-part, but parsers MUST - * accept it. ASCII only; no UTF-8 in local-part or domain. + * Enforces dot-atom local-part structure per §3.2.3 (`1*atext *("." 1*atext)`): + * a leading, trailing, or consecutive dot is rejected. This matches the actual + * obs-local-part ABNF (§4.4: `word *("." word)`, words non-empty) — obs-local-part + * never permitted empty words either. For the maximally permissive dot placement + * some legacy systems emit, use rfc2822() or `->withAllowObsLocalPart(true)`. + * ASCII only; no UTF-8 in local-part or domain. */ public static function rfc5322(): self { return new self( allowUtf8LocalPart: false, - allowObsLocalPart: true, + allowObsLocalPart: false, allowQuotedString: true, validateQuotedContent: false, rejectEmptyQuotedLocalPart: false, diff --git a/tests/ParseTest.php b/tests/ParseTest.php index 41a5882..1a09032 100644 --- a/tests/ParseTest.php +++ b/tests/ParseTest.php @@ -276,6 +276,69 @@ public function testDisplayNamePhraseValidationAllowsQuotedNames(): void $this->assertFalse($result->invalid); } + /** + * rfc5322() enforces dot-atom structure (§3.2.3) — leading, trailing, and + * consecutive dots in the local part are rejected, matching the actual + * obs-local-part ABNF (§4.4: `word *("." word)`, words non-empty). The + * maximally permissive dot placement lives in rfc2822()/withAllowObsLocalPart. + */ + public function testRfc5322RejectsEdgeAndConsecutiveDotsWithLenientEscapeHatch(): void + { + $tight = new Parse(null, ParseOptions::rfc5322()); + $lenient = new Parse(null, ParseOptions::rfc2822()); + $optIn = new Parse(null, ParseOptions::rfc5322()->withAllowObsLocalPart(true)); + + foreach (['.a@example.com', 'a.@example.com', 'a..b@example.com'] as $addr) { + $this->assertTrue($tight->parseSingle($addr)->invalid, "rfc5322 should reject {$addr}"); + $this->assertFalse($lenient->parseSingle($addr)->invalid, "rfc2822 should accept {$addr}"); + $this->assertFalse($optIn->parseSingle($addr)->invalid, "obs opt-in should accept {$addr}"); + } + // A well-formed dotted local part stays valid in the tight preset. + $this->assertFalse($tight->parseSingle('a.b.c@example.com')->invalid); + } + + /** + * `""@domain` is a syntactically-empty quoted local-part (RFC 5321 §4.1.2). + * rejectEmptyQuotedLocalPart controls whether it is accepted; the default + * (false) accepts it. Guards the state-machine fix that recognises an empty + * quote as quoted (quote_temp is empty, so content-emptiness can't signal it). + */ + public function testEmptyQuotedLocalPartAcceptanceIsConfigurable(): void + { + $accept = new Parse(null, ParseOptions::rfc5322()->withRejectEmptyQuotedLocalPart(false)); + $reject = new Parse(null, ParseOptions::rfc5322()->withRejectEmptyQuotedLocalPart(true)); + + $ok = $accept->parseSingle('""@example.com'); + $this->assertFalse($ok->invalid); + $this->assertSame('""@example.com', $ok->address); + + $bad = $reject->parseSingle('""@example.com'); + $this->assertTrue($bad->invalid); + $this->assertSame(\Email\ParseErrorCode::EmptyQuotedLocalPart, $bad->invalidReasonCode); + + // A display-name quote must not leak the quoted flag onto the real local-part. + $named = $accept->parseSingle('"John Doe" '); + $this->assertFalse($named->invalid); + $this->assertSame('j', $named->localPart); + } + + /** + * The RFC 5321 §4.5.3.1 octet limits can be turned off wholesale via + * enforceLengthLimits(false) — or raised via withLengthLimits() — for callers + * on systems that permit longer local parts than the 64-octet default. + */ + public function testLengthLimitsCanBeDisabled(): void + { + $long = str_repeat('a', 65).'@example.com'; + $this->assertSame( + \Email\ParseErrorCode::LocalPartTooLong, + (new Parse(null, ParseOptions::rfc5322()))->parseSingle($long)->invalidReasonCode, + ); + $this->assertFalse( + (new Parse(null, ParseOptions::rfc5322()->withEnforceLengthLimits(false)))->parseSingle($long)->invalid, + ); + } + public function testStrictIdnaAcceptsValidIdn(): void { // "bücher.de" is a well-formed IDNA label — valid under strict IDNA2008. @@ -848,7 +911,7 @@ public function testFactoryPresetsHaveExpectedRuleValues(): void ]], 'rfc5322' => [ParseOptions::rfc5322(), [ 'allowUtf8LocalPart' => false, - 'allowObsLocalPart' => true, + 'allowObsLocalPart' => false, 'allowQuotedString' => true, 'validateQuotedContent' => false, 'rejectEmptyQuotedLocalPart' => false, diff --git a/tests/testspec.yml b/tests/testspec.yml index 878ff14..e0d5452 100644 --- a/tests/testspec.yml +++ b/tests/testspec.yml @@ -4224,12 +4224,14 @@ # NORMAL Mode Tests (RFC 5322 + obsolete syntax) -# Test that obsolete syntax is accepted per RFC 5322 §4 +# Test that obsolete lenient dot placement is accepted by the relaxed (RFC 2822) +# preset. rfc5322() enforces dot-atom (no leading/trailing/consecutive dots) per +# the actual obs-local-part ABNF; use relaxed / withAllowObsLocalPart(true) for it. - emails: 'user..name@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: 'user..name@example.com' @@ -4250,7 +4252,7 @@ - emails: '.user@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: '.user@example.com' @@ -4271,7 +4273,7 @@ - emails: 'user.@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: 'user.@example.com' @@ -4292,7 +4294,7 @@ - emails: 'user...name@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: 'user...name@example.com' @@ -4381,7 +4383,7 @@ - emails: 'user....name@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: 'user....name@example.com' @@ -4402,7 +4404,7 @@ - emails: '..user@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: '..user@example.com' @@ -4423,7 +4425,7 @@ - emails: 'user..@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: 'user..@example.com' @@ -4444,7 +4446,7 @@ - emails: '.@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: '.@example.com' @@ -4528,7 +4530,7 @@ - emails: 'test.user..name@example.com' multiple: false - rfc_mode: normal + rfc_mode: relaxed allow_smtputf8: false result: address: 'test.user..name@example.com' From 4db8a9ee30fd95590c2e5656ee7c00e710bd175f Mon Sep 17 00:00:00 2001 From: Matthew J Mucklo Date: Tue, 21 Jul 2026 23:43:09 -0700 Subject: [PATCH 2/2] docs: track gold-standard RFC-conformance follow-ups in ROADMAP --- ROADMAP.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ROADMAP.md b/ROADMAP.md index 6df466e..4e9d8e2 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -86,6 +86,19 @@ Not tied to a specific release; picked up as time allows. - [~] Parse.php line coverage — now 87.98% (up from 86.69%). Overall project line coverage 91.15% (up from 89.61%). Remaining gaps are obscure error branches, the "shouldn't ever get here" default case, and code paths reachable only via internal state corruption. Target ≥95% aspirational. - [x] CI matrix: PHP 8.5 added as a required job; PHP 8.6 added as an allowed-to-fail experimental (nightly) job until its stable release (~Nov 2026). +**RFC conformance (gold-standard differential):** + +Differential testing against the `dominicsayers/isemail` reference corpus (164 cases) and a reference RFC validator surfaced a set of over-acceptance edge cases — inputs the parser currently treats as valid that the reference standard rejects. Clustered by root cause, in rough priority order: + +- [ ] **Comment (CFWS) parsing** — unclosed comments (`((comment)test@`, `test@iana.org(comment\`) and atext after a comment in the local part (`test(comment)test@`) are wrongly accepted. RFC 5322 §3.2.2: a comment must be balanced, and CFWS may not sit between atext runs of a dot-atom. +- [ ] **Quoted-string boundaries** — atext adjacent to a quoted string (`"test"test@`) and consecutive quoted strings (`"test""test"@`) are wrongly accepted. A quoted-string is a whole `word`; nothing may abut it without a separating dot. +- [ ] **Unclosed domain literal** — `test@[1.2.3.4` (missing `]`) is wrongly accepted. +- [ ] **CR/LF & folding-whitespace strictness** — trailing/embedded bare CR or LF and malformed CRLF folding (`test@iana.org\r`, `...\r\n\r\n`) are accepted. Partly intentional (the batch parser trims surrounding whitespace), so decide per-mode: strict presets should reject; the lenient/batch path may keep trimming. Document the chosen contract. +- [ ] **Control character in domain** — a C0 control in the domain (`test@\x07.org`) should be rejected. +- [ ] **Trailing domain dot** — `test@iana.org.` is accepted as the RFC 5321 §2.3.5 root-label dot; the reference corpus flags it. Likely keep (defensible), but confirm and document the divergence rather than leave it implicit. + +Approach: one PR per cluster, each adding the failing corpus cases as regression tests. The comparison harness is a local dev tool (not a CI gate) until the disagreements are triaged, since ~20 reference cases currently diverge. + **Static analysis:** - [x] PHPStan level 6 → 8 — tighter generics and inference; required four small nullable-return guards (`idn_to_ascii`, `mb_split`, `file_get_contents`) and one local docblock shape on `parseMultiple()`. - [x] Psalm alongside PHPStan — level 3 with baseline (66 entries, all false positives or duplicates of PHPStan findings). Found no genuinely new bugs vs PHPStan level 8; serves as a cross-check for future regressions. `composer psalm`.