fix(who): prevent integer overflow in limit calculations
When `--limit` is omitted, the default value is `usize::MAX` to mean "unlimited". The previous code used `(limit + 1) as i64` to fetch one extra row for "has more" detection. This caused integer overflow: usize::MAX + 1 = 0 (wraps around) The resulting `LIMIT 0` clause returned zero rows, making the `who` subcommands appear to find nothing even when data existed. Fix: Use `saturating_add(1)` to cap at `usize::MAX` instead of wrapping, then `.min(i64::MAX as usize)` to ensure the value fits in SQLite's signed 64-bit LIMIT parameter. Includes regression tests that verify `usize::MAX` limit returns results. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -13,7 +13,9 @@ pub(super) fn query_active(
|
||||
limit: usize,
|
||||
include_closed: bool,
|
||||
) -> Result<ActiveResult> {
|
||||
let limit_plus_one = (limit + 1) as i64;
|
||||
// Prevent overflow: saturating_add caps at usize::MAX instead of wrapping to 0.
|
||||
// The .min() ensures the value fits in i64 for SQLite's LIMIT clause.
|
||||
let limit_plus_one = limit.saturating_add(1).min(i64::MAX as usize) as i64;
|
||||
|
||||
// State filter for open-entities-only (default behavior)
|
||||
let state_joins = if include_closed {
|
||||
|
||||
@@ -16,7 +16,9 @@ pub(super) fn query_workload(
|
||||
limit: usize,
|
||||
include_closed: bool,
|
||||
) -> Result<WorkloadResult> {
|
||||
let limit_plus_one = (limit + 1) as i64;
|
||||
// Prevent overflow: saturating_add caps at usize::MAX instead of wrapping to 0.
|
||||
// The .min() ensures the value fits in i64 for SQLite's LIMIT clause.
|
||||
let limit_plus_one = limit.saturating_add(1).min(i64::MAX as usize) as i64;
|
||||
|
||||
// Query 1: Open issues assigned to user
|
||||
let issues_sql = "SELECT i.iid,
|
||||
|
||||
@@ -3394,3 +3394,38 @@ fn active_excludes_closed_entity_discussions() {
|
||||
assert_eq!(result.discussions.len(), 2);
|
||||
assert_eq!(result.total_unresolved_in_window, 2);
|
||||
}
|
||||
|
||||
// ─── Regression: Unlimited limit must not overflow ─────────────────────────
|
||||
|
||||
#[test]
|
||||
fn workload_unlimited_limit_returns_results() {
|
||||
// Regression test for integer overflow bug: when limit=usize::MAX, the
|
||||
// expression (limit + 1) wrapped to 0, causing LIMIT 0 to return no rows.
|
||||
let conn = setup_test_db();
|
||||
insert_project(&conn, 1, "group/repo");
|
||||
insert_issue(&conn, 1, 1, 100, "alice");
|
||||
insert_assignee(&conn, 1, "alice");
|
||||
|
||||
// usize::MAX simulates the "unlimited" default when --limit is omitted
|
||||
let result = query_workload(&conn, "alice", None, None, usize::MAX, false).unwrap();
|
||||
assert!(
|
||||
!result.assigned_issues.is_empty(),
|
||||
"usize::MAX limit should return results, not overflow to LIMIT 0"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_unlimited_limit_returns_results() {
|
||||
// Same regression test for query_active
|
||||
let conn = setup_test_db();
|
||||
insert_project(&conn, 1, "group/repo");
|
||||
insert_issue(&conn, 1, 1, 100, "alice");
|
||||
insert_discussion(&conn, 1, 1, None, Some(1), true, false);
|
||||
insert_note(&conn, 1, 1, 1, "alice");
|
||||
|
||||
let result = query_active(&conn, None, 0, usize::MAX, false).unwrap();
|
||||
assert!(
|
||||
!result.discussions.is_empty(),
|
||||
"usize::MAX limit should return results, not overflow to LIMIT 0"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user