Make Assert::regex() report PCRE failures instead of a bogus mismatch

[dualfroz]

Problem

Fixes #119 (one instance of it -- see "Scope" below).

Problem

Assert::regex() treats the return value of preg_match() as a boolean. preg_match()
has three outcomes, not two: 1 (match), 0 (no match) and false (the engine could not
run the match at all). Folding false into the 0 branch produces two bad behaviours.

1. A raw PHP warning escapes from inside the library, followed by a misleading exception.

Assert::regex('abc', '/(/');

on PHP 8.3.6 before this change:

PHP Warning:  preg_match(): Compilation failed: missing closing parenthesis at offset 1 in .../src/Assert.php on line 1510
Webmozart\Assert\InvalidArgumentException: The value "abc" does not match the expected pattern.

The warning is emitted from library code the caller does not control, and the exception
then blames the value for what is actually a broken pattern.

2. A PCRE engine failure is silently reclassified as a failed assertion.

With pcre.backtrack_limit=100:

Assert::regex(str_repeat('a', 30).'c', '/^(a+)+$/');
// Webmozart\Assert\InvalidArgumentException: The value "aaaa...c" does not match the expected pattern.
// preg_last_error_msg() === 'Backtrack limit exhausted'

Here preg_match() returned false because it hit the backtrack limit -- it never
determined whether the value matches. The caller is told the value failed validation.
That is the more dangerous of the two: on a large input, a resource limit becomes an
indistinguishable "invalid input" error, and the real cause is invisible.

The same masking happens for PREG_BAD_UTF8_ERROR, PREG_JIT_STACKLIMIT_ERROR, and so on.

Root cause

src/Assert.php:1510

if (!\preg_match($pattern, $value)) {

!false and !0 are both true, so the "engine failed" and "value did not match" cases
are indistinguishable, and nothing suppresses or reports the warning preg_match() raises
on its way out.

Solution

Separate the three outcomes, mirroring how this repository already handles a native
function that can return false (Assert::strlen(), src/Assert.php:2590:
if (false === $encoding = \mb_detect_encoding($value)) {).

if (false === $result = @\preg_match($pattern, $value)) {
    static::reportInvalidArgument(\sprintf(
        'The pattern %s could not be evaluated: %s.',
        static::valueToString($pattern),
        \preg_last_error_msg()
    ));
}

if (0 === $result) {
    // unchanged: the existing mismatch message, including the caller's custom message
}

Now:

Assert::regex('abc', '/(/');
  -> InvalidArgumentException: The pattern "/(/" could not be evaluated: Internal error.
     (no PHP warning)

Assert::regex(str_repeat('a', 30).'c', '/^(a+)+$/');   // pcre.backtrack_limit=100
  -> InvalidArgumentException: The pattern "/^(a+)+$/" could not be evaluated: Backtrack limit exhausted.

Notes on the two deliberate choices here:

• The caller's custom $message is not applied to the engine-failure branch. A custom
  message such as 'The value %s is not a valid slug.' describes a mismatch. Reusing it for
  a pattern that never compiled would preserve exactly the confusion this change removes.
• The diagnostic comes from preg_last_error_msg() rather than from the raised warning
  text. Capturing the warning text ("Compilation failed: missing closing parenthesis at
  offset 1") would require set_error_handler()/restore_error_handler(), and both are
  listed in Psalm's dictionaries/ImpureFunctionsList.php, so calling them inside a method
  annotated @psalm-pure is the wrong trade. preg_match() and preg_last_error_msg() are
  not in that list.

Behaviour on the normal paths is unchanged: a matching value still returns the value, and a
genuine mismatch still throws with the existing default or custom message. No public
signature changes, so no BC break.

How this was tested

Environment: PHP 8.3.6, Composer 2.10.2, on commit 2ccb7c2 ("Prepare for 2.4.1 release").

Four tests were added to tests/AssertTest.php, following the existing standalone-test
convention (testResourceOfTypeCustomMessage, testEnumAssertionErrorMessage):

• testRegexRejectsAnUncompilablePattern
• testRegexSuppressesTheWarningForAnUncompilablePattern
• testRegexDoesNotUseTheCustomMessageForAnUncompilablePattern
• testRegexRejectsAPatternExceedingTheBacktrackLimit

Each new test fails without the source change and passes with it.

With src/Assert.php reverted to its original state and only the new tests applied:

With the fix applied:

Full suite before the change:

Full suite after the change:

Same 2 notices and 56 skips in both runs -- they are pre-existing on a clean checkout
(the notices originate from tests/AssertTest.php:618) and are unrelated to this change.
The 4 added tests account for the entire delta.

One caveat on local verification: composer run cs-check and composer run static-analysis
could not be run on this machine. friendsofphp/php-cs-fixer v3.92.3 requires PHP >= 8.4
via symfony/console v8.0.3, and vimeo/psalm 6.14.3 refuses to start with
Psalm requires a PHP version ">= 8.3.16". You are running 8.3.6. The added code was instead
matched by hand to the conventions already in the tree: Yoda comparisons (false === $x, as
in src/Assert.php:829/897/2590), global classes imported rather than written with a leading
backslash (as with LogicException, Error, Exception in tests/AssertTest.php), and
concatenation without surrounding spaces. CI will be the real check on both of those gates.

[shadowhand]

Honestly, this seems like the sort of thing that should be accounted for by tests in downstream usage. There are many places where this package assumes that the written (not user input) values are expected to be verified in downstream use.