Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,21 @@ top-level `class Foo {}` in `web/includes/` (see "Anti-patterns").
`Database::query()` rewrites the placeholder. Never inline the prefix.
- Pattern: `query` → `bind` → `execute` / `single` / `resultset`.
- ADOdb was fully removed (commit `b9c812b2`). **Do not reintroduce it.**
- Row-derived `IN (...)` lists go through
`Database::resultsetInList()` / `executeInList()`. Both methods split
values into 10,000-item statements so native MySQL / MariaDB prepares
stay below the 65,535-placeholder ceiling. Do not build an unbounded
placeholder string with `array_fill(count($rows), '?')`: an install
with 75,000 bans fails during `PDO::prepare()` with error 1390 before
any values are bound. Compile-time constant lists (for example the
export subsystem's short forbidden-setting-key list) may stay inline.
Chunked SELECT ordering is only per statement; regroup returned rows
by key instead of relying on one globally ordered result. The helper
accepts only list-shaped `PDO::FETCH_ASSOC` / `PDO::FETCH_COLUMN`
results; keyed modes cannot be merged safely across chunks. Pass
`atomic: true` to `executeInList()` when splitting a formerly single
write must preserve all-or-nothing behavior; the helper owns the
transaction in that mode, so callers must not open a nested one.
- Each named placeholder (`:name`) inside one query needs as many
`bind()` calls as occurrences. The panel runs PDO with
`PDO::ATTR_EMULATE_PREPARES => false` (`Sbpp\Db\Database::__construct`
Expand Down
144 changes: 144 additions & 0 deletions web/includes/Db/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@
*/
final class Database
{
public const MAX_PREPARED_STATEMENT_PLACEHOLDERS = 65_535;

/**
* Keep generated IN-list statements comfortably below MySQL /
* MariaDB's 65,535-placeholder prepared-statement ceiling. Fixed
* parameters that appear before or after the list also count toward
* that ceiling, so callers must not build the list themselves.
*/
public const IN_LIST_CHUNK_SIZE = 10_000;

private readonly string $prefix;

private PDO $dbh;
Expand Down Expand Up @@ -160,6 +170,140 @@ public function single(?array $inputParams = null, int $fetchType = PDO::FETCH_A
return $this->stmt->fetch($fetchType);
}

/**
* Execute a SELECT once per bounded slice of an IN-list and merge
* the rows. Ordering is guaranteed within each slice only; callers
* should consume the result as a set or regroup it by key.
*
* `$sqlBeforeValues` must end immediately before the first generated
* placeholder and `$sqlAfterValues` must begin immediately after the
* last one. For example:
*
* $db->resultsetInList(
* 'SELECT aid, user FROM `:prefix_admins` WHERE aid IN (',
* $aids,
* ')',
* );
*
* @param list<int|bool|null|string> $values
* @param list<int|bool|null|string> $paramsBefore
* @param list<int|bool|null|string> $paramsAfter
* @return list<mixed>
* @throws \InvalidArgumentException When a keyed PDO fetch mode is requested.
*/
public function resultsetInList(
string $sqlBeforeValues,
array $values,
string $sqlAfterValues = '',
array $paramsBefore = [],
array $paramsAfter = [],
int $fetchType = PDO::FETCH_ASSOC,
): array {
if (!in_array($fetchType, [PDO::FETCH_ASSOC, PDO::FETCH_COLUMN], true)) {
throw new \InvalidArgumentException(
'Chunked IN-list SELECTs support only PDO::FETCH_ASSOC and PDO::FETCH_COLUMN.'
);
}

$rows = [];
foreach ($this->inListChunks($values, count($paramsBefore) + count($paramsAfter)) as $chunk) {
$placeholders = implode(',', array_fill(0, count($chunk), '?'));
$chunkRows = $this
->query($sqlBeforeValues . $placeholders . $sqlAfterValues)
->resultset([...$paramsBefore, ...$chunk, ...$paramsAfter], $fetchType);
array_push($rows, ...$chunkRows);
}

return $rows;
}

/**
* Execute a write once per bounded slice of an IN-list.
*
* Pass `$atomic = true` when every chunk must commit or roll back as
* one operation. The caller must not already have a transaction open
* in that mode because PDO does not support nested transactions.
*
* @param list<int|bool|null|string> $values
* @param list<int|bool|null|string> $paramsBefore
* @param list<int|bool|null|string> $paramsAfter
*/
public function executeInList(
string $sqlBeforeValues,
array $values,
string $sqlAfterValues = '',
array $paramsBefore = [],
array $paramsAfter = [],
bool $atomic = false,
): int {
$chunks = $this->inListChunks($values, count($paramsBefore) + count($paramsAfter));
if ($chunks === []) {
return 0;
}

$affected = 0;
$transactionOpen = false;
if ($atomic) {
if ($this->dbh->inTransaction()) {
throw new \LogicException('Atomic IN-list execution cannot start inside an existing transaction.');
}
$this->beginTransaction();
$transactionOpen = true;
}
try {
foreach ($chunks as $chunk) {
$placeholders = implode(',', array_fill(0, count($chunk), '?'));
$this
->query($sqlBeforeValues . $placeholders . $sqlAfterValues)
->execute([...$paramsBefore, ...$chunk, ...$paramsAfter]);
$affected += $this->rowCount();
}
if ($atomic) {
$this->endTransaction();
$transactionOpen = false;
}
} catch (\Throwable $e) {
if ($transactionOpen) {
$this->cancelTransaction();
}
throw $e;
}

return $affected;
}

/**
* @param list<int|bool|null|string> $values
* @return list<list<int|bool|null|string>>
*/
private function inListChunks(array $values, int $reservedPlaceholders): array
{
if ($values === []) {
return [];
}

$available = self::MAX_PREPARED_STATEMENT_PLACEHOLDERS - $reservedPlaceholders;
if ($available < 1) {
throw new \InvalidArgumentException('IN-list query has no placeholder capacity left after fixed parameters.');
}

$unique = [];
$seen = [];
foreach ($values as $value) {
// MariaDB compares 5 and '5' as equal, so dedupe on the same
// terms: otherwise both could land in different chunks and
// return the same row twice.
$key = serialize(is_int($value) ? (string) $value : $value);
if (isset($seen[$key])) {
continue;
}
$seen[$key] = true;
$unique[] = $value;
}

return array_chunk($unique, min(self::IN_LIST_CHUNK_SIZE, $available));
}

/**
* Yields rows one at a time so callers can stream large result sets
* without materialising the full set in PHP memory like resultset() does.
Expand Down
75 changes: 40 additions & 35 deletions web/includes/system-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -323,47 +323,52 @@ function PruneBans(): void
$pdo->bind(':id', $adminId);
$pdo->execute();

// Two single-column SELECTs are intentionally separate from the
// composite UPDATE below: `UPDATE … WHERE` locks every row it
// examines for the predicate, not just the rows it changes. We
// surface the candidate `subid`s with a SELECT first so the
// UPDATE only locks rows it'll mutate.
$steamIds = $pdo
->query('SELECT DISTINCT authid FROM `:prefix_bans` WHERE `type` = 0 AND `RemoveType` IS NULL')
->resultset(null, PDO::FETCH_COLUMN);
$banIps = $pdo
->query('SELECT ip FROM `:prefix_bans` WHERE type = 1 AND RemoveType IS NULL')
->resultset(null, PDO::FETCH_COLUMN);

if ($steamIds === [] && $banIps === []) {
return;
}

$clauses = [];
$args = [];
if ($steamIds !== []) {
$clauses[] = 'SteamId IN (' . implode(',', array_fill(0, count($steamIds), '?')) . ')';
array_push($args, ...$steamIds);
}
if ($banIps !== []) {
$clauses[] = 'sip IN (' . implode(',', array_fill(0, count($banIps), '?')) . ')';
array_push($args, ...$banIps);
}

// Keep the candidate lookup read-only: a composite UPDATE would
// lock every submissions row examined, not just rows it changes.
// Two set-based arms avoid materialising every active ban identifier
// as one prepared-statement IN-list (MariaDB rejects statements above
// 65,535 placeholders). UNION DISTINCT de-duplicates a submission
// that happens to match both its Steam ID and IP. Keeping the arms
// separate lets MariaDB probe type_authid / type_ip directly for each
// submission instead of materialising all active identifiers first.
// No FORCE INDEX: the (type, authid) / (type, ip) equality join picks
// those indexes on its own, and a hint would turn an install missing
// either index (e.g. a half-applied updater 702) into error 1176 on
// every banlist render, ban add/edit and GET /api/v1/bans.
$subIds = $pdo
->query('SELECT `subid` FROM `:prefix_submissions` WHERE `archiv` = 0 AND (' . implode(' OR ', $clauses) . ')')
->resultset($args, PDO::FETCH_COLUMN);
->query(
'SELECT S.`subid`
FROM `:prefix_submissions` AS S
INNER JOIN `:prefix_bans` AS BSteam
ON BSteam.`type` = 0
AND BSteam.`authid` = S.`SteamId`
AND BSteam.`RemoveType` IS NULL
WHERE S.`archiv` = 0
UNION DISTINCT
SELECT S.`subid`
FROM `:prefix_submissions` AS S
INNER JOIN `:prefix_bans` AS BIp
ON BIp.`type` = 1
AND BIp.`ip` = S.`sip`
AND BIp.`RemoveType` IS NULL
WHERE S.`archiv` = 0'
)
->resultset(null, PDO::FETCH_COLUMN);

if ($subIds === []) {
return;
}

$pdo
->query('UPDATE `:prefix_submissions`
SET `archiv` = 3,
`archivedby` = ?
WHERE `subid` IN (' . implode(',', array_fill(0, count($subIds), '?')) . ')')
->execute([$adminId, ...$subIds]);
$pdo->executeInList(
'UPDATE `:prefix_submissions`
SET `archiv` = 3,
`archivedby` = ?
WHERE `subid` IN (',
$subIds,
')',
[$adminId],
atomic: true,
);
}

/**
Expand Down
Loading
Loading