Skip to content

Commit 647dffa

Browse files
lohanidamodarclaude
andcommitted
fix: harden cursor pagination against null values and stale keys
Three small follow-ups based on review feedback on the audit PR's twin implementation: - Drop the always-true `!empty($orderAttributes)` guard inside the cursor branch. resolveCursorOrder() always appends an `id` tiebreaker, so the guard is dead code and was misleading. - normalizeCursorRow now removes `$id` after copying it to `id`, so cursor state is no longer carrying both keys. - Throw an explicit Exception when a cursor value is null. The previous path silently routed null `time` cursors through formatDateTime(null) which returns the current timestamp — a misconfigured cursor would filter on `time < now()` and produce wrong pages instead of failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4298db5 commit 647dffa

1 file changed

Lines changed: 14 additions & 5 deletions

File tree

src/Usage/Adapter/ClickHouse.php

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,7 +1523,9 @@ private function findFromTable(array $queries, string $type): array
15231523
$params = $whereData['params'];
15241524

15251525
$orderClause = '';
1526-
if (isset($parsed['cursor']) && !empty($orderAttributes)) {
1526+
if (isset($parsed['cursor'])) {
1527+
// $orderAttributes is always non-empty here — resolveCursorOrder
1528+
// appends an `id` tiebreaker when no order is specified.
15271529
$orderSql = $this->buildOrderBySql($orderAttributes, flip: $cursorDirection === 'before');
15281530
$orderClause = ' ORDER BY ' . implode(', ', $orderSql);
15291531
} elseif (!empty($parsed['orderBy'])) {
@@ -2446,6 +2448,7 @@ private function normalizeCursorRow(mixed $rawCursor): array
24462448

24472449
if (!array_key_exists('id', $row) && array_key_exists('$id', $row)) {
24482450
$row['id'] = $row['$id'];
2451+
unset($row['$id']);
24492452
}
24502453

24512454
return $row;
@@ -2524,36 +2527,42 @@ private function buildCursorWhere(array $orderAttributes, array $cursor, string
25242527
throw new \Exception("Cursor is missing required attribute '{$prevAttr}'");
25252528
}
25262529
$prevValue = $cursor[$prevAttr];
2530+
if ($prevValue === null) {
2531+
throw new \Exception("Cursor value for '{$prevAttr}' cannot be null");
2532+
}
25272533
$prevEscaped = $this->escapeIdentifier($prevAttr);
25282534
$prevType = $this->getParamType($prevAttr);
25292535
$paramName = "cursor_eq_{$i}_{$j}";
25302536

25312537
if ($prevAttr === 'time') {
2532-
/** @var \DateTime|string|null $timeValue */
2538+
/** @var \DateTime|string $timeValue */
25332539
$timeValue = $prevValue;
25342540
$conditions[] = "{$prevEscaped} = {{$paramName}:DateTime64(3)}";
25352541
$params[$paramName] = $this->formatDateTime($timeValue);
25362542
} else {
2537-
/** @var bool|float|int|string|null $scalarValue */
2543+
/** @var bool|float|int|string $scalarValue */
25382544
$scalarValue = $prevValue;
25392545
$conditions[] = "{$prevEscaped} = {{$paramName}:{$prevType}}";
25402546
$params[$paramName] = $this->formatParamValue($scalarValue);
25412547
}
25422548
}
25432549

25442550
$value = $cursor[$attr];
2551+
if ($value === null) {
2552+
throw new \Exception("Cursor value for '{$attr}' cannot be null");
2553+
}
25452554
$escaped = $this->escapeIdentifier($attr);
25462555
$chType = $this->getParamType($attr);
25472556
$operator = $direction === 'DESC' ? '<' : '>';
25482557
$paramName = "cursor_cmp_{$i}";
25492558

25502559
if ($attr === 'time') {
2551-
/** @var \DateTime|string|null $timeValue */
2560+
/** @var \DateTime|string $timeValue */
25522561
$timeValue = $value;
25532562
$conditions[] = "{$escaped} {$operator} {{$paramName}:DateTime64(3)}";
25542563
$params[$paramName] = $this->formatDateTime($timeValue);
25552564
} else {
2556-
/** @var bool|float|int|string|null $scalarValue */
2565+
/** @var bool|float|int|string $scalarValue */
25572566
$scalarValue = $value;
25582567
$conditions[] = "{$escaped} {$operator} {{$paramName}:{$chType}}";
25592568
$params[$paramName] = $this->formatParamValue($scalarValue);

0 commit comments

Comments
 (0)