Skip to content

fix(cli): reject non-finite --timeout values - #3122

Open
twelfthlabor wants to merge 1 commit into
sherlock-project:masterfrom
twelfthlabor:fix/timeout-nonfinite
Open

twelfthlabor wants to merge 1 commit into
sherlock-project:masterfrom
twelfthlabor:fix/timeout-nonfinite

Conversation

@twelfthlabor

@twelfthlabor twelfthlabor commented Sep 10, 2026

Copy link
Copy Markdown

Part of #3120.

Problem

timeout_check only rejects values <= 0. --timeout inf and --timeout nan pass validation and crash the entire run with unhandled exceptions from the socket layer (OverflowError: timestamp out of range for platform time_t / ValueError: Invalid value NaN), escaping get_response's except clauses. Huge finite values like --timeout 1e5 are accepted (user's explicit choice, intentionally not capped here).

Fix

if not math.isfinite(float_value) or float_value <= 0:
    raise ArgumentTypeError(...)

nan previously slipped even the <= 0 check (nan <= 0 is False). Large finite values remain accepted; only non-finite values are rejected. The "huge finite values block indefinitely" half of #3120's title stays out of scope: capping large finite values would override the user's explicit choice, so the issue stays open for it.

Relation to open PRs #2876 / #2867 / #2955

All three address #2866 (non-numeric input) and keep if float_value <= 0 unchanged, so inf/nan pass through all of them. This is complementary; no conflict.

Testing

  • New test test_timeout_check_rejects_nonfinite_values: fails on master (DID NOT RAISE), passes here; '1e5' acceptance pinned as a regression guard.
  • End-to-end (offline, stubbed network): --timeout 1e5 → 100000.0 reaches the query layer; inf/nan/-5 → clean SystemExit(2) instead of a crash.
  • Full offline suite: pytest tests -m "not online" -q → 16 passed
  • ruff: findings identical to master (single-line insertion shift only)

Verification

Independently re-verified on fresh checkout: condition reviewed line-by-line, test proven to bite on master, success paths unchanged end-to-end, ' 1.5 ' whitespace behavior unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant