Edit report at https://bugs.php.net/bug.php?id=77193&edit=1
ID: 77193
User updated by: mlocati at gmail dot com
Reported by: mlocati at gmail dot com
Summary: Infinite loop in preg_replace_callback
Status: Open
Type: Bug
Package: PCRE related
PHP Version: 7.3.0RC6
Block user comment: N
Private report: N
New Comment:
Just for the records, I'm compiling a 32-bit PHP under Windows with
configure --disable-all --with-all-shared --enable-cli --enable-phar --enable-json --enable-filter
--with-iconv --enable-pdo --with-mysqli --with-mysqlnd --with-pdo-mysql --with-openssl
--with-simplexml --with-libxml --with-gd --enable-mbstring --enable-tokenizer --with-dom --with-xml
--enable-xmlreader --enable-xmlwriter --enable-hash --enable-fileinfo --enable-session --enable-zip
nmake
Previous Comments:
------------------------------------------------------------------------
[2018-11-29 13:33:56] mlocati at gmail dot com
@cmb I just tried your patch instead of the @ab one, but with it we still have the infinite loop...
------------------------------------------------------------------------
[2018-11-29 11:34:17] cmb@php.net
I wonder whether we don't have to call pcre2_get_mark() before
calling pcre2_get_ovector_pointer(), i.e. partly reverting commit
b81d712.
I mean something like this instead the patch above:
ext/pcre/php_pcre.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c
index 5165209b85..4277ffb6aa 100644
--- a/ext/pcre/php_pcre.c
+++ b/ext/pcre/php_pcre.c
@@ -1792,6 +1792,7 @@ static zend_string *php_pcre_replace_func_impl(pcre_cache_entry *pce,
zend_strin
char *match, /* The current match */
*piece; /* The current piece of subject */
size_t result_len; /* Length of result */
+ PCRE2_SPTR mark = NULL; /* Target for MARK name */
zend_string *result; /* Result of replacement */
zend_string *eval_result; /* Result of custom function */
pcre2_match_data *match_data;
@@ -1852,6 +1853,8 @@ static zend_string *php_pcre_replace_func_impl(pcre_cache_entry *pce,
zend_strin
while (1) {
piece = subject + start_offset;
+ mark = pcre2_get_mark(match_data);
+
if (count >= 0 && limit) {
/* Check for too many substrings condition. */
if (UNEXPECTED(count == 0)) {
@@ -1881,8 +1884,7 @@ matched:
new_len = result_len + offsets[0] - start_offset; /* part before the match */
/* Use custom function to get replacement string and its length. */
- eval_result = preg_do_repl_func(fci, fcc, subject, offsets, subpat_names, count,
- pcre2_get_mark(match_data));
+ eval_result = preg_do_repl_func(fci, fcc, subject, offsets, subpat_names, count, mark);
ZEND_ASSERT(eval_result);
new_len = zend_safe_address_guarded(1, ZSTR_LEN(eval_result), new_len);
[1] <http://git.php.net/?p=php-src.git;a=commit;h=b81d712961aa3cbc64dc8bb521f2427cf443e550>
------------------------------------------------------------------------
[2018-11-29 09:04:48] mlocati at gmail dot com
@ab Yes! That patch fixed the issue:
C:\MyPath>composer test -- --filter=ContentPageTranslateTest::testFrom
> phpunit "--filter=ContentPageTranslateTest::testFrom"
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
Runtime: PHP 7.3.0-dev
Configuration: C:\MyPath\phpunit.xml
.
Time: 4.68 seconds, Memory: 30.00MB
OK (1 test, 1 assertion)
Great job, Anatol!
------------------------------------------------------------------------
[2018-11-28 20:32:20] ab@php.net
@mlocati thanks, with the docker way reproduced it as well.
I still can't repro it on a reduced snippet. I guess, the method call inside the callback
somehow resets the match context (so somewhere in the call trace is a PCRE API call), thus
preventing loop termination. Could you please check, whether this fixes it?
diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c
index 5165209b85..f8fc721d81 100644
--- a/ext/pcre/php_pcre.c
+++ b/ext/pcre/php_pcre.c
@@ -1880,6 +1880,9 @@ matched:
new_len = result_len + offsets[0] - start_offset; /* part before the match
*/
+ /* Advance to the next piece. */
+ start_offset = offsets[1];
+
/* Use custom function to get replacement string and its length. */
eval_result = preg_do_repl_func(fci, fcc, subject, offsets, subpat_names,
count,
pcre2_get_mark(match_data));
@@ -1908,9 +1911,6 @@ matched:
limit--;
- /* Advance to the next piece. */
- start_offset = offsets[1];
-
/* If we have matched an empty string, mimic what Perl's /g options
does.
This turns out to be rather cunning. First we set PCRE2_NOTEMPTY_ATSTART
and try
the match again at the same point. If this fails (picked up above) we
------------------------------------------------------------------------
[2018-11-27 15:56:27] mlocati at gmail dot com
> This doesn't look like a PCRE issue. URL::to() seems to block the callback,
> so then it never returns. The actual issue should be somewhere later in
> the call stack starting from URL::to(), something like an endless loop or
> unterminated recursive call.
I modified the code a bit to show what's happening.
With this code:
ob_end_clean(); // Required because PHPUnit enables output buffering
echo 'Method: ', __METHOD__, "\n";
echo 'Text: ', $text, "\n";
$text = preg_replace_callback(
'/{CCM:CID_([0-9]+)}/i',
function ($matches) {
print_r($matches);
$cID = $matches[1];
if ($cID > 0) {
$c = Page::getByID($cID, 'ACTIVE');
if ($c->isActive()) {
return (string) \URL::to($c);
}
}
},
$text
);
The output is the following:
Method: Concrete\Core\Editor\LinkAbstractor::translateFrom
Text: <a href="{CCM:CID_3}">Super Cool!</a>
Array
(
[0] => {CCM:CID_3}
[1] => 3
)
Array
(
[0] => {CCM:CID_3}
[1] => 3
)
Array
(
[0] => {CCM:CID_3}
[1] => 3
)
[...endless list of Array()...]
As you can see, the \URL::to() call is not blocking the execution, but the callback is called
forever even if we have just one match.
------------------------------------------------------------------------
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=77193
--
Edit this bug report at https://bugs.php.net/bug.php?id=77193&edit=1