Skip to content

Commit 73fb4e0

Browse files
jturbidejturbide
committed
Fix debug backtrace frame normalization
1 parent 7ab8c54 commit 73fb4e0

5 files changed

Lines changed: 209 additions & 71 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ history, the old changelog, and committed file changes. Older Zemit-era entries
1515
are summarized where the commit history is too granular to be useful as
1616
release notes.
1717

18+
## 3.7.2 - Unreleased
19+
20+
### Fixed
21+
22+
- Kept PhalconKit debug backtrace frame normalization from exhausting PCRE
23+
backtracking limits on large runner-generated exception pages.
24+
- Added a local-only test fixture for visually inspecting rendered debug error
25+
pages without packaging it as a public asset.
26+
1827
## 3.7.1 - 2026-06-23
1928

2029
### Fixed

‎phpunit.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
<directory suffix=".php">tests</directory>
1717
<exclude>tests/Unit/AbstractUnit.php</exclude>
1818
<exclude>tests/Unit/Provider/ClamavTest.php</exclude>
19+
<exclude>tests/Fixtures</exclude>
1920
<exclude>tests/Unit/Bootstrap/Fixtures</exclude>
2021
<exclude>tests/Unit/Db/Fixtures</exclude>
2122
<exclude>tests/Unit/Events/Fixtures</exclude>

‎src/Support/Debug.php‎

