Fix #1311: cache + debounce A2S queries on the public servers page

AGENTS.md

@@ -312,6 +312,7 @@ matching its directory. PSR-4 autoloads from `web/includes/` →
 | `Sbpp\Api\ApiError`                      | structured API error                  |
 | `Sbpp\View\AdminTabs`                    | admin sub-route sidebar mounter       |
 | `Sbpp\View\*`                            | view DTOs (page-level + partials)     |
+| `Sbpp\Servers\SourceQueryCache`          | per-`(ip, port)` on-disk cache around the xPaw A2S probe (#1311) |
 | `Sbpp\Markup\IntroRenderer`              | admin-authored Markdown renderer      |
 | `Sbpp\Mail\Mail` / `Sbpp\Mail\Mailer` / `Sbpp\Mail\EmailType` | `Mail::send(...)` entry point + Symfony Mailer SMTP wrapper + email-type enum |
 | `Sbpp\Theme`                             | theme registry + per-theme behavior gates (e.g. `wantsLegacyAdminCounts()`) |
@@ -1261,6 +1262,7 @@ audit (#1207) locked in. New CTAs:
 | Edit the player-detail drawer (open trigger, tabs, panes, lazy loaders) | `web/themes/default/js/theme.js` (`renderDrawerBody` / `loadPaneIfNeeded`) |
 | Edit the command palette (icon-only trigger, ⌘K binding, result rows, kbd hints, Ctrl+Enter copy) | `web/themes/default/js/theme.js` (`openPalette` / `closePalette` / `renderPaletteResults` / `applyPlatformHints` / `handlePaletteCopyShortcut`) + `core/title.tpl` (the `.topbar__search` icon button) + the `.palette__row*` rules in `web/themes/default/css/theme.css`. Player rows carry `data-drawer-bid="<bid>"` (bare Enter / click → `loadDrawer`, palette closes itself) + `data-steamid="<steam>"` (`Ctrl/Cmd+Enter` → `navigator.clipboard.writeText` + `showToast`). The kbd glyphs are server-rendered in non-Mac form (`Enter`, `Ctrl`); `applyPlatformHints` swaps `[data-enterkey]` → ⏎ and `[data-modkey]` → ⌘ on Mac/iOS at boot and after every render (#1184, #1207 DET-2). |
 | Add admin-only per-player notes | `web/api/handlers/notes.php` (CRUD) — Notes tab is gated by `bans.detail`'s `notes_visible` flag |
+| Cache an A2S `GetInfo + GetPlayers` round-trip / add another public server-query handler | `web/includes/Servers/SourceQueryCache.php` (`Sbpp\Servers\SourceQueryCache::fetch($ip, $port, $ttl=30)` — per-`(ip, port)` on-disk cache under `SB_CACHE/srvquery/`, atomic tempfile + `rename()` writes mirroring `system.check_version`'s release cache; both success and failure cache so an unreachable server costs ONE A2S probe per ~30s window). Every public handler under `web/api/handlers/servers.php` (`api_servers_host_players` / `host_property` / `host_players_list` / `players`) goes through this — never call `new SourceQuery()` directly from a handler. The cache stamps user-agnostic data only; the handler stamps per-caller fields (`is_owner`, `can_ban`, the per-call `trunchostname`) on top. Per-tile JS debounce on the public servers page lives in `web/themes/default/page_servers.tpl` (`loadTile()` flips `tile.__sbppLoading` + the Re-query button's `disabled` attr while a probe is in flight, releases both in the success / error tails). The matching JS gate on the toggle button has been the precedent since v2.0.0; #1311 brought the refresh button onto the same shape. Tests: `web/tests/integration/SourceQueryCacheTest.php` (cache shape + coalescing + TTL + invalidation, drives `setProbeOverrideForTesting()` so the assertion is deterministic without UDP) + `testHostPlayersCoalescesRapidRepeatCallsViaCache` / `testHostPlayersNegativeCachesUnreachableServers` in `web/tests/api/ServersTest.php` (handler-shape coverage). E2E: `web/tests/e2e/specs/flows/server-refresh-debounce.spec.ts`. |
 | Render admin-authored Markdown to safe HTML | `web/includes/Markup/IntroRenderer.php` (`Sbpp\Markup`) |
 | Build / extend the anonymous opt-out daily telemetry payload (#1126) | `web/includes/Telemetry/Telemetry.php` (`Sbpp\Telemetry\Telemetry` — `tickIfDue`, `collect`, `send`) + `web/includes/Telemetry/Schema1.php` (`Sbpp\Telemetry\Schema1::payloadFieldNames()`, drives the parity tests) + `web/includes/Telemetry/schema-1.lock.json` (vendored from [sbpp/cf-analytics](https://github.com/sbpp/cf-analytics) — manual sync via `make sync-telemetry-schema`). Tick is registered at the tail of `init.php` via `register_shutdown_function`; on FPM, `fastcgi_finish_request()` flushes the response BEFORE the cURL POST so telemetry never delays a panel page. Slot reservation is atomic (`UPDATE :prefix_settings WHERE CAST(value AS UNSIGNED) <= :threshold`) at the START of the attempt, so a flapping endpoint costs one ping/day, not one ping/request. Audit-log only enable/disable transitions, never individual pings. Help-icon copy in `page_admin_settings_features.tpl` + `README.md`'s `## Privacy & telemetry` section + `UPGRADING.md` are the in-panel + upgrade-time disclosure surfaces (no first-login modal). |
 | Add a cross-repo JSON contract (vendored schema lock + reader + parity tests) | `web/includes/Telemetry/Schema1.php` is the reference shape (`payloadFieldNames(): list<string>` over a Draft-7 JSON Schema lock file). Pair with two PHPUnit tests: an extractor parity test (collect() vs. lock file in both directions) and a doc parity test (README `<!-- …-START -->` / `<!-- …-END -->` block vs. lock file). Sync via a manual `make sync-<subsystem>-schema` target — no scheduled auto-PR. See "Cross-repo JSON contracts" under Conventions. |

ARCHITECTURE.md

@@ -94,6 +94,7 @@ web/
 │   ├── View/AdminTabs.php    Sbpp\View\AdminTabs — Pattern A admin sub-section nav
 │   ├── View/                 Sbpp\View\* — typed Smarty view-model DTOs
 │   ├── Markup/               Sbpp\Markup\IntroRenderer — admin Markdown -> safe HTML
+│   ├── Servers/              Sbpp\Servers\SourceQueryCache — per-(ip, port) cache around the xPaw A2S probe (#1311)
 │   ├── Mail/                 Sbpp\Mail\{Mail,Mailer,EmailType} — Symfony Mailer wrapper + enum
 │   ├── Telemetry/            Sbpp\Telemetry\{Telemetry,Schema1} — anonymous opt-out daily ping (#1126); schema-1.lock.json is the vendored cross-repo contract
 │   ├── SteamID/              SteamID parsing / vanity-URL resolution
@@ -599,6 +600,41 @@ companion changes:
   bundle is no longer referenced and its directory is a follow-up
   cleanup.
 
+### Server query cache (`includes/Servers/`)
+
+`Sbpp\Servers\SourceQueryCache::fetch($ip, $port, $ttl=30)` is a thin
+on-disk cache around `xPaw\SourceQuery\SourceQuery` — the UDP A2S
+probe used by the public servers page (`?p=servers`) and any
+third-party theme that still emits the legacy `__sbppLoadServerHost`
+helper from `page.servers.php`. Returns either the cached
+`{info, players}` payload or `null` when the underlying probe failed;
+both states are persisted under `SB_CACHE/srvquery/<sha1(ip:port)>.json`
+so an unreachable server costs ONE A2S probe per ~30s window instead
+of one per request.
+
+Keyed by `(ip, port)` rather than by `sid` — multiple
+`:prefix_servers` rows pointing at the same game server share a slot,
+and the cache stays user-agnostic. Per-caller fields (`is_owner`,
+`can_ban`, the per-call `trunchostname` truncation) are stamped on
+top by the handler after the fetch returns. Atomic writes use the
+same tempfile + `rename()` shape as `_api_system_release_save_cache`
+in `system.php` so two concurrent FPM workers never read a
+half-written entry.
+
+Issue #1311 is the audit that introduced this: every public servers
+page hit, plus every per-tile Re-query button click (anonymous-callable
+through `servers.host_players` / `servers.host_property` /
+`servers.host_players_list` / `servers.players`, `public=true` in
+`_register.php`), used to fan out one fresh `xPaw\SourceQuery::Connect`
++ `GetInfo` + `GetPlayers` UDP round-trip per configured server. A
+hand-mash of the refresh button or `for i in $(seq 1 100); do curl
+?p=servers; done` translated 1:1 to A2S queries leaving the panel
+host. The handlers now go through `SourceQueryCache::fetch(...)` so
+back-to-back panel hits coalesce at the cache boundary; the matching
+client-side debounce on the public servers page (`page_servers.tpl`'s
+`loadTile()` flips `tile.__sbppLoading` + the Re-query button's
+`disabled` attr while a probe is in flight) closes the UX vector.
+
 ### Logging (`includes/Log.php`)
 
 `Log::add('m', 'Topic', 'Detail')` writes a row to `sb_log` with the

web/api/handlers/_register.php

@@ -122,6 +122,20 @@
 Api::register('servers.add',                 'api_servers_add',                 ADMIN_OWNER | ADMIN_ADD_SERVER);
 Api::register('servers.remove',              'api_servers_remove',              ADMIN_OWNER | ADMIN_DELETE_SERVERS);
 Api::register('servers.setup_edit',          'api_servers_setup_edit',          ADMIN_OWNER | ADMIN_EDIT_SERVERS);
+// servers.refresh + the server-query family below mirror the public
+// servers page's reach: every visitor of `?p=servers` (and any
+// third-party theme that still emits the legacy `__sbppLoadServerHost`
+// helper from `page.servers.php`) needs the live A2S `GetInfo` /
+// `GetPlayers` data, so the actions are anonymous-callable. The A2S
+// amplification this used to enable (#1311) is contained inside the
+// handlers via `Sbpp\Servers\SourceQueryCache` (per-`(ip, port)`
+// on-disk cache, ~30s window, negative caching for unreachable
+// servers) — without that cache, a hand-mash of the per-tile Re-query
+// button or `for i in $(seq 1 100); do curl ?p=servers; done` would
+// translate 1:1 to UDP queries leaving the panel host. Don't widen
+// the perm gate or drop the public reach without re-evaluating the
+// cache contract; don't drop the cache without re-introducing some
+// other rate limit at the dispatcher.
 Api::register('servers.refresh',             'api_servers_refresh',             0, false, true);
 Api::register('servers.host_players',        'api_servers_host_players',        0, false, true);
 Api::register('servers.host_property',       'api_servers_host_property',       0, false, true);

web/api/handlers/servers.php

@@ -11,7 +11,7 @@
 work.  If not, see <http://creativecommons.org/licenses/by-nc-sa/3.0/>.
 *************************************************************************/
 
-use xPaw\SourceQuery\SourceQuery;
+use Sbpp\Servers\SourceQueryCache;
 
 /**
  * Mirrors the access loop in web/pages/admin.rcon.php so the API enforces
@@ -183,64 +183,63 @@ function api_servers_refresh(array $params): array
 /**
  * Returns server info + player list for the requested server. Pure data —
  * the client decides how to render it.
+ *
+ * The A2S `GetInfo + GetPlayers` round-trip is delegated to
+ * `Sbpp\Servers\SourceQueryCache` so back-to-back calls (a hand-mashed
+ * Re-query button, a `for` loop hitting `?p=servers`) coalesce into ONE
+ * UDP probe per `(ip, port)` per ~30s window. The handler still maps
+ * the cached payload through the per-caller permission check
+ * (`is_owner` / `can_ban`) and the per-call hostname truncation, so the
+ * cache stays user-agnostic. See #1311 for the threat model.
  */
 function api_servers_host_players(array $params): array
 {
     global $userbank;
     $sid           = (int)($params['sid'] ?? 0);
     $trunchostname = (int)($params['trunchostname'] ?? 48);
 
-    $GLOBALS['PDO']->query("SELECT ip, port FROM `:prefix_servers` WHERE sid = :sid");
-    $GLOBALS['PDO']->bind(':sid', $sid, PDO::PARAM_INT);
-    $server = $GLOBALS['PDO']->single();
+    $server = _api_servers_lookup_address($sid);
 
-    if (empty($server['ip']) || empty($server['port'])) {
-        throw new ApiError('not_found', 'Server not found');
-    }
-
-    $query = new SourceQuery();
-    try {
-        $query->Connect($server['ip'], $server['port'], 1, SourceQuery::SOURCE);
-        $info = $query->GetInfo();
-        $info['HostName'] = preg_replace('/[\x00-\x1f]/', '', htmlspecialchars($info['HostName']));
-        $players = $query->GetPlayers();
-    } catch (\Throwable $e) {
+    $cached = SourceQueryCache::fetch((string) $server['ip'], (int) $server['port']);
+    if ($cached === null) {
         return [
-            'sid'     => $sid,
-            'ip'      => $server['ip'],
-            'port'    => $server['port'],
-            'error'   => 'connect',
+            'sid'      => $sid,
+            'ip'       => $server['ip'],
+            'port'     => $server['port'],
+            'error'    => 'connect',
             'is_owner' => $userbank->HasAccess(WebPermission::Owner),
         ];
-    } finally {
-        $query->Disconnect();
     }
 
-    $os = match ($info['Os']) {
+    $info    = $cached['info'];
+    $players = $cached['players'];
+    $info['HostName'] = (string) preg_replace('/[\x00-\x1f]/', '', htmlspecialchars((string) ($info['HostName'] ?? '')));
+
+    $os = match ($info['Os'] ?? '') {
         'w'     => 'fab fa-windows',
         'l'     => 'fab fa-linux',
         default => 'fas fa-server',
     };
 
     return [
-        'sid'      => $sid,
-        'ip'       => $server['ip'],
-        'port'     => $server['port'],
-        'hostname' => trunc($info['HostName'], $trunchostname),
-        'players'  => (int)$info['Players'],
-        'maxplayers' => (int)$info['MaxPlayers'],
-        'map'      => basename($info['Map']),
-        'mapfull'  => $info['Map'],
-        'mapimg'   => GetMapImage($info['Map']),
-        'os_class' => $os,
-        'secure'   => (bool)$info['Secure'],
+        'sid'        => $sid,
+        'ip'         => $server['ip'],
+        'port'       => $server['port'],
+        'hostname'   => trunc($info['HostName'], $trunchostname),
+        'players'    => (int) ($info['Players']    ?? 0),
+        'maxplayers' => (int) ($info['MaxPlayers'] ?? 0),
+        'map'        => basename((string) ($info['Map'] ?? '')),
+        'mapfull'    => (string) ($info['Map'] ?? ''),
+        'mapimg'     => GetMapImage((string) ($info['Map'] ?? '')),
+        'os_class'   => $os,
+        'secure'     => (bool) ($info['Secure'] ?? false),
         'player_list' => array_map(fn($p) => [
-            'id'     => $p['Id'],
-            'name'   => $p['Name'],
-            'frags'  => (int)$p['Frags'],
-            'time'   => $p['Time'],
-            'time_f' => $p['TimeF'],
-        ], $players ?: []),
+            'id'     => $p['Id']    ?? null,
+            'name'   => $p['Name']  ?? '',
+            'frags'  => (int) ($p['Frags'] ?? 0),
+            'time'   => $p['Time']  ?? 0,
+            'time_f' => $p['TimeF'] ?? '',
+        ], $players),
         'can_ban' => $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::AddBan)),
     ];
 }
@@ -250,25 +249,19 @@ function api_servers_host_property(array $params): array
     $sid           = (int)($params['sid'] ?? 0);
     $trunchostname = (int)($params['trunchostname'] ?? 48);
 
-    $GLOBALS['PDO']->query("SELECT ip, port FROM `:prefix_servers` WHERE sid = :sid");
-    $GLOBALS['PDO']->bind(':sid', $sid, PDO::PARAM_INT);
-    $server = $GLOBALS['PDO']->single();
-    if (empty($server['ip']) || empty($server['port'])) {
-        throw new ApiError('not_found', 'Server not found');
-    }
+    $server = _api_servers_lookup_address($sid);
 
-    $query = new SourceQuery();
-    try {
-        $query->Connect($server['ip'], $server['port'], 1, SourceQuery::SOURCE);
-        $info = $query->GetInfo();
-        $info['HostName'] = preg_replace('/[\x00-\x1f]/', '', htmlspecialchars($info['HostName']));
-    } catch (\Throwable $e) {
+    $cached = SourceQueryCache::fetch((string) $server['ip'], (int) $server['port']);
+    if ($cached === null) {
         return ['ip' => $server['ip'], 'port' => $server['port'], 'error' => 'connect'];
-    } finally {
-        $query->Disconnect();
     }
 
-    return ['hostname' => trunc($info['HostName'] ?: '', $trunchostname), 'ip' => $server['ip'], 'port' => $server['port']];
+    $hostname = (string) preg_replace('/[\x00-\x1f]/', '', htmlspecialchars((string) ($cached['info']['HostName'] ?? '')));
+    return [
+        'hostname' => trunc($hostname, $trunchostname),
+        'ip'       => $server['ip'],
+        'port'     => $server['port'],
+    ];
 }
 
 function api_servers_host_players_list(array $params): array
@@ -287,17 +280,13 @@ function api_servers_host_players_list(array $params): array
         if (empty($server['ip']) || empty($server['port'])) {
             continue;
         }
-        $query = new SourceQuery();
-        try {
-            $query->Connect($server['ip'], $server['port'], 1, SourceQuery::SOURCE);
-            $info = $query->GetInfo();
-            $info['HostName'] = preg_replace('/[\x00-\x1f]/', '', htmlspecialchars($info['HostName']));
-            $lines[] = trunc($info['HostName'] ?: '', 48);
-        } catch (\Throwable $e) {
+        $cached = SourceQueryCache::fetch((string) $server['ip'], (int) $server['port']);
+        if ($cached === null) {
             $lines[] = "ERROR " . $server['ip'] . ':' . $server['port'];
-        } finally {
-            $query->Disconnect();
+            continue;
         }
+        $hostname = (string) preg_replace('/[\x00-\x1f]/', '', htmlspecialchars((string) ($cached['info']['HostName'] ?? '')));
+        $lines[]  = trunc($hostname, 48);
     }
     return ['lines' => $lines];
 }
@@ -313,26 +302,42 @@ function api_servers_players(array $params): array
         return ['sid' => $sid, 'players' => []];
     }
 
-    $query = new SourceQuery();
-    try {
-        $query->Connect($server['ip'], $server['port'], 1, SourceQuery::SOURCE);
-        $players = $query->GetPlayers();
-    } catch (\Throwable $e) {
+    $cached = SourceQueryCache::fetch((string) $server['ip'], (int) $server['port']);
+    if ($cached === null) {
         return ['sid' => $sid, 'players' => []];
-    } finally {
-        $query->Disconnect();
     }
 
     return [
         'sid' => $sid,
         'players' => array_map(fn($p) => [
-            'name'  => $p['Name'],
-            'frags' => (int)$p['Frags'],
-            'time'  => $p['Time'],
-        ], $players ?: []),
+            'name'  => $p['Name']  ?? '',
+            'frags' => (int) ($p['Frags'] ?? 0),
+            'time'  => $p['Time']  ?? 0,
+        ], $cached['players']),
     ];
 }
 
+/**
+ * Lookup the `(ip, port)` for a `sid`, throwing the same `not_found`
+ * envelope every public server-query handler used to throw inline.
+ * Extracted so the cache-fronted handlers don't repeat the SELECT and
+ * the empty-row guard.
+ *
+ * @return array{ip: string, port: int}
+ */
+function _api_servers_lookup_address(int $sid): array
+{
+    $GLOBALS['PDO']->query("SELECT ip, port FROM `:prefix_servers` WHERE sid = :sid");
+    $GLOBALS['PDO']->bind(':sid', $sid, PDO::PARAM_INT);
+    $server = $GLOBALS['PDO']->single();
+
+    if (empty($server['ip']) || empty($server['port'])) {
+        throw new ApiError('not_found', 'Server not found');
+    }
+
+    return ['ip' => (string) $server['ip'], 'port' => (int) $server['port']];
+}
+
 function api_servers_send_rcon(array $params): array
 {
     global $userbank;