Edit report at https://bugs.php.net/bug.php?id=73948&edit=1
ID: 73948
Updated by: cmb@php.net
Reported by: tomasyorke at hotmail dot com
Summary: Preg_match_all should return NULLs on trailing
optional capture groups.
Status: Analyzed
Type: Bug
Package: PCRE related
Operating System: Windows 7
PHP Version: 7.0.14
Block user comment: N
Private report: N
New Comment:
Regarding the BC break: when fixing <https://bugs.php.net/61780> I
would have preferred to unset unmatched captures, but that
appeared too much of a BC break (consider somebody is count()ing
the $matches). Therefore I changed the empty strings to NULL.
Adding unmatched trailing captures as NULL would still have the
same BC concerns â I don't think we can do that before PHP 7.2,
and even that might require the RFC process.
Previous Comments:
------------------------------------------------------------------------
[2017-01-23 15:59:31] tomasyorke at hotmail dot com
Regarding BC
The proposed change would be to change the representation of missed capture groups from 2
representations to 1 representation.
The only code that would break is code that supported only the more obscure representation that only
appears with trailing groups. And if that code exists, then it will still break on non trailing
missing groups even without the change.
------------------------------------------------------------------------
[2017-01-23 15:48:19] tomasyorke at hotmail dot com
As noted by cmb, this bug does not appear with the PREG_PATTERN_ORDER flag.
Thus I switched the flag and my code to use PREG_PATTERN_ORDER and implemented my own
preg_set_order() function.
But I found a nearly identical bug with the PREG_OFFSET_CAPTURE flag.
Consider the code
preg_match_all("#(a)?(b)(c)?#","b",$matches,PREG_OFFSET_CAPTURE);
var_dump($matches);
Returns
array(4) {
[0] => array(1) {
[0] => array(2) {
[0] => string(1) "b" [1] => int(0)
}
} [1] => array(1) {
[0] => array(2) {
[0] => string(0) "" [1] => int(-1)
}
} [2] => array(1) {
[0] => array(2) {
[0] => string(1) "b" [1] => int(0)
}
} [3] => array(1) {
[0] => string(0) ""
}
}
As you can see, the third optional capture group returns something different than the first optional
capture group.
This is too a bug. Again, I don't care what representation is chosen. But it must be the same
for any optional capture no matter if trailing or not.
------------------------------------------------------------------------
[2017-01-17 13:28:22] cmb@php.net
For PREG_PATTERN_ORDER unmatched subpatterns are already added[1];
something like that would also have to be done for PREG_SET_ORDER.
However, that would cause BC issues, and adding another flag isn't
appealing to me. I also think that NULL and unset are close enough
to stick with the current behavior for now; checking isset()
would suffice.
[1] <https://github.com/php/php-src/blob/PHP-7.1.0/ext/pcre/php_pcre.c#L862-L871>
------------------------------------------------------------------------
[2017-01-16 15:50:23] requinix@php.net
Actually I can. Yay for Linux on Windows!
~/php/master/bin# ./php
<?php var_dump(preg_match('/(a)?(b)?(c)?/', 'b', $matches), $matches);
int(1)
array(3) {
[0]=>
string(1) "b"
[1]=>
NULL
[2]=>
string(1) "b"
}
[1] is clearly NULL but there is no [3] for the unmatched (c)? group.
------------------------------------------------------------------------
[2017-01-16 14:57:13] requinix@php.net
Hmm, yes, I misunderstood this report: 61780 is about turning empty strings from unmatched
subpatterns into NULLs while this is about *adding* trailing unmatched subpatterns.
I don't see anything in the 61780 changes that clearly indicate this is also fixed, but some
parts look like they might do it accidentally. Can anyone check? (It'd be nice if 3v4l could do
master too...)
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=73948
--
Edit this bug report at https://bugs.php.net/bug.php?id=73948&edit=1