Lines changed: 112 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -222,92 +222,133 @@ static function (array $m): string {
222222
*/
223223
private static function normalizeBacktraceFrames(string $html): string
224224
{
225-
$html = preg_replace_callback(
226-
"#<details class='frame(?P<class>[^']*)'(?P<open>\\s+open)?>(?P<body>.*?)</details>#s",
227-
static function (array $match): string {
228-
$body = $match['body'];
229-
230-
if (!preg_match(
231-
"#\\s*<summary>\\s*(<div class='frame-head'>.*?</div>)\\s*</summary>#s",
232-
$body,
233-
$summary
234-
)) {
235-
return $match[0];
236-
}
225+
$offset = 0;
226+
$normalized = '';
227+
$openNeedle = "<details class='frame";
228+
$closeNeedle = '</details>';
237229

238-
$head = preg_replace("#\\s*<span class='chev'>.*?</span>#s", '', $summary[1]);
239-
$head = self::requireRenderedHtml($head, 'removing the frame-level backtrace chevron');
240-
$body = str_replace($summary[0], '', $body);
230+
while (($start = strpos($html, $openNeedle, $offset)) !== false) {
231+
$openEnd = strpos($html, '>', $start);
241232

242-
$fileBlock = '';
243-
$file = null;
244-
$line = null;
233+
if ($openEnd === false) {
234+
break;
235+
}
245236

246-
if (preg_match("#\\s*(<div class='frame-file'[^>]*>.*?</div>)#s", $body, $fileMatch)) {
247-
$fileBlock = $fileMatch[1];
248-
$body = str_replace($fileMatch[0], '', $body);
249-
$file = self::readHtmlAttribute($fileBlock, 'data-file');
250-
$lineValue = self::readHtmlAttribute($fileBlock, 'data-line');
251-
$line = is_numeric($lineValue) ? (int) $lineValue : null;
252-
}
237+
$closeStart = strpos($html, $closeNeedle, $openEnd + 1);
253238

254-
$codeBlock = '';
239+
if ($closeStart === false) {
240+
break;
241+
}
255242

256-
if (preg_match("#\\s*(<div class='code'>.*?</div>)#s", $body, $codeMatch)) {
257-
$codeBlock = $codeMatch[1];
258-
$body = str_replace($codeMatch[0], '', $body);
259-
}
243+
$closeEnd = $closeStart + strlen($closeNeedle);
244+
$openingTag = substr($html, $start, $openEnd - $start + 1);
245+
$body = substr($html, $openEnd + 1, $closeStart - $openEnd - 1);
246+
$originalFrame = substr($html, $start, $closeEnd - $start);
260247

261-
$isCodeOpen = ($match['open'] ?? '') !== '';
262-
$classes = trim(
263-
'frame'
264-
. $match['class']
265-
. ($codeBlock !== '' ? ' has-source' : ' no-source')
266-
. ($codeBlock !== '' && $isCodeOpen ? ' is-code-open' : '')
267-
);
248+
$normalized .= substr($html, $offset, $start - $offset);
249+
$normalized .= self::normalizeBacktraceFrame($openingTag, $body, $originalFrame);
250+
$offset = $closeEnd;
251+
}
268252

269-
if ($codeBlock !== '') {
270-
$head = self::makeBacktraceFrameHeadToggle($head, $isCodeOpen);
271-
}
253+
if ($offset === 0) {
254+
return $html;
255+
}
272256

273-
$frame = "\n <article class='" . htmlspecialchars($classes, ENT_QUOTES) . "'>\n {$head}";
257+
return $normalized . substr($html, $offset);
258+
}
274259

275-
if ($fileBlock !== '') {
276-
$frame .= "\n {$fileBlock}";
277-
}
260+
/**
261+
* Normalize one native debug frame after the frame boundary is known.
262+
*
263+
* Frame extraction intentionally avoids a document-wide regex so large
264+
* backtraces cannot exhaust PCRE backtracking before the per-frame rewrite
265+
* runs. If Phalcon changes the opening tag shape, the original frame is
266+
* preserved instead of dropping debug output.
267+
*
268+
* @throws RuntimeException When an internal frame rewrite fails.
269+
*/
270+
private static function normalizeBacktraceFrame(string $openingTag, string $body, string $originalFrame): string
271+
{
272+
if (!preg_match("#^<details class='frame(?P<class>[^']*)'(?P<open>\\s+open)?\\s*>$#s", $openingTag, $match)) {
273+
return $originalFrame;
274+
}
278275

279-
if ($codeBlock !== '') {
280-
$fullFileTemplate = self::buildFullFileSourceTemplate($file, $line);
281-
$fullFileButton = $fullFileTemplate !== ''
282-
? "\n <button class='btn code-btn' "
283-
. "data-action='toggle-full-file'>Show full file</button>"
284-
: '';
285-
286-
$hidden = $isCodeOpen ? '' : ' hidden';
287-
$frame .= "\n <div class='frame-code-body'{$hidden}>"
288-
. "\n <div class='code-shell'>"
289-
. "\n {$codeBlock}"
290-
. "\n <div class='code-actions'>"
291-
. "\n <button class='btn code-btn' data-action='focus-line'>Focus line</button>"
292-
. str_replace("\n ", "\n ", $fullFileButton)
293-
. "\n </div>"
294-
. "\n </div>"
295-
. $fullFileTemplate
296-
. "\n </div>";
297-
}
276+
if (!preg_match(
277+
"#\\s*<summary>\\s*(<div class='frame-head'>.*?</div>)\\s*</summary>#s",
278+
$body,
279+
$summary
280+
)) {
281+
return $originalFrame;
282+
}
298283

299-
$extra = trim($body);
284+
$head = preg_replace("#\\s*<span class='chev'>.*?</span>#s", '', $summary[1]);
285+
$head = self::requireRenderedHtml($head, 'removing the frame-level backtrace chevron');
286+
$body = str_replace($summary[0], '', $body);
300287

301-
if ($extra !== '') {
302-
$frame .= "\n <div class='frame-extra'>{$extra}</div>";
303-
}
288+
$fileBlock = '';
289+
$file = null;
290+
$line = null;
304291

305-
return $frame . "\n </article>";
306-
},
307-
$html
292+
if (preg_match("#\\s*(<div class='frame-file'[^>]*>.*?</div>)#s", $body, $fileMatch)) {
293+
$fileBlock = $fileMatch[1];
294+
$body = str_replace($fileMatch[0], '', $body);
295+
$file = self::readHtmlAttribute($fileBlock, 'data-file');
296+
$lineValue = self::readHtmlAttribute($fileBlock, 'data-line');
297+
$line = is_numeric($lineValue) ? (int) $lineValue : null;
298+
}
299+
300+
$codeBlock = '';
301+
302+
if (preg_match("#\\s*(<div class='code'>.*?</div>)#s", $body, $codeMatch)) {
303+
$codeBlock = $codeMatch[1];
304+
$body = str_replace($codeMatch[0], '', $body);
305+
}
306+
307+
$isCodeOpen = ($match['open'] ?? '') !== '';
308+
$classes = trim(
309+
'frame'
310+
. $match['class']
311+
. ($codeBlock !== '' ? ' has-source' : ' no-source')
312+
. ($codeBlock !== '' && $isCodeOpen ? ' is-code-open' : '')
308313
);
309314

310-
return self::requireRenderedHtml($html, 'normalizing backtrace frame layout');
315+
if ($codeBlock !== '') {
316+
$head = self::makeBacktraceFrameHeadToggle($head, $isCodeOpen);
317+
}
318+
319+
$frame = "\n <article class='" . htmlspecialchars($classes, ENT_QUOTES) . "'>\n {$head}";
320+
321+
if ($fileBlock !== '') {
322+
$frame .= "\n {$fileBlock}";
323+
}
324+
325+
if ($codeBlock !== '') {
326+
$fullFileTemplate = self::buildFullFileSourceTemplate($file, $line);
327+
$fullFileButton = $fullFileTemplate !== ''
328+
? "\n <button class='btn code-btn' "
329+
. "data-action='toggle-full-file'>Show full file</button>"
330+
: '';
331+
332+
$hidden = $isCodeOpen ? '' : ' hidden';
333+
$frame .= "\n <div class='frame-code-body'{$hidden}>"
334+
. "\n <div class='code-shell'>"
335+
. "\n {$codeBlock}"
336+
. "\n <div class='code-actions'>"
337+
. "\n <button class='btn code-btn' data-action='focus-line'>Focus line</button>"
338+
. str_replace("\n ", "\n ", $fullFileButton)
339+
. "\n </div>"
340+
. "\n </div>"
341+
. $fullFileTemplate
342+
. "\n </div>";
343+
}
344+
345+
$extra = trim($body);
346+
347+
if ($extra !== '') {
348+
$frame .= "\n <div class='frame-extra'>{$extra}</div>";
349+
}
350+
351+
return $frame . "\n </article>";
311352
}
312353

313354
/**

‎tests/Fixtures/debug-error-page.php‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* Local-only smoke page for visually inspecting PhalconKit debug HTML.
7+
*
8+
* Run it from the repository root with:
9+
*
10+
* php -S 127.0.0.1:8976 tests/Fixtures/debug-error-page.php
11+
*
12+
* Then open http://127.0.0.1:8976/ in a browser.
13+
*/
14+
15+
use PhalconKit\Support\Debug;
16+
17+
require dirname(__DIR__, 2) . '/vendor/autoload.php';
18+
19+
$remoteAddress = $_SERVER['REMOTE_ADDR'] ?? '127.0.0.1';
20+
21+
if (!in_array($remoteAddress, ['127.0.0.1', '::1'], true)) {
22+
http_response_code(403);
23+
header('Content-Type: text/plain; charset=UTF-8');
24+
echo 'This debug smoke page is only available from localhost.';
25+
return;
26+
}
27+
28+
$debug = new Debug();
29+
30+
set_exception_handler(static function (\Throwable $exception) use ($debug): void {
31+
if (!headers_sent()) {
32+
http_response_code(500);
33+
header('Content-Type: text/html; charset=UTF-8');
34+
}
35+
36+
echo $debug->renderHtml($exception);
37+
});
38+
39+
/**
40+
* @param array{class: class-string, message: string} $payload
41+
*/
42+
$levelTwo = static function (array $payload): void {
43+
throw new \RuntimeException(sprintf(
44+
'PhalconKit debug error page smoke test for %s. %s',
45+
$payload['class'],
46+
$payload['message']
47+
));
48+
};
49+
50+
$levelOne = static function () use ($levelTwo): void {
51+
$levelTwo([
52+
'class' => Debug::class,
53+
'message' => 'This synthetic payload makes the debug argument table visible.',
54+
]);
55+
};
56+
57+
$levelOne();

‎tests/Unit/Support/DebugTest.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,36 @@ public function testRenderHtmlLinksPhalconKitClasses(): void
198198
$this->assertStringContainsString('PhalconKit\\Support\\Debug', $html);
199199
}
200200

201+
public function testBacktraceFrameNormalizationAvoidsDocumentWidePcreBacktracking(): void
202+
{
203+
$previousLimit = ini_get('pcre.backtrack_limit');
204+
$method = new \ReflectionMethod(Debug::class, 'normalizeBacktraceFrames');
205+
$html = "<section id='backtrace'>"
206+
. "<details class='frame app' open>"
207+
. "<summary><div class='frame-head'><span class='frame-num'>#0</span>"
208+
. "<span class='frame-call'><span class='fn'>demo</span></span>"
209+
. "<span class='chev' aria-hidden='true'>x</span></div></summary>"
210+
. str_repeat('x', 20_000)
211+
. '</details>'
212+
. '</section>';
213+
214+
try {
215+
ini_set('pcre.backtrack_limit', '1000');
216+
217+
$normalized = $method->invoke(null, $html);
218+
}
219+
finally {
220+
if (is_string($previousLimit)) {
221+
ini_set('pcre.backtrack_limit', $previousLimit);
222+
}
223+
}
224+
225+
$this->assertIsString($normalized);
226+
$this->assertStringContainsString("<article class='frame app no-source'>", $normalized);
227+
$this->assertStringNotContainsString("<details class='frame", $normalized);
228+
$this->assertStringContainsString("class='frame-extra'", $normalized);
229+
}
230+
201231
public function testUncaughtExceptionDebugPagesSetServerErrorStatus(): void
202232
{
203233
$debug = new class extends Debug {

0 commit comments

Comments
 (0)