Bug #73948 [Ana]: Preg_match_all should return NULLs on trailing optional capture groups.
| From: | cmb@php.net | Date: | Mon, 23 Jan 2017 18:31:20 +0000 |
| Subject: | Bug #73948 [Ana]: Preg_match_all should return NULLs on trailing optional capture groups. | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-206903@lists.php.net to get a copy of this message | ||
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:
> But why would you need an RFC for a bugfix?
An RFC might be over the top, but at least a PR to get some
attention appears to be appropriate.
Previous Comments:
------------------------------------------------------------------------
[2017-01-23 17:53:31] tomasyorke at hotmail dot com
If you wish to delay this change until 7.2, that sounds fine.
But why would you need an RFC for a bugfix?
------------------------------------------------------------------------
[2017-01-23 17:43:39] tomasyorke at hotmail dot com
I'm glad we didn't go the unset route. Having fixed sized arrays is useful for looping by
groups/matches independently of the Flag used.
You also can very easily know how many groups/matches the regex returned/contained.
By making the output array dynamicly sized. You only allow for an easy way to determine one of these
things. While making the other more complex.
------------------------------------------------------------------------
[2017-01-23 17:17:20] cmb@php.net
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.
------------------------------------------------------------------------
[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.
------------------------------------------------------------------------
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