Skip to content

fix(cli): ignore incomplete --budget pairs instead of panicking - #415

Merged
j-mendez merged 1 commit into
spider-rs:mainfrom
Zi-Yi-Ming:fix/cli-budget-odd-pair-panic
Oct 16, 2026
Merged

j-mendez merged 1 commit into
spider-rs:mainfrom
Zi-Yi-Ming:fix/cli-budget-odd-pair-panic

Conversation

@Zi-Yi-Ming

Copy link
Copy Markdown
Contributor

What does this PR do?

Fix a panic in spider_cli when parsing --budget with an incomplete path,limit pair.

The flag documents comma-separated pairs such as "*,1" or "*,100,/blog/,10". The parser used chunks(2) and always indexed [1]:

.chunks(2)
.map(|x| (x[0], x[1].parse::<u32>().unwrap_or_default()))

An odd token count produced a final one-element slice and panicked with an index-out-of-bounds abort. Realistic inputs that hit this:

  • --budget "*,100,/blog" — trailing path without a limit
  • --budget "*,100," — trailing comma

Incomplete pairs are now skipped so the CLI keeps the valid pairs instead of crashing.

How was this tested?

Extracted parse_budget and added unit tests in spider_cli/src/main.rs:

  • single pair "*,1"
  • multiple pairs "*,100,/blog/,10"
  • trailing path without limit (previously panicked)
  • trailing comma (previously panicked)
  • empty string
  • non-numeric limit (still defaults to 0, unchanged)
cargo test -p spider_cli

Note: this environment has no local Rust toolchain; please rely on CI for the full spider_cli test run.

Checklist

  • cargo test passes
  • cargo fmt applied
  • New public APIs have doc comments
  • Feature-gated behind a flag (if adding optional functionality)

Internal CLI helper only; no public library API change.

--budget expects comma-separated path,limit pairs. An odd number of
tokens (for example a trailing path with no limit, or a trailing comma)
made chunks(2) yield a one-element slice and indexing [1] panicked.

Parse pairs only when both path and limit are present so incomplete
input is ignored instead of aborting the process.
Copilot AI lite review requested due to automatic review settings September 16, 2026 13:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Zi-Yi-Ming

Copy link
Copy Markdown
Contributor Author

Note on CI: Security Audit is failing on
ustls\ (RUSTSEC-2026-0285). That advisory is also failing on \main\ and is unrelated to this CLI parser change. Lint, Rust, and Benches are green.

@j-mendez
j-mendez merged commit e27a159 into spider-rs:main Oct 16, 2026
6 of 7 checks passed
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.

3 participants