From 8dd2b759821570fe2631768cc5ba5c0fe399aca9 Mon Sep 17 00:00:00 2001 From: emy Date: Wed, 9 Sep 2026 20:19:42 +0200 Subject: [PATCH] fix: harden production error handling and HTTP responses --- src/php/template.php | 107 +++++++++++++++++------------ tests/php/ProductionErrorsTest.php | 89 ++++++++++++++++++++++++ tests/php/render.php | 11 +++ 3 files changed, 162 insertions(+), 45 deletions(-) create mode 100644 tests/php/ProductionErrorsTest.php diff --git a/src/php/template.php b/src/php/template.php index fbcbb15..525ff52 100755 --- a/src/php/template.php +++ b/src/php/template.php @@ -154,9 +154,53 @@ * Enables console output in JS and PHP debugging. * Also enables random query-strings for js/css files to bust the cache */ - 'debug' => true + 'debug' => false ]; +/* Keep startup/configuration failures private until debug is explicitly enabled. */ +ini_set('display_errors', '0'); +ini_set('display_startup_errors', '0'); +ini_set('log_errors', '1'); + +/* Buffer this response so a rendering failure cannot leave a partial 200 page. */ +$responseBufferLevel = ob_get_level(); +ob_start(); + +function respondToError($status, $error = NULL) +{ + global $config, $responseBufferLevel; + + while(ob_get_level() > $responseBufferLevel) + { + ob_end_clean(); + } + + http_response_code($status); + header('Content-Type: text/html; charset=UTF-8'); + $messages = [403 => 'Forbidden', 404 => 'Not Found', 500 => 'Internal Server Error']; + echo '

' . $status . ' ' . $messages[$status] . '

'; + + if($error !== NULL) + { + if($status === 500) + { + /* Avoid logging trace arguments, which can contain configuration secrets. */ + error_log('IVFi: ' . get_class($error) . ': ' . $error->getMessage() + . ' in ' . $error->getFile() . ':' . $error->getLine()); + } + if(isset($config['debug']) && $config['debug'] === true) + { + echo '
' . htmlspecialchars((string) $error,
+        ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8') . '
'; + } + } + exit; +} + +set_exception_handler(function (Throwable $error) { + respondToError(500, $error); +}); + /* Any potential libraries and so on for extra features will appear here */ <%= buildInject.readmeSupport && buildInject.readmeSupport.PARSEDOWN_LIBRARY ? @@ -414,10 +458,7 @@ public function readJson($filePath) if(json_last_error() !== JSON_ERROR_NONE) { - if($this->debug) - { - echo json_last_error_msg(); - } + error_log('IVFi: Invalid JSON in ' . $filePath . ': ' . json_last_error_msg()); return false; } @@ -539,7 +580,7 @@ function http_digest_parse($text) /* Create header for when unathorized */ function createHeader($realm) { - header($_SERVER['SERVER_PROTOCOL'] . '401 Unauthorized'); + http_response_code(401); header('WWW-Authenticate: Digest realm="' . $realm . '",qop="auth",nonce="' . uniqid() . '",opaque="' . md5($realm) . '"'); } @@ -646,6 +687,9 @@ function getThemes($basePath, $themesPath) $config = include('.' . CONFIG_FILE); } +/* Apply the explicit setting before authentication and other request processing. */ +ini_set('display_errors', isset($config['debug']) && $config['debug'] === true ? '1' : '0'); + /* Default configuration values. Used if values from the above config are unset */ $defaults = array('authentication' => false,'single_page' => false,'format' => array('title' => 'Index of %s','date' => array('m/d/y H:i', 'd/m/y'),'sizes' => array(' B', ' KiB', ' MiB', ' GiB', ' TiB')),'icon' => array('path' => '/favicon.png','mime' => 'image/png'),'sorting' => array('enabled' => false,'order' => SORT_ASC,'types' => 0,'sort_by' => 'name','use_mbstring' => false),'gallery' => array('enabled' => true,'reverse_options' => false,'scroll_interval' => 50,'list_alignment' => 0,'fit_content' => true,'image_sharpen' => false),'preview' => array('enabled' => true,'hover_delay' => 75,'cursor_indicator' => true),'extensions' => array('image' => array('jpg', 'jpeg', 'png', 'gif', 'ico', 'svg', 'bmp', 'webp'),'video' => array('webm', 'mp4', 'ogv', 'ogg', 'mov')),'inject' => false,'style' => array('themes' => array('path' => '/<%= indexerPath %>/themes/','default' => false),'css' => array('additional' => false),'compact' => false),'filter' => array('file' => false,'directory' => false),'exclude' => false,'directory_sizes' => array('enabled' => false, 'recursive' => false),'processor' => false,'encode_all' => false,'allow_direct_access' => false,'path_checking' => 'strict','performance' => false,'footer' => array('enabled' => true, 'show_server_name' => true),'credits' => true,'debug' => false); @@ -886,8 +930,7 @@ function __construct($path, $options = []) /* If direct access is disabled, deny access */ if($this->allowDirectAccess === false) { - http_response_code(403); - die('Forbidden'); + respondToError(403); } else { /* If direct access is allowed, show current directory of script (if it is above base directory) */ $this->path = dirname($this->path); @@ -1282,13 +1325,7 @@ public function buildTable($sorting = false, $sortItems = 0, $sortType = 'modifi if($useMb === true && !function_exists('mb_strtolower')) { - http_response_code(500); - - die( - 'Error (mb_strtolower is not defined): In order to use mbstring, you\'ll need to ' . - 'install ' . - 'it first.' - ); + throw new RuntimeException('The mbstring extension is required when use_mbstring is enabled.'); } /** @@ -1407,7 +1444,12 @@ public function buildTable($sorting = false, $sortItems = 0, $sortType = 'modifi */ private function getFiles() { - return scandir($this->path, SCANDIR_SORT_NONE); + $files = scandir($this->path, SCANDIR_SORT_NONE); + if($files === false) + { + throw new RuntimeException('Unable to read directory: ' . $this->path); + } + return $files; } /** @@ -2018,34 +2060,9 @@ private function getReadableFileSize($bytes, $decimals = 1) ] ); } catch (Exception $e) { - http_response_code(500); - - /** Get error code */ - $eCode = $e->getCode(); - - echo implode('', [ - Helpers::createElement('h3', [], 'Error:'), - Helpers::createElement('p', [], $e . '({' . $eCode . '})') - ]); - - if($eCode === 1 || $eCode === 2) - { - echo Helpers::createElementHtml( - 'p', [], sprintf( - 'This error occurs when the requested directory is below the directory of the PHP file. %s', - $eCode === 1 - ? ( - '
You can try setting path_checking to weak ' . - 'if you are working with symbolic links etc.' - ) - : '' - ) - ); - } - - exit(Helpers::createElement( - 'p', [], 'Fatal error - Exiting.') - ); + /* These codes belong to the existing constructor's path checks only. */ + $statuses = [1 => 403, 2 => 403, 3 => 403, 4 => 404]; + respondToError(isset($statuses[$e->getCode()]) ? $statuses[$e->getCode()] : 500, $e); } /* Get directory data */ @@ -2593,4 +2610,4 @@ function constructJsConfig($config, $sorting, $timestamp, $bust, $theme) - \ No newline at end of file + \ No newline at end of file diff --git a/tests/php/ProductionErrorsTest.php b/tests/php/ProductionErrorsTest.php new file mode 100644 index 0000000..05fe74b --- /dev/null +++ b/tests/php/ProductionErrorsTest.php @@ -0,0 +1,89 @@ +root = sys_get_temp_dir() . '/ivfi-errors-' . bin2hex(random_bytes(8)); + mkdir($this->root, 0700); + } + + protected function tearDown(): void + { + if(is_file($this->root . '/.indexer.config.php')) { + unlink($this->root . '/.indexer.config.php'); + } + rmdir($this->root); + } + + private function request(string $uri, ?string $config = null): array + { + $build = dirname(__DIR__, 2) . '/build/indexer.php'; + self::assertFileExists($build, 'Run pnpm build before the PHP tests.'); + if($config !== null) { + file_put_contents($this->root . '/.indexer.config.php', ' ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes, $this->root + ); + self::assertIsResource($process); + fwrite($pipes[0], json_encode(['uri' => $uri, 'prepend' => '', 'captureResponse' => true])); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $errors); + return json_decode($output, true, 512, JSON_THROW_ON_ERROR) + ['log' => $errors]; + } + + public static function expectedErrors(): array + { + return [ + 'missing path, default config' => ['/missing/', null, 404, 'Not Found'], + 'missing path, explicit production' => ['/missing/', "['debug' => false]", 404, 'Not Found'], + 'path outside base' => ['/../', null, 403, 'Forbidden'], + 'direct file access disabled' => ['/.indexer.config.php', '[]', 403, 'Forbidden'], + ]; + } + + #[DataProvider('expectedErrors')] + public function testExpectedErrorsArePrivate(string $uri, ?string $config, int $status, string $message): void + { + $response = $this->request($uri, $config); + self::assertSame($status, $response['status']); + self::assertSame('

' . $status . ' ' . $message . '

', $response['body']); + self::assertSame('0', $response['displayErrors']); + self::assertSame('', $response['log']); + } + + public function testDebugIsExplicitAndKeepsTheCorrectStatus(): void + { + $response = $this->request('/missing/', "['debug' => true]"); + self::assertSame(404, $response['status']); + self::assertStringContainsString('Stack trace:', $response['body']); + self::assertSame('1', $response['displayErrors']); + } + + public function testRenderingErrorsDiscardPartialOutputAndLogDetails(): void + { + $response = $this->request('/', <<<'PHP' +['inject' => ['footer' => function () { + echo 'partial-output-marker'; + throw new TypeError('private failure '); +}]] +PHP); + self::assertSame(500, $response['status']); + self::assertSame('

500 Internal Server Error

', $response['body']); + self::assertStringContainsString('IVFi: TypeError: private failure ', $response['log']); + } +} diff --git a/tests/php/render.php b/tests/php/render.php index 6b9b491..09598cd 100644 --- a/tests/php/render.php +++ b/tests/php/render.php @@ -1,6 +1,17 @@ http_response_code() ?: 200, + 'body' => $body, + 'displayErrors' => ini_get('display_errors'), + ], JSON_THROW_ON_ERROR); + }); +} $_SERVER = [ 'REQUEST_URI' => $request['uri'], 'REQUEST_METHOD' => 'GET',