diff --git a/lib/Common/Logging/ExceptionHandler.php b/lib/Common/Logging/ExceptionHandler.php index e91d94d49b..e25c3ec64d 100644 --- a/lib/Common/Logging/ExceptionHandler.php +++ b/lib/Common/Logging/ExceptionHandler.php @@ -26,6 +26,9 @@ public static function Handle($exception) class WebExceptionHandler extends ExceptionHandler { + private const ANSI_BOLD_RED = '1;31'; + private const ANSI_DIM = '2'; + /** * @var callable */ @@ -38,10 +41,7 @@ public function __construct($callback) public function HandleException($exception) { - error_log('Error: ' . $exception); - ob_start(); - debug_print_backtrace(); - error_log(ob_get_clean()); + $this->WriteToStderr($exception); // Uncaught exceptions indicate a server-side failure. // Set 500 only while headers are still mutable. @@ -61,6 +61,84 @@ public function HandleException($exception) call_user_func($this->callback, $errorMessageId); } + + /** + * Written directly to stderr (not error_log()) so the trace keeps real + * newlines instead of Apache stamping its prefix on every line. Colors are + * only used when stderr is a terminal. Never throws. + */ + private function WriteToStderr($exception): void + { + $stderr = false; + try { + $stderr = $this->OpenStderr(); + $color = $stderr !== false && function_exists('stream_isatty') && stream_isatty($stderr); + $text = self::FormatException($exception, $color); + $written = $stderr === false ? false : fwrite($stderr, $text); + if ($written !== strlen($text)) { + // stderr unavailable or short write: plain text, never ANSI, to the PHP error log + error_log($color ? self::FormatException($exception) : $text); + } + } catch (Throwable $ignored) { + // logging must never prevent the error page from rendering + try { + error_log((string) $exception); + } catch (Throwable $alsoIgnored) { + } + } finally { + if (is_resource($stderr)) { + fclose($stderr); + } + } + } + + /** + * @return resource|false + */ + protected function OpenStderr() + { + return fopen('php://stderr', 'ab'); + } + + /** + * Formats an exception and its previous-exception chain for stderr. + * Control characters in messages are escaped so they cannot forge log + * lines or inject terminal escape sequences. + * + * @param Throwable $exception + * @param bool $color + * @return string + */ + public static function FormatException($exception, $color = false): string + { + $style = static fn (string $code, string $text): string => $color ? "\033[{$code}m{$text}\033[0m" : $text; + // C0 controls except tab, DEL, C1 controls (U+0080-U+009F) and U+2028/U+2029, matched bytewise + // so invalid UTF-8 in a message cannot make the match fail. + $clean = static fn (string $text): string => preg_replace_callback( + '/[\x00-\x08\x0A-\x1F\x7F]|\xC2[\x80-\x9F]|\xE2\x80[\xA8\xA9]/', + static fn (array $m): string => $m[0] === "\n" + ? "\n " + : implode('', array_map(static fn (string $b): string => sprintf('\\x%02X', ord($b)), str_split($m[0]))), + $text + ) ?? '[unprintable]'; + + $out = ''; + for ($e = $exception, $label = 'Uncaught exception'; $e !== null; $e = $e->getPrevious(), $label = 'Caused by') { + $out .= $style(self::ANSI_BOLD_RED, sprintf( + '%s: %s: %s in %s:%d', + $label, + $clean(get_class($e)), + $clean($e->getMessage()), + $clean($e->getFile()), + $e->getLine() + )) . "\n"; + foreach (explode("\n", $e->getTraceAsString()) as $line) { + $out .= $style(self::ANSI_DIM, ' => ' . $clean($line)) . "\n"; + } + } + + return $out; + } } set_exception_handler(['ExceptionHandler', 'Handle']); diff --git a/tests/lib/Common/Logging/ExceptionHandlerTest.php b/tests/lib/Common/Logging/ExceptionHandlerTest.php new file mode 100644 index 0000000000..19087449b8 --- /dev/null +++ b/tests/lib/Common/Logging/ExceptionHandlerTest.php @@ -0,0 +1,120 @@ +assertStringContainsString('Uncaught exception: RuntimeException: outer', $text); + $this->assertStringContainsString('Caused by: LogicException: root cause', $text); + } + + public function testEscapesControlCharactersAndNewlines(): void + { + $e = new Exception("bad\nUncaught exception: forged\033[31m"); + + $text = WebExceptionHandler::FormatException($e); + + $this->assertStringNotContainsString("\033", $text); + $this->assertStringNotContainsString("\nUncaught exception: forged", $text); + $this->assertStringContainsString('\x1B[31m', $text); + } + + public function testColorOnlyWhenRequested(): void + { + $e = new Exception('x'); + + $this->assertStringNotContainsString("\033", WebExceptionHandler::FormatException($e)); + $this->assertStringContainsString("\033[1;31m", WebExceptionHandler::FormatException($e, true)); + } + + public function testEscapesC1ControlsAndUnicodeLineSeparators(): void + { + $e = new Exception("a\u{0085}b\u{2028}c\u{009B}d"); + + $text = WebExceptionHandler::FormatException($e); + + $this->assertStringNotContainsString("\u{0085}", $text); + $this->assertStringNotContainsString("\u{2028}", $text); + $this->assertStringNotContainsString("\u{009B}", $text); + $this->assertStringContainsString('\xC2\x85', $text); + } + + public function testKeepsTabsAndEscapesNulInClassNames(): void + { + $text = WebExceptionHandler::FormatException(new Exception("a\tb")); + $this->assertStringContainsString("a\tb", $text); + + $anonymous = new class ('x') extends Exception { + }; + $this->assertStringNotContainsString("\0", WebExceptionHandler::FormatException($anonymous)); + } + + public function testFormatsNonExceptionThrowables(): void + { + $text = WebExceptionHandler::FormatException(new TypeError('boom')); + + $this->assertStringContainsString('Uncaught exception: TypeError: boom', $text); + } + + public function testHandleExceptionWritesToStderrStream(): void + { + $file = tempnam(sys_get_temp_dir(), 'lbexc'); + $handler = new StreamedWebExceptionHandler(static function () { + }, $file); + + try { + $handler->HandleException(new Exception('to stream')); + $written = file_get_contents($file); + } finally { + unlink($file); + } + + $this->assertStringContainsString('Uncaught exception: Exception: to stream', $written); + } + + public function testHandleExceptionFallsBackToErrorLogWhenStderrUnavailable(): void + { + $logFile = tempnam(sys_get_temp_dir(), 'lbexc'); + $previous = ini_set('error_log', $logFile); + $called = false; + + try { + $handler = new StreamedWebExceptionHandler(static function () use (&$called) { + $called = true; + }, false); + $handler->HandleException(new Exception('fallback')); + $logged = file_get_contents($logFile); + } finally { + ini_set('error_log', $previous === false ? '' : $previous); + unlink($logFile); + } + + $this->assertStringContainsString('Uncaught exception: Exception: fallback', $logged); + $this->assertTrue($called, 'error page callback must still run'); + } +} + +class StreamedWebExceptionHandler extends WebExceptionHandler +{ + /** @var string|false path to write to, or false to simulate unavailable stderr */ + private $target; + + public function __construct($callback, $target) + { + parent::__construct($callback); + $this->target = $target; + } + + protected function OpenStderr() + { + return $this->target === false ? false : fopen($this->target, 'ab'); + } +}