diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 313184e2..dcf58f3a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,7 +47,7 @@ jobs: - uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} - extensions: ${{ !matrix.intl && ':intl' || '' }} + extensions: pdo_mysql${{ !matrix.intl && ', :intl' || '' }} - uses: actions/checkout@v7 diff --git a/Dockerfile b/Dockerfile index 200f9868..047863f9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,10 +1,14 @@ FROM docker.io/library/php:8.4-apache COPY --from=ghcr.io/mlocati/php-extension-installer /usr/bin/install-php-extensions /usr/local/bin/ -RUN install-php-extensions exif gd zip pdo_mysql intl +RUN install-php-extensions exif gd zip pdo_mysql pdo_sqlite intl imap RUN a2enmod rewrite +# Container health check (Apache on :80) +HEALTHCHECK --interval=30s --timeout=5s --start-period=15s \ + CMD php -r "exit(@fsockopen('127.0.0.1', 80) ? 0 : 1);" + COPY --chown=33:33 . /var/www/html/ -RUN echo 'upload_max_filesize = 1024M' > /usr/local/etc/php/conf.d/limits.ini +RUN printf 'upload_max_filesize = 1024M\npost_max_size = 1100M\n' > /usr/local/etc/php/conf.d/limits.ini RUN mv "$PHP_INI_DIR/php.ini-production" "$PHP_INI_DIR/php.ini" diff --git a/app/controller/admin.php b/app/controller/admin.php index 30583486..9e7a3b69 100644 --- a/app/controller/admin.php +++ b/app/controller/admin.php @@ -349,11 +349,11 @@ public function user_save(\Base $f3): void throw new \Exception("Please enter a name."); } - if (preg_match("/#?[0-9a-f]{3,6}/i", (string) $f3->get("POST.task_color")) === false) { + if (preg_match("/^#?(?:[0-9a-f]{3}|[0-9a-f]{6})\\z/i", (string) $f3->get("POST.task_color")) !== 1) { throw new \Exception("Please enter a valid hex color."); } - if (preg_match("/[0-9a-z_-]+/i", (string) $f3->get("POST.username")) === false) { + if (preg_match("/^[0-9a-z_-]+\\z/i", (string) $f3->get("POST.username")) !== 1) { throw new \Exception("Usernames can only contain letters, numbers, hyphens, and underscores."); } @@ -593,7 +593,12 @@ public function group_ajax(\Base $f3): void $this->_printJson(["changed" => 1]); break; case "change_task_color": - $group->task_color = ltrim((string) $f3->get("POST.value"), '#'); + $color = ltrim((string) $f3->get("POST.value"), '#'); + if (!ctype_xdigit($color) || (strlen($color) !== 3 && strlen($color) !== 6)) { + $f3->error(400); + return; + } + $group->task_color = $color; $group->save(); $this->_printJson(["changed" => 1]); break; diff --git a/app/controller/index.php b/app/controller/index.php index fcba48b0..1fbd0063 100644 --- a/app/controller/index.php +++ b/app/controller/index.php @@ -148,7 +148,7 @@ public function registerpost($f3): void $errors[] = "Name is required"; } - if (preg_match("/^[0-9a-z]{4,}$/i", (string) $f3->get("POST.register-username")) === false) { + if (preg_match("/^[0-9a-z]{4,}$/i", (string) $f3->get("POST.register-username")) !== 1) { $errors[] = "Usernames must be at least 4 characters and can only contain letters and numbers."; } diff --git a/app/helper/security.php b/app/helper/security.php index 7d141eb8..fc67890d 100644 --- a/app/helper/security.php +++ b/app/helper/security.php @@ -103,23 +103,82 @@ public function checkDatabaseVersion(): string|bool } /** - * Install latest core database updates + * Install a core database update + * @return bool TRUE if the update was applied, FALSE on failure */ - public function updateDatabase(string $version): void + public function updateDatabase(string $version): bool { $f3 = \Base::instance(); - if (file_exists("db/{$version}.sql")) { - $update_db = file_get_contents("db/{$version}.sql"); - $db = $f3->get("db.instance"); - foreach (explode(";", $update_db) as $stmt) { - $db->exec($stmt); + if (!file_exists("db/{$version}.sql")) { + $f3->set("error", " Database file not found for version: {$version}"); + return false; + } + + $update_db = file_get_contents("db/{$version}.sql"); + $db = $f3->get("db.instance"); + try { + // Note: MySQL implicitly commits around DDL statements, so a + // failed migration there can't be rolled back. The version row + // is only updated by the last statement, so a failure leaves + // the database at the previous version and surfaces an error. + // Already-applied DDL (e.g. an added column) stays in place, + // but replaying it on retry is tolerated (see + // isAlreadyAppliedSchemaError()), so no manual cleanup is + // needed before retrying. SQLite migrations are fully atomic. + $db->begin(); + foreach (explode(";", (string) $update_db) as $stmt) { + if (trim($stmt) !== "") { + try { + $db->exec($stmt); + } catch (\Throwable $e) { + if (!$this->isAlreadyAppliedSchemaError($db->driver(), $e)) { + throw $e; + } + } + } + } + if ($db->pdo()->inTransaction()) { + $db->commit(); } + } catch (\Throwable $e) { + try { + if ($db->pdo()->inTransaction()) { + $db->rollback(); + } + } catch (\Throwable) { + // Keep the original migration error. + } + $f3->set("error", " Database update to version {$version} failed: " . $e->getMessage()); + return false; + } - \Cache::instance()->reset(); - $f3->set("success", " Database updated to version: {$version}"); - } else { - $f3->set("error", " Database file not found for version: {$version}"); + \Cache::instance()->reset(); + $f3->set("success", " Database updated to version: {$version}"); + return true; + } + + /** + * Check whether a migration statement failure only means the statement + * was already applied by an earlier partial run. + * + * MySQL implicitly commits DDL, so when a migration fails partway + * through, the statements that already ran stay applied; retrying the + * migration replays them, and each replayed statement fails. A replayed + * statement fails with a "duplicate"/"already exists" schema error, + * which is safe to skip because the desired end state is already in + * place. Anything else — including data-dependent "Duplicate entry" + * errors — is a real failure and must still abort the migration. + * @param string $driver PDO driver name ("mysql", "sqlite", ...) + */ + protected function isAlreadyAppliedSchemaError(string $driver, \Throwable $e): bool + { + if ($driver !== "mysql") { + return false; } + return (bool) preg_match( + "/\\bduplicate\\s+(column|key|index|foreign key constraint)\\s+name\\b|\\balready exists\\b/i", + $e->getMessage() + ); } /** diff --git a/app/helper/view.php b/app/helper/view.php index e9f4c482..6c267994 100644 --- a/app/helper/view.php +++ b/app/helper/view.php @@ -258,11 +258,18 @@ protected function _parseEmoticons(string $str): string */ protected function _parseTextile(string $str, bool $escape = true): string { - error_reporting(E_ALL ^ E_DEPRECATED); // Temporarily ignore deprecations because this lib is incompatible - $tex = new \Netcarver\Textile\Parser('html5'); - $tex->setDimensionlessImages(true); - $tex->setRestricted($escape); - return $tex->parse($str); + // Temporarily ignore deprecations because this lib is incompatible, + // restoring the previous level afterwards so deprecations elsewhere + // in the request are still reported. + $level = error_reporting(E_ALL ^ E_DEPRECATED); + try { + $tex = new \Netcarver\Textile\Parser('html5'); + $tex->setDimensionlessImages(true); + $tex->setRestricted($escape); + return $tex->parse($str); + } finally { + error_reporting($level); + } } /** diff --git a/app/model/issue.php b/app/model/issue.php index 9e37f1a2..d6761c2e 100644 --- a/app/model/issue.php +++ b/app/model/issue.php @@ -50,8 +50,15 @@ public static function create(array $data, bool $notify = true): static } if (!empty($data["due_date"])) { - if (preg_match("/\\d{4}(-\\d{2}){2}/", (string) $data["due_date"]) === false) { - $data["due_date"] = date("Y-m-d", strtotime((string) $data["due_date"])); + $dueDate = (string) $data["due_date"]; + $date = \DateTime::createFromFormat("Y-m-d", $dueDate); + if ($date === false || $date->format("Y-m-d") !== $dueDate) { + // Not already a valid Y-m-d date, try to parse it + $parsed = strtotime($dueDate); + if ($parsed === false) { + throw new \Exception("Please enter a valid due date."); + } + $data["due_date"] = date("Y-m-d", $parsed); } if (empty($data["sprint_id"]) && !empty($data['due_date_sprint'])) { diff --git a/composer.json b/composer.json index c675b57e..b6562770 100644 --- a/composer.json +++ b/composer.json @@ -22,6 +22,7 @@ } }, "require-dev": { + "phpstan/phpstan": "^2.2.14", "squizlabs/php_codesniffer": "^3.13.6", "rector/rector": "^2.7.0", "phpunit/phpunit": "^10.5.66" diff --git a/composer.lock b/composer.lock index d659cf72..ecd46c95 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "f504002b81dba8d7a6e86312822577b0", + "content-hash": "10dfbafc328a68e64422db0a9405f314", "packages": [ { "name": "bcosca/fatfree-core", @@ -2633,15 +2633,15 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": {}, + "stability-flags": [], "prefer-stable": false, "prefer-lowest": false, "platform": { "php": ">=8.1.0" }, - "platform-dev": {}, + "platform-dev": [], "platform-overrides": { "php": "8.1.33" }, - "plugin-api-version": "2.9.0" + "plugin-api-version": "2.6.0" } diff --git a/cron/base.php b/cron/base.php index 92fd29bc..228269d7 100644 --- a/cron/base.php +++ b/cron/base.php @@ -23,14 +23,27 @@ ]); // Load local configuration -$f3->mset(require_once(__DIR__ . '/config.php')); +$config_file = $homedir . "config.php"; +if (!is_file($config_file) && is_file(__DIR__ . "/config.php")) { + // Fall back to legacy cron-specific config location + $config_file = __DIR__ . "/config.php"; +} +$f3->mset(require_once($config_file)); // Connect to database -$f3->set("db.instance", new \Helper\SQL( - "mysql:host=" . $f3->get("db.host") . ";port=3306;dbname=" . $f3->get("db.name"), - $f3->get("db.user"), - $f3->get("db.pass") -)); +if ($f3->get("db.engine") == "sqlite") { + $db_name = $f3->get("db.name"); + if ($db_name !== ":memory:" && substr($db_name, 0, 1) !== "/") { + $db_name = $homedir . $db_name; + } + $f3->set("db.instance", new \Helper\SQL("sqlite:" . $db_name)); +} else { + $f3->set("db.instance", new \Helper\SQL( + "mysql:host=" . $f3->get("db.host") . ";port=" . ($f3->get("db.port") ?: 3306) . ";dbname=" . $f3->get("db.name"), + $f3->get("db.user"), + $f3->get("db.pass") + )); +} // Load database-backed config \Model\Config::loadAll(); diff --git a/index.php b/index.php index 5acfb34f..e383ae36 100644 --- a/index.php +++ b/index.php @@ -16,6 +16,7 @@ "CACHE" => true, "AUTOLOAD" => "app/;lib/vendor/", "JAR.samesite" => "Lax", + "JAR.secure" => $f3->get("SCHEME") == "https", "PACKAGE" => "Phproject", "TZ" => "UTC", "microtime" => microtime(true), @@ -82,17 +83,36 @@ "mysql:host=" . $f3->get("db.host") . ";port=" . $f3->get("db.port") . ";dbname=" . $f3->get("db.name"), $f3->get("db.user"), $f3->get("db.pass"), - [\PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8mb4;'] + // Pdo\Mysql::ATTR_INIT_COMMAND exists on PHP 8.4+; PDO::MYSQL_ATTR_INIT_COMMAND is deprecated since 8.5 + [(\PHP_VERSION_ID >= 80400 ? \Pdo\Mysql::ATTR_INIT_COMMAND : \PDO::MYSQL_ATTR_INIT_COMMAND) => 'SET NAMES utf8mb4;'] )); } // Load final configuration \Model\Config::loadAll(); -// Ensure database is up to date -$version = \Helper\Security::instance()->checkDatabaseVersion(); -if ($version !== true) { - \Helper\Security::instance()->updateDatabase($version); +// Ensure database is up to date, applying all pending migrations in one request. +// Serialized with a non-blocking file lock so concurrent requests can't run +// the same migration twice; a request that can't acquire the lock skips this, +// since another request is already migrating. +$lock = fopen($f3->get("TEMP") . "migrations.lock", "c"); +if ($lock && flock($lock, LOCK_EX | LOCK_NB)) { + try { + $security = \Helper\Security::instance(); + $migrations = 0; + while (($version = $security->checkDatabaseVersion()) !== true && $migrations < 100) { + if (!$security->updateDatabase($version)) { + break; + } + $f3->set("version", $version); + $migrations++; + } + } finally { + flock($lock, LOCK_UN); + fclose($lock); + } +} elseif ($lock) { + fclose($lock); } // Set up CSRF protection diff --git a/nginx-example.conf b/nginx-example.conf index d7e34336..45c9b28a 100644 --- a/nginx-example.conf +++ b/nginx-example.conf @@ -32,5 +32,5 @@ server { include fastcgi_params; } - client_max_body_size 64M; + client_max_body_size 1100M; # must be >= PHP post_max_size (see Dockerfile) } diff --git a/phpstan.neon b/phpstan.neon index 67386ceb..37ac9cf6 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -6,6 +6,8 @@ parameters: - tests excludePaths: - app/plugin/* + scanFiles: + - phpstan/scan/pdo_mysql.php ignoreErrors: - '#Unsafe usage of new static\(\).#' - '#Access to an undefined property Model\\Custom::.*#' diff --git a/phpstan/scan/pdo_mysql.php b/phpstan/scan/pdo_mysql.php new file mode 100644 index 00000000..a028a5a3 --- /dev/null +++ b/phpstan/scan/pdo_mysql.php @@ -0,0 +1,17 @@ + needle => one-time failure message */ + public array $failOnce = []; + + public function driver(): string + { + return "mysql"; + } + + public function begin(): bool + { + return true; + } + + public function commit(): bool + { + return true; + } + + public function rollback(): bool + { + return true; + } + + public function pdo(): object + { + return new class { + public function inTransaction(): bool + { + return true; + } + }; + } + + public function exec(string $stmt) + { + $stmt = trim($stmt); + foreach ($this->failOnce as $needle => $message) { + if (str_contains($stmt, $needle)) { + unset($this->failOnce[$needle]); + // Statements applied before this one stay applied: + // MySQL implicitly committed them. + throw new \Exception("PDOStatement: " . $message); + } + } + if (in_array($stmt, $this->applied, true)) { + // Replay of already-applied DDL, exactly as F3 surfaces it. + throw new \Exception("PDOStatement: " . $this->duplicateMessage($stmt)); + } + $this->applied[] = $stmt; + return 0; + } + + private function duplicateMessage(string $stmt): string + { + if (str_contains($stmt, "ADD COLUMN")) { + return "Duplicate column name 'c1'"; + } + if (str_contains($stmt, "CREATE TABLE")) { + return "Table 't' already exists"; + } + return "Duplicate key name 'k'"; + } +} + diff --git a/tests/apiTest.php b/tests/apiTest.php index 71a62dd3..7c6398a8 100644 --- a/tests/apiTest.php +++ b/tests/apiTest.php @@ -60,7 +60,8 @@ protected function setUp(): void "mysql:host=" . $f3->get("db.host") . ";port=" . $f3->get("db.port") . ";dbname=" . $f3->get("db.name"), $f3->get("db.user"), $f3->get("db.pass"), - [\PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8mb4;'] + // Pdo\Mysql::ATTR_INIT_COMMAND exists on PHP 8.4+; PDO::MYSQL_ATTR_INIT_COMMAND is deprecated since 8.5 + [(\PHP_VERSION_ID >= 80400 ? \Pdo\Mysql::ATTR_INIT_COMMAND : \PDO::MYSQL_ATTR_INIT_COMMAND) => 'SET NAMES utf8mb4;'] )); } diff --git a/tests/migrationTest.php b/tests/migrationTest.php new file mode 100644 index 00000000..26027f9a --- /dev/null +++ b/tests/migrationTest.php @@ -0,0 +1,152 @@ +migrationFile = dirname(__DIR__) . "/db/99.99.99.sql"; + } + + protected function tearDown(): void + { + if (file_exists($this->migrationFile)) { + unlink($this->migrationFile); + } + $f3 = \Base::instance(); + $f3->clear("db.instance"); + $f3->clear("error"); + $f3->clear("success"); + } + + private function writeMigration(string $sql): void + { + file_put_contents($this->migrationFile, $sql); + } + + private function isAlreadyApplied(string $driver, string $message): bool + { + $method = new \ReflectionMethod(\Helper\Security::class, "isAlreadyAppliedSchemaError"); + $method->setAccessible(true); + return $method->invoke(\Helper\Security::instance(), $driver, new \Exception($message)); + } + + public function testClassifierToleratesOnlyReplayedSchemaObjects(): void + { + // Replayed DDL on MySQL: safe to skip. + $this->assertTrue($this->isAlreadyApplied("mysql", "PDOStatement: Duplicate column name 'c1'")); + $this->assertTrue($this->isAlreadyApplied("mysql", "PDOStatement: Duplicate key name 'idx'")); + $this->assertTrue($this->isAlreadyApplied("mysql", "PDOStatement: Table 't' already exists")); + $this->assertTrue( + $this->isAlreadyApplied("mysql", "PDOStatement: Duplicate foreign key constraint name 'fk'") + ); + // Data-dependent and other errors: must still abort. + $this->assertFalse( + $this->isAlreadyApplied("mysql", "PDOStatement: Duplicate entry '1' for key 'PRIMARY'") + ); + $this->assertFalse( + $this->isAlreadyApplied("mysql", "PDOStatement: You have an error in your SQL syntax") + ); + $this->assertFalse( + $this->isAlreadyApplied("mysql", "PDOStatement: Unknown column 'nope' in 'field list'") + ); + // Other drivers: never tolerated (SQLite migrations are atomic). + $this->assertFalse($this->isAlreadyApplied("sqlite", "PDOStatement: Duplicate column name 'c1'")); + } + + public function testPartialMySqlMigrationCanBeRetriedWithoutCleanup(): void + { + $f3 = \Base::instance(); + $db = new FakeMySqlForMigrationTest(); + // First attempt fails on the second statement. + $db->failOnce = ["THIS IS NOT VALID SQL" => "You have an error in your SQL syntax near 'THIS'"]; + $f3->set("db.instance", $db); + + $this->writeMigration( + "ALTER TABLE `t` ADD COLUMN `c1` INT NULL;\n" . + "THIS IS NOT VALID SQL;\n" . + "UPDATE `config` SET `value` = '99.99.99' WHERE `attribute` = 'version';\n" + ); + + $security = \Helper\Security::instance(); + $this->assertFalse($security->updateDatabase("99.99.99")); + $this->assertStringContainsString("failed", (string) $f3->get("error")); + // The first DDL statement stayed applied (MySQL implicit commit). + $this->assertCount(1, $db->applied); + + // The fixed migration is retried as-is on the next request: the + // already-applied statement replays and must be tolerated. + $this->writeMigration( + "ALTER TABLE `t` ADD COLUMN `c1` INT NULL;\n" . + "ALTER TABLE `t` ADD COLUMN `c2` INT NULL;\n" . + "UPDATE `config` SET `value` = '99.99.99' WHERE `attribute` = 'version';\n" + ); + $f3->clear("error"); + + $this->assertTrue($security->updateDatabase("99.99.99")); + $this->assertSame(" Database updated to version: 99.99.99", $f3->get("success")); + // c1 was applied exactly once; c2 and the version update ran on retry. + $this->assertCount(3, $db->applied); + } + + public function testRealMySqlErrorsStillAbortTheMigration(): void + { + $f3 = \Base::instance(); + $db = new FakeMySqlForMigrationTest(); + // A data-dependent failure must never be mistaken for replayed DDL. + $db->failOnce = ["INSERT" => "Duplicate entry '1' for key 'PRIMARY'"]; + $f3->set("db.instance", $db); + + $this->writeMigration( + "ALTER TABLE `t` ADD COLUMN `c1` INT NULL;\n" . + "INSERT INTO `t` (`id`) VALUES (1);\n" . + "UPDATE `config` SET `value` = '99.99.99' WHERE `attribute` = 'version';\n" + ); + + $this->assertFalse(\Helper\Security::instance()->updateDatabase("99.99.99")); + $this->assertStringContainsString("Duplicate entry", (string) $f3->get("error")); + } + + public function testSqliteMigrationStaysAtomic(): void + { + $f3 = \Base::instance(); + $dbFile = $f3->get("TEMP") . "migration-test.sqlite"; + @unlink($dbFile); + $db = new \Helper\SQL("sqlite:" . $dbFile); + $db->exec("CREATE TABLE `config` (`attribute` TEXT PRIMARY KEY, `value` TEXT)"); + $db->exec("INSERT INTO `config` VALUES ('version', '00.00.00')"); + $f3->set("db.instance", $db); + + try { + // A failing migration rolls back entirely on SQLite. + $this->writeMigration( + "CREATE TABLE `migtest` (`id` INTEGER PRIMARY KEY);\n" . + "THIS IS NOT VALID SQL;\n" . + "UPDATE `config` SET `value` = '99.99.99' WHERE `attribute` = 'version';\n" + ); + $this->assertFalse(\Helper\Security::instance()->updateDatabase("99.99.99")); + $tables = $db->exec("SELECT name FROM sqlite_master WHERE type='table' AND name='migtest'"); + $this->assertSame([], $tables); + $version = $db->exec("SELECT value FROM `config` WHERE `attribute` = 'version'"); + $this->assertSame("00.00.00", $version[0]["value"]); + + // A clean migration applies and bumps the version. + $this->writeMigration( + "CREATE TABLE `migtest` (`id` INTEGER PRIMARY KEY);\n" . + "UPDATE `config` SET `value` = '99.99.99' WHERE `attribute` = 'version';\n" + ); + $this->assertTrue(\Helper\Security::instance()->updateDatabase("99.99.99")); + $version = $db->exec("SELECT value FROM `config` WHERE `attribute` = 'version'"); + $this->assertSame("99.99.99", $version[0]["value"]); + } finally { + @unlink($dbFile); + } + } +} diff --git a/tests/webFeatureTest.php b/tests/webFeatureTest.php index 4ecf4c55..28552a5d 100644 --- a/tests/webFeatureTest.php +++ b/tests/webFeatureTest.php @@ -12,6 +12,12 @@ class WebFeatureTest extends TestCase protected bool $configured = false; protected int $initialOutputBufferLevel = 0; + /** + * HTTP status captured from F3's error handling during the last mock() + * call; null when the route completed without an error. + */ + protected ?int $lastMockStatus = null; + protected function setUp(): void { $this->initialOutputBufferLevel = ob_get_level(); @@ -41,13 +47,18 @@ protected function setUp(): void 'mysql:host=' . $f3->get('db.host') . ';port=' . $f3->get('db.port') . ';dbname=' . $f3->get('db.name'), $f3->get('db.user'), $f3->get('db.pass'), - [\PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8mb4;'] + // Pdo\Mysql::ATTR_INIT_COMMAND exists on PHP 8.4+; PDO::MYSQL_ATTR_INIT_COMMAND is deprecated since 8.5 + [(\PHP_VERSION_ID >= 80400 ? \Pdo\Mysql::ATTR_INIT_COMMAND : \PDO::MYSQL_ATTR_INIT_COMMAND) => 'SET NAMES utf8mb4;'] )); } \Model\Config::loadAll(); \Helper\Security::instance()->initCsrfToken(); + // Replicate index.php's global template data so full-page renders + // (e.g. the footer's issue-type picker) behave as in production. + $f3->set('issue_types', (new \Model\Issue\Type())->find()); + $this->configured = true; } @@ -75,6 +86,11 @@ protected function mock(string $route, ?array $args = null, ?array $headers = nu // Prevent F3 from calling exit by setting HALT to false $f3->set('HALT', false); + // Capture the error status from F3's error handling rather than + // parsing output: a route that emits partial content before failing + // would otherwise be misread as a 200. + $this->lastMockStatus = null; + // Set custom error handler that won't exit $f3->set('ONERROR', function ($f3) { echo json_encode([ @@ -92,11 +108,20 @@ protected function mock(string $route, ?array $args = null, ?array $headers = nu $f3->mock($route, $args, $headers); } catch (\Throwable $e) { // Catch any uncaught exceptions + $this->lastMockStatus = 500; echo json_encode([ "status" => 500, "error" => $e->getMessage() ], JSON_THROW_ON_ERROR); } finally { + // Capture F3's recorded error status before clearing it. This + // works regardless of which ONERROR handler ran (controllers + // may install their own) and even when the route emitted partial + // output before failing. + if ($this->lastMockStatus === null) { + $code = $f3->get('ERROR.code'); + $this->lastMockStatus = $code === null ? null : (int) $code; + } // Restore original handlers $f3->set('ONERROR', $originalErrorHandler); $f3->set('ONREROUTE', $originalOnReroute); @@ -295,4 +320,68 @@ public function testAdminCannotModifyHigherRankUser(): void $peer->erase(); } } + + /** + * Public routes that should respond without a fatal error. + * @return array + */ + public static function publicRouteProvider(): array + { + return [ + 'login page renders' => ['GET /login', 200], + 'anonymous home reroutes' => ['GET /', null], + 'opensearch renders' => ['GET /opensearch.xml', 200], + 'api without key is rejected' => ['GET /issues.json', 401], + ]; + } + + /** + * Smoke test: hit public routes and assert none of them fatal (HTTP 500). + * Catches PHP-version incompatibilities in controllers/templates early. + */ + #[\PHPUnit\Framework\Attributes\DataProvider('publicRouteProvider')] + public function testPublicRouteDoesNotFatal(string $route, ?int $expectedStatus): void + { + if (!$this->configured) { + $this->markTestSkipped(); + } + + // Ensure no user leaks in from other tests; these routes are public + // and the API case specifically asserts anonymous access is rejected. + $f3 = \Base::instance(); + $f3->clear('user'); + $f3->clear('user_obj'); + + $output = $this->mock($route); + $this->assertIsString($output, "Route {$route} failed to mock"); + + // Use the status captured from F3's error handling; a route that + // emits partial content before failing must still report its 500. + $status = $this->lastMockStatus ?? 200; + + $this->assertNotSame(500, $status, "Route {$route} caused a fatal error"); + if ($expectedStatus !== null) { + $this->assertSame($expectedStatus, $status, "Route {$route} returned an unexpected status"); + } + } + + /** + * A route that emits partial output before failing must still report + * its 500 status instead of being misread as a 200. + */ + public function testMockCapturesStatusAfterPartialOutput(): void + { + if (!$this->configured) { + $this->markTestSkipped(); + } + + $f3 = \Base::instance(); + $f3->route('GET /_test_partial_output', function (): void { + echo 'partial output'; + throw new \Exception('boom'); + }); + + $this->mock('GET /_test_partial_output'); + $this->assertSame(500, $this->lastMockStatus); + } } diff --git a/uploads/.htaccess b/uploads/.htaccess index 3a428827..da186912 100644 --- a/uploads/.htaccess +++ b/uploads/.htaccess @@ -1 +1,6 @@ +# Apache 2.2 syntax (ignored on 2.4 without mod_access_compat) Deny from all +# Apache 2.4 syntax + + Require all denied +