search

beberlei / assert

Thin assertion library for use in libraries and business-model
Other
2.41k stars 188 forks source link

2.9.6: Assertion::url('https://www.domain.ext') throws exception on PHP 7.3.2 #275

Closed holtkamp closed 3 years ago

holtkamp commented 5 years ago

For version beberlei/assert 2.9.6, Assertion::url('http://www.domain.ext'); fails on PHP 7.3.2, while it worked on 7.1.

Any idea what the cause is of this? Does PHP 7.3.2 interpret the regular expression differently?

Is that the reason why the regular expression was changed as mentioned here: https://github.com/beberlei/assert/issues/263 and documented here: https://github.com/beberlei/assert/blob/master/CHANGELOG.md#301---2018-07-04

I suppose this might be due to the upgrade to from PCRE to PCRE2 in PHP 7.3...

An upgrade to beberlei/assert 3.2.0 resolved this issue.

Maybe the 2.x versions should require PHP < 7.3. Not sure whether changing that "backwards" is a back practice though 😄

rquadling commented 5 years ago

Thank you for raising this. I'll investigate this today.

rquadling commented 5 years ago

I've introduced a specific test for the Assertion::url('http://www.domain.ext') against PHP 7.3.0, 7.3.1, and 7.3.2.

Tests seem to pass (once I limited the PHPUnit to < V8).

rquadling commented 5 years ago

I've no real problem limiting a pre-existing version if it is not forward compatible.

rquadling commented 5 years ago

But I think I'll probably leave it as it is as I don't want to maintain multiple versions simultaneously.

holtkamp commented 5 years ago

Yeah, that would require a branch right?

https://github.com/beberlei/assert/issues/275#issuecomment-465084916

Did you manage to reproduce the problem of beberlei/assert 2.x @ php 7.3? In that case, maybe a warning in the documentation / changelog suffices...

rquadling commented 5 years ago

Testing https://travis-ci.org/beberlei/assert/builds/495412959

rquadling commented 5 years ago

Replicated : https://travis-ci.org/beberlei/assert/jobs/495412967#L902 preg_match(): Compilation failed: invalid range in character class at offset 187

https://github.com/symfony/validator/commit/ea91e49c40a943fd44bf515397dd4d88e0fd1374 is the fix.

I'll branch V2 and do a new release.

holtkamp commented 5 years ago

Ok, good to hear it can be replicated. PCRE2 is a bit stricter when dealing with the regular expressions:

https://wiki.php.net/rfc/pcre2-migration https://ayesh.me/Upgrade-PHP-7.3#pcre2

kudos for the swift response!

rquadling commented 5 years ago

It was a bug in the original Symfony regex that I copied and updated later in V3.

adri commented 5 years ago

@rquadling thanks for the fix! Do you know which branch/release to use?

adri commented 5 years ago

@rquadling Ah I see, it should have been 2.9.7 but that's not released. The builds failed here https://github.com/beberlei/assert/commits/v2. Looks like phpstan failed, but I don't see it related to the code changes.

rquadling commented 5 years ago

@adri, can you upgrade to V3?