From ca0a0f860e77dba72006b5571985e931edaf6d9c Mon Sep 17 00:00:00 2001 From: Matthew J Mucklo Date: Sun, 12 Jul 2026 17:32:17 -0700 Subject: [PATCH] Fix PHP 8.6 mb_regex_encoding deprecation in domain validation (#57) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateDomainName() called mb_regex_encoding()/mb_split(), both of which emit E_DEPRECATED under PHP 8.6 (the underlying oniguruma library is unmaintained). The domain is already ASCII at that point (post-punycode, via normalizeDomainAscii()), so label splitting now uses explode('.') and the hyphen-boundary checks use substr() instead of mb_substr(). The unused $encoding parameter is dropped — the sole caller never passed it. Behavior is unchanged. Two now-stale psalm-baseline entries are removed. CI: extend the test matrix to PHP 8.5 (required) and 8.6 (experimental, continue-on-error against nightly with --ignore-platform-req=php+), so removed/deprecated functions surface early without nightly breakage blocking PRs. Composer cache is now keyed per PHP version. Test coverage for the touched code and the MB/encoding paths around it: - testspec: trailing-hyphen label, invalid-char label, over-255 domain - non-UTF-8 tokenizer (ISO-8859-1) and variable-width (Shift-JIS) parsing - RFC 5321 length boundaries: accept-at-max, reject-one-over - PunycodeConversionFailed and LocalPartCannotBeNormalized (previously never triggered by any functional test) - mismatched caller-encoding domain fails gracefully - malformed-UTF-8 handling gated on a runtime mbstring probe: 8.1/8.2 preserve invalid bytes (rejected), 8.3+ substitute them during tokenization before the guard runs Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 21 ++++-- CHANGELOG.md | 3 + ROADMAP.md | 2 +- psalm-baseline.xml | 7 -- src/Parse.php | 27 +++----- tests/ParseTest.php | 134 +++++++++++++++++++++++++++++++++++++++ tests/testspec.yml | 58 +++++++++++++++++ 7 files changed, 223 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7fa29e1..f2cccd3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,7 @@ jobs: tests: name: Tests (PHP ${{ matrix.php-version }}) runs-on: ubuntu-latest + continue-on-error: ${{ matrix.experimental }} strategy: fail-fast: false matrix: @@ -18,7 +19,17 @@ jobs: - '8.2' - '8.3' - '8.4' - + - '8.5' + experimental: + - false + # PHP 8.6 is not yet released (stable ~Nov 2026); test against nightly + # builds so removed/deprecated functions surface early, but allow it to + # fail — nightly breakage and PHPUnit 9.6's own deprecations must not + # block PRs. + include: + - php-version: '8.6' + experimental: true + steps: - name: Checkout code uses: actions/checkout@v4 @@ -41,11 +52,13 @@ jobs: uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} - restore-keys: ${{ runner.os }}-composer- + key: ${{ runner.os }}-composer-${{ matrix.php-version }}-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-composer-${{ matrix.php-version }}- + # Dev dependencies may not yet declare support for the unreleased PHP 8.6; + # ignore only the php upper-bound so the suite can still run there. - name: Install dependencies - run: composer install --prefer-dist --no-progress --no-suggest + run: composer install --prefer-dist --no-progress ${{ matrix.experimental && '--ignore-platform-req=php+' || '' }} - name: Run test suite run: bin/phpunit diff --git a/CHANGELOG.md b/CHANGELOG.md index 063f9ba..14844d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Fixed +- **PHP 8.6 deprecation** ([#57](https://github.com/mmucklo/email-parse/issues/57)): `validateDomainName()` no longer calls `mb_regex_encoding()`/`mb_split()`, both of which emit `E_DEPRECATED` under PHP 8.6 (the underlying oniguruma library is unmaintained). The domain is already ASCII at that point (post-punycode, via `normalizeDomainAscii()`), so label splitting now uses a plain `explode('.', …)` — behavior is unchanged. + ## [3.3.1] ### Fixed diff --git a/ROADMAP.md b/ROADMAP.md index 9f71d56..9afadc3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -84,7 +84,7 @@ Not tied to a specific release; picked up as time allows. - [~] Mutation testing with Infection — wired in via `composer infect` with thresholds `minMsi=80`, `minCoveredMsi=85` (current baseline, up from 74/79). Target remains ≥85% overall MSI; raise threshold as more error-path tests land. - [x] Property-based testing — `tests/PropertyTest.php` with 10 invariants across 200 random iterations each: no-crash on arbitrary bytes, determinism, reason+code consistency, severity classification, Stringable contract, toArray ↔ parse() round-trip, valid-address round-trip, and all-presets-never-crash. No extra dependency (native PHPUnit + `mt_rand`; deterministic via `SEED` envvar). - [~] 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. -- [ ] CI matrix: add PHP 8.5 once released. +- [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). **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()`. diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 7c3ea55..9451d0d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6,9 +6,6 @@ - - - @@ -42,10 +39,6 @@ - - - - diff --git a/src/Parse.php b/src/Parse.php index 0946a2f..5be9a0f 100644 --- a/src/Parse.php +++ b/src/Parse.php @@ -1374,30 +1374,23 @@ protected function normalizeDomainAscii(string $domain): ?string * - RFC 1123 §2.1 relaxed the original restriction that allowed labels starting * with a letter only, permitting labels that start with a digit. * - * @param string $domain The ASCII domain name to validate (after punycode conversion) - * @param string $encoding The encoding of the string (if not UTF-8) + * @param string $domain The ASCII domain name to validate (after punycode conversion) * * @return array{valid: bool, reason?: string, code?: ParseErrorCode} */ - protected function validateDomainName(string $domain, string $encoding = 'UTF-8'): array + protected function validateDomainName(string $domain): array { // RFC 5321 §4.5.3.1.2: total domain length limit is in octets if (strlen($domain) > 255) { return ['valid' => false, 'reason' => 'Domain name too long', 'code' => Err::DomainTooLong]; } else { - // mb_regex_encoding() can return false on failure; only restore when - // we got back a real encoding name. - $origEncoding = mb_regex_encoding(); - mb_regex_encoding($encoding); - $parts = mb_split('\\.', $domain); - if ($origEncoding) { - mb_regex_encoding($origEncoding); - } - // mb_split() can return false on failure; treat that as a validation - // failure rather than iterating over a bogus value. - if ($parts === false) { - return ['valid' => false, 'reason' => 'Domain name could not be tokenized', 'code' => Err::DomainInvalid]; - } + // $domain is always ASCII here (post-punycode, via normalizeDomainAscii), + // so a plain explode on the label separator is sufficient. This avoids + // mb_regex_encoding(), deprecated since PHP 8.6 (the underlying oniguruma + // library is no longer maintained). See GitHub issue #57. + // Labels are guaranteed non-empty: the state machine rejects consecutive + // and edge dots (ConsecutiveDots) before the domain validator runs. + $parts = explode('.', $domain); $maxLabelLen = $this->options->getLengthLimits()->maxDomainLabelLength; foreach ($parts as $part) { if (strlen($part) > $maxLabelLen) { @@ -1406,7 +1399,7 @@ protected function validateDomainName(string $domain, string $encoding = 'UTF-8' if (!preg_match('/^[a-zA-Z0-9\-]+$/', $part)) { return ['valid' => false, 'reason' => "Domain name '{$domain}' can only contain letters a through z, numbers 0 through 9 and hyphen. The part '{$part}' contains characters outside of that range.", 'code' => Err::DomainContainsInvalidChars]; } - if ('-' == mb_substr($part, 0, 1, $encoding) || '-' == mb_substr($part, mb_strlen($part) - 1, 1, $encoding)) { + if ('-' == substr($part, 0, 1) || '-' == substr($part, -1)) { return ['valid' => false, 'reason' => "Parts of the domain name '{$domain}' can not start or end with '-'. This part does: {$part}", 'code' => Err::DomainLabelStartsOrEndsWithHyphen]; } } diff --git a/tests/ParseTest.php b/tests/ParseTest.php index cd5904a..41a5882 100644 --- a/tests/ParseTest.php +++ b/tests/ParseTest.php @@ -294,6 +294,140 @@ public function testStrictIdnaRejectsBareLeadingHyphenLabel(): void $this->assertTrue($result->invalid); } + /** + * The main-loop tokenizer walks the input with mb_substr($emails, $i, 1, $encoding), + * so a non-UTF-8 caller encoding must be honored: the ISO-8859-1 byte 0xF6 ("ö") is + * one character, not an invalid UTF-8 lead byte. Guards the encoding-threading path, + * which the YAML spec harness (always UTF-8) never exercises. + */ + public function testNonUtf8EncodingIsHonoredByTokenizer(): void + { + // "Jörg " with ö as the single ISO-8859-1 byte 0xF6. + $input = "J\xF6rg "; + $result = Parse::getInstance()->parseSingle($input, 'ISO-8859-1'); + + $this->assertFalse($result->invalid); + $this->assertSame('j', $result->localPart); + $this->assertSame('example.com', $result->domain); + // The 0xF6 byte is preserved verbatim as one character in the display name. + $this->assertSame("J\xF6rg", $result->name); + } + + /** + * Latin-1 is single-byte, so it does not exercise mb_substr's variable-width + * character indexing. Shift-JIS does: the kanji 日 is two bytes (0x93 0xFA) but + * one character, and the tokenizer must advance by character, not by byte. + */ + public function testVariableWidthEncodingIsHonoredByTokenizer(): void + { + $input = mb_convert_encoding('日本 ', 'SJIS', 'UTF-8'); + $result = Parse::getInstance()->parseSingle($input, 'SJIS'); + + $this->assertFalse($result->invalid); + $this->assertSame('j', $result->localPart); + $this->assertSame('example.com', $result->domain); + // The two 2-byte kanji round-trip intact — proof the byte offsets never split a char. + $this->assertSame('日本', mb_convert_encoding($result->name, 'UTF-8', 'SJIS')); + } + + /** + * Malformed UTF-8 in the local part (a lone 0x80 continuation byte). Whether the + * mb_check_encoding guard ever sees the bad byte depends on mbstring: PHP 8.1/8.2 + * preserve it through mb_substr (so it is rejected as InvalidUtf8Encoding), while + * 8.3+ substitute it during tokenization (so it never reaches the guard). Probe the + * runtime behavior rather than the version, and require rejection only where the + * tokenizer preserves invalid bytes; everywhere else the parser must still return a + * deterministic, crash-free result. + */ + public function testMalformedUtf8LocalPartHandling(): void + { + $loneByte = pack('C', 0x80); // a lone UTF-8 continuation byte + $input = 'us'.$loneByte.'er@example.com'; + $result = Parse::getInstance()->parseSingle($input); + + if (mb_substr($loneByte, 0, 1, 'UTF-8') === $loneByte) { + $this->assertTrue($result->invalid); + $this->assertSame(\Email\ParseErrorCode::InvalidUtf8Encoding, $result->invalidReasonCode); + } else { + // Byte sanitized before validation; result must be deterministic. + $this->assertSame($result->invalid, Parse::getInstance()->parseSingle($input)->invalid); + } + } + + /** + * RFC 5321 §4.5.3.1 octet limits are exclusive upper bounds; verify each comparison + * accepts the exact maximum and rejects one octet past it (guards `>` vs `>=`): + * - local part 64 (§4.5.3.1.1), domain label 63 (§4.5.3.1.2), whole address 254. + */ + public function testLengthBoundariesAcceptMaxAndRejectOneOver(): void + { + $p = Parse::getInstance(); + $Err = \Email\ParseErrorCode::class; + + // Local part: 64 octets is the maximum; 65 is over. + $this->assertFalse($p->parseSingle(str_repeat('a', 64).'@example.com')->invalid); + $this->assertSame($Err::LocalPartTooLong, $p->parseSingle(str_repeat('a', 65).'@example.com')->invalidReasonCode); + + // Domain label: 63 octets is the maximum; 64 is over. + $this->assertFalse($p->parseSingle('u@'.str_repeat('a', 63).'.com')->invalid); + $this->assertSame($Err::DomainLabelTooLong, $p->parseSingle('u@'.str_repeat('a', 64).'.com')->invalidReasonCode); + + // Whole address: 254 octets is the maximum; 255 is over. Labels kept <= 63. + $at254 = str_repeat('a', 64).'@'.str_repeat('b', 63).'.'.str_repeat('c', 63).'.'.str_repeat('d', 61); + $this->assertSame(254, strlen($at254)); + $this->assertFalse($p->parseSingle($at254)->invalid); + $at255 = str_repeat('a', 64).'@'.str_repeat('b', 63).'.'.str_repeat('c', 63).'.'.str_repeat('d', 62); + $this->assertSame($Err::TotalLengthExceeded, $p->parseSingle($at255)->invalidReasonCode); + } + + /** + * When idn_to_ascii() cannot produce a valid A-label (here a U-label whose punycode + * expansion overflows the 63-octet limit), normalizeDomainAscii() returns null and + * the address is rejected with PunycodeConversionFailed rather than crashing. + */ + public function testPunycodeConversionFailureIsReported(): void + { + $result = Parse::getInstance()->parseSingle('user@'.str_repeat('ä', 70).'.de'); + $this->assertTrue($result->invalid); + $this->assertSame(\Email\ParseErrorCode::PunycodeConversionFailed, $result->invalidReasonCode); + } + + /** + * IDN conversion assumes UTF-8. A non-ASCII domain supplied under a mismatched caller + * encoding (Shift-JIS bytes reinterpreted by idn_to_ascii) must fail gracefully — a + * clean invalid result, never an exception or warning. + */ + public function testMismatchedEncodingDomainFailsGracefully(): void + { + $input = mb_convert_encoding('user@日本.com', 'SJIS', 'UTF-8'); + $result = Parse::getInstance()->parseSingle($input, 'SJIS'); + $this->assertTrue($result->invalid); + $this->assertNotNull($result->invalidReasonCode); + } + + /** + * With NFC normalization enabled (rfc6531), Normalizer::normalize() runs before the + * mb_check_encoding guard and returns false on malformed UTF-8, so a preserved bad + * byte surfaces as LocalPartCannotBeNormalized (RFC 6532 §3.1) rather than + * InvalidUtf8Encoding. Gated on the same mbstring tokenizer behavior as + * testMalformedUtf8LocalPartHandling: 8.3+ substitutes the byte before it is reached. + */ + public function testLocalPartNormalizationFailureIsReported(): void + { + $opts = ParseOptions::rfc6531()->withRequireFqdn(false); + $parser = new Parse(null, $opts); + $loneByte = pack('C', 0x80); + $input = 'us'.$loneByte.'er@example.com'; + $result = $parser->parseSingle($input); + + if (mb_substr($loneByte, 0, 1, 'UTF-8') === $loneByte) { + $this->assertTrue($result->invalid); + $this->assertSame(\Email\ParseErrorCode::LocalPartCannotBeNormalized, $result->invalidReasonCode); + } else { + $this->assertSame($result->invalid, $parser->parseSingle($input)->invalid); + } + } + /** * Exercises every `withX()` fluent builder. Each call must return a new * instance with the targeted field flipped and every other field preserved. diff --git a/tests/testspec.yml b/tests/testspec.yml index 02c6cd1..878ff14 100644 --- a/tests/testspec.yml +++ b/tests/testspec.yml @@ -714,6 +714,64 @@ invalid: true invalid_reason: "Domain invalid: Parts of the domain name '-bad-domain.com' can not start or end with '-'. This part does: -bad-domain" comments: [] +- + emails: testing@baddomain-.com + multiple: false + result: + address: '' + simple_address: '' + original_address: testing@baddomain-.com + name: '' + name_parsed: '' + local_part: testing + local_part_parsed: testing + domain_part: 'baddomain-.com' + domain: 'baddomain-.com' + domain_ascii: null + ip: '' + invalid: true + invalid_reason: "Domain invalid: Parts of the domain name 'baddomain-.com' can not start or end with '-'. This part does: baddomain-" + comments: [] +- + # Domain label containing a char outside [A-Za-z0-9-] reaches validateDomainName's + # per-label preg_match check (underscore survives the state machine but fails here). + emails: user@bad_domain.com + multiple: false + result: + address: '' + simple_address: '' + original_address: user@bad_domain.com + name: '' + name_parsed: '' + local_part: user + local_part_parsed: user + domain_part: 'bad_domain.com' + domain: 'bad_domain.com' + domain_ascii: null + ip: '' + invalid: true + invalid_reason: "Domain invalid: Domain name 'bad_domain.com' can only contain letters a through z, numbers 0 through 9 and hyphen. The part 'bad_domain' contains characters outside of that range." + comments: [] +- + # Domain exceeding RFC 5321 §4.5.3.1.2 total octet limit (256 > 255) trips the + # length guard before label splitting. + emails: user@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com + multiple: false + result: + address: '' + simple_address: '' + original_address: user@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com + name: '' + name_parsed: '' + local_part: user + local_part_parsed: user + domain_part: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com' + domain: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com' + domain_ascii: null + ip: '' + invalid: true + invalid_reason: "Domain invalid: Domain name too long" + comments: [] - emails: testing@192.168.0.1 multiple: false