|
| 1 | +<?php |
| 2 | + |
| 3 | +declare(strict_types=1); |
| 4 | + |
| 5 | +namespace Sbpp\Tests\Integration; |
| 6 | + |
| 7 | +use PDOException; |
| 8 | +use PHPUnit\Framework\Attributes\PreserveGlobalState; |
| 9 | +use PHPUnit\Framework\Attributes\RunInSeparateProcess; |
| 10 | +use Sbpp\Db\Database; |
| 11 | +use Sbpp\Tests\ApiTestCase; |
| 12 | +use Smarty\Smarty; |
| 13 | + |
| 14 | +/** |
| 15 | + * Issue #1314: `web/pages/admin.srvadmins.php` issued a query that |
| 16 | + * mentioned the named placeholder `:sid` twice (one in the outer |
| 17 | + * `WHERE server_id = :sid`, one in the inner subquery's |
| 18 | + * `WHERE server_id = :sid`) but called `bind(':sid', ...)` only |
| 19 | + * ONCE. Under emulated prepares (PDO's pre-#1124 default), client-side |
| 20 | + * substitution rewrote every `:sid` occurrence to the literal value |
| 21 | + * before the SQL hit MariaDB, so the duplicate-name pattern Just |
| 22 | + * Worked. After #1124 / #1167 flipped `PDO::ATTR_EMULATE_PREPARES` |
| 23 | + * to `false` (so `LIMIT '0','30'` would stop tripping MariaDB strict |
| 24 | + * mode), the MySQL driver started expanding each `:name` occurrence |
| 25 | + * into its own positional `?` slot in the prepared statement — |
| 26 | + * `bind()`-ing one occurrence leaves the others unbound and |
| 27 | + * `execute()` raises `SQLSTATE[HY093] Invalid parameter number`. |
| 28 | + * |
| 29 | + * The fix renames the inner placeholder to `:sid_inner` and binds |
| 30 | + * both. The two test methods here pin both halves of the contract: |
| 31 | + * |
| 32 | + * 1. {@see testReusedNamedPlaceholderUnderNativePreparesIsRejected} |
| 33 | + * — a small standalone query against `Sbpp\Db\Database` that |
| 34 | + * reproduces the offending shape (one `bind()` for two `:sid` |
| 35 | + * slots) and asserts it throws `HY093`. This documents WHY the |
| 36 | + * rename in `admin.srvadmins.php` matters; if a future |
| 37 | + * contributor "tidies up" the SQL back into `:sid` + a single |
| 38 | + * bind, this test fails before the page does in production. |
| 39 | + * It also doubles as a regression guard if anyone re-flips |
| 40 | + * `EMULATE_PREPARES` back to `true` (which would silently |
| 41 | + * reintroduce the original masking). |
| 42 | + * |
| 43 | + * 2. {@see testAdminSrvadminsPageRendersWithoutPdoException} — the |
| 44 | + * end-to-end shape: requires the page handler with `$_GET['id']` |
| 45 | + * set, asserts the SELECT prepares + executes cleanly through |
| 46 | + * the global `$PDO` wrapper. Mirrors the |
| 47 | + * Php82DeprecationsTest stub-Smarty + process-isolation pattern |
| 48 | + * so the page handler's top-level helper declarations don't |
| 49 | + * collide across cases and `$_GET` / `$_SESSION` state stays |
| 50 | + * clean per case. |
| 51 | + */ |
| 52 | +final class SrvAdminsPdoParamTest extends ApiTestCase |
| 53 | +{ |
| 54 | + /** |
| 55 | + * Capture-only Smarty stub. The page handler calls |
| 56 | + * `$theme->assign(...)` and `$theme->display(...)`; we don't care |
| 57 | + * about the rendered output, only that the PHP code path |
| 58 | + * (including the SELECT under test) runs without raising. |
| 59 | + * Mirrors `Php82DeprecationsTest::makeStubTheme()`. |
| 60 | + */ |
| 61 | + private function makeStubTheme(): Smarty |
| 62 | + { |
| 63 | + return new class extends Smarty { |
| 64 | + /** @phpstan-ignore method.childParameterType */ |
| 65 | + public function assign($tpl_var, $value = null, $nocache = false, $scope = null) |
| 66 | + { |
| 67 | + return $this; |
| 68 | + } |
| 69 | + |
| 70 | + public function display($template = null, $cache_id = null, $compile_id = null) |
| 71 | + { |
| 72 | + return ''; |
| 73 | + } |
| 74 | + }; |
| 75 | + } |
| 76 | + |
| 77 | + /** |
| 78 | + * Wire the globals every page handler under `web/pages/*` reads |
| 79 | + * straight from `$GLOBALS` (`$theme`, `$userbank`, `$PDO`). The |
| 80 | + * production `init.php` does this on every real request; for the |
| 81 | + * test we mirror what `Php82DeprecationsTest::bootRenderHarness()` |
| 82 | + * sets up so the require'd page handler finds the same names. |
| 83 | + */ |
| 84 | + private function bootRenderHarness(): Smarty |
| 85 | + { |
| 86 | + $theme = $this->makeStubTheme(); |
| 87 | + $GLOBALS['theme'] = $theme; |
| 88 | + $GLOBALS['userbank'] = $GLOBALS['userbank'] ?? new \CUserManager(null); |
| 89 | + $GLOBALS['username'] = $GLOBALS['username'] ?? 'tester'; |
| 90 | + |
| 91 | + return $theme; |
| 92 | + } |
| 93 | + |
| 94 | + /** |
| 95 | + * Standalone contract pin: under the panel's production PDO |
| 96 | + * options (`EMULATE_PREPARES => false`, set in |
| 97 | + * `Sbpp\Db\Database::__construct`), reusing the same `:name` |
| 98 | + * placeholder more than once with a SINGLE `bind()` call leaves |
| 99 | + * the second slot unbound and `PDOStatement::execute()` raises |
| 100 | + * `SQLSTATE[HY093] Invalid parameter number`. |
| 101 | + * |
| 102 | + * Constructing a fresh `Database` (rather than reaching for |
| 103 | + * `$GLOBALS['PDO']`) keeps the test self-contained — and proves |
| 104 | + * the assertion is about `Database`'s configuration (the |
| 105 | + * `EMULATE_PREPARES => false` option in the constructor), not |
| 106 | + * about whatever stale state another test might have left on |
| 107 | + * the global wrapper. |
| 108 | + * |
| 109 | + * The `:prefix_admins` table is one of the tables `Fixture` seeds |
| 110 | + * by default (the admin row from `seedAdmin`), so the SELECT |
| 111 | + * itself has something to query — the assertion is purely on |
| 112 | + * the parameter-binding mismatch, not on row presence. |
| 113 | + */ |
| 114 | + public function testReusedNamedPlaceholderUnderNativePreparesIsRejected(): void |
| 115 | + { |
| 116 | + $db = new Database(DB_HOST, DB_PORT, DB_NAME, DB_USER, DB_PASS, DB_PREFIX, DB_CHARSET); |
| 117 | + |
| 118 | + // Two `:sid` occurrences, one `bind()` call. This is the |
| 119 | + // exact failure mode `admin.srvadmins.php` exhibited before |
| 120 | + // the fix. |
| 121 | + $db->query('SELECT 1 FROM `:prefix_admins` WHERE aid = :sid OR aid = :sid LIMIT 1'); |
| 122 | + $db->bind(':sid', 1); |
| 123 | + |
| 124 | + $this->expectException(PDOException::class); |
| 125 | + $this->expectExceptionMessageMatches('/HY093|Invalid parameter number/i'); |
| 126 | + |
| 127 | + $db->resultset(); |
| 128 | + } |
| 129 | + |
| 130 | + /** |
| 131 | + * The headline regression: hitting `?p=admin&c=servers&o=admincheck&id=<sid>` |
| 132 | + * pre-fix raised `PDOException` from the SELECT in |
| 133 | + * `admin.srvadmins.php` (line 38 in the issue trace). The test |
| 134 | + * requires the page file with `$_GET['id']` set so the same |
| 135 | + * code path runs, and asserts no exception escapes. |
| 136 | + * |
| 137 | + * Process-isolation matches `Php82DeprecationsTest`: the page |
| 138 | + * handler defines top-level helpers / variables PHP can't |
| 139 | + * redeclare in a single process across multiple test cases, and |
| 140 | + * we want a clean `$_GET` / `$_SESSION` / `$GLOBALS` per case. |
| 141 | + * |
| 142 | + * `$_GET['id']` is set to `0` deliberately — the seeded test DB |
| 143 | + * has no `:prefix_admins_servers_groups` rows, so any server |
| 144 | + * id (real or sentinel `0`) returns an empty result set. The |
| 145 | + * regression is on the SELECT's PREPARE + EXECUTE step, which |
| 146 | + * runs whether the result set is empty or not. Setting `0` |
| 147 | + * also avoids needing to seed a server row just for the test. |
| 148 | + * |
| 149 | + * `checkMultiplePlayers($sid, ...)` is gated on |
| 150 | + * `count($admsteam) > 0`; with an empty result set the call is |
| 151 | + * skipped, so we don't risk a real RCON socket open against an |
| 152 | + * arbitrary IP. |
| 153 | + */ |
| 154 | + #[RunInSeparateProcess] |
| 155 | + #[PreserveGlobalState(false)] |
| 156 | + public function testAdminSrvadminsPageRendersWithoutPdoException(): void |
| 157 | + { |
| 158 | + $this->loginAsAdmin(); |
| 159 | + $this->bootRenderHarness(); |
| 160 | + |
| 161 | + $_SESSION = []; |
| 162 | + $_GET = ['p' => 'admin', 'c' => 'servers', 'o' => 'admincheck', 'id' => '0']; |
| 163 | + |
| 164 | + ob_start(); |
| 165 | + try { |
| 166 | + require ROOT . 'pages/admin.srvadmins.php'; |
| 167 | + } finally { |
| 168 | + ob_end_clean(); |
| 169 | + } |
| 170 | + |
| 171 | + $this->assertTrue(true, |
| 172 | + 'admin.srvadmins.php must prepare + execute its admin-list SELECT ' |
| 173 | + . 'without raising PDOException HY093 (issue #1314).'); |
| 174 | + } |
| 175 | +} |
0 commit comments