Skip to content

fix: Address PR review findings - resource limits and code cleanup#108

Merged
inureyes merged 2 commits intofeature/issue-98-hostlist-expressionfrom
pr-107
Dec 16, 2025
Merged

fix: Address PR review findings - resource limits and code cleanup#108
inureyes merged 2 commits intofeature/issue-98-hostlist-expressionfrom
pr-107

Conversation

@inureyes
Copy link
Member

Implement range expansion syntax for specifying multiple hosts:

  • Simple ranges: node[1-5] -> node1, node2, node3, node4, node5
  • Zero-padded: node[01-05] -> node01, node02, node03, node04, node05
  • Comma-separated: node[1,3,5] -> node1, node3, node5
  • Mixed: node[1-3,7] -> node1, node2, node3, node7
  • Cartesian product: rack[1-2]-node[1-3] -> 6 hosts
  • File input: ^/path/to/file reads hosts from file

Integrate with -H option, --filter, and --exclude options.
Add 52 unit tests for hostlist module.HIGH Priority Fixes:

  • Add resource exhaustion protection in parse_hostfile():
    • Maximum file size limit of 1 MB
    • Maximum line count limit of 100,000 lines
    • Check file size before reading to prevent DoS attacks

MEDIUM Priority Fixes:

  • Remove code duplication:

    • Move is_hostlist_expression() and looks_like_hostlist_range()
      from src/main.rs and src/app/nodes.rs to src/hostlist/mod.rs
    • Export functions as public API from hostlist module
    • Update all call sites to use the exported functions
  • Remove unused variable:

    • Remove unused 'sign' variable from parse_number() in parser.rs
  • Add overflow protection:

    • Use checked_mul() for cartesian product allocations in expander.rs
    • Return RangeTooLarge error if overflow would occur
    • Prevent integer overflow in intermediate calculations

All changes compile successfully and pass tests (cargo test, cargo clippy).

HIGH Priority Fixes:
- Add resource exhaustion protection in parse_hostfile():
  * Maximum file size limit of 1 MB
  * Maximum line count limit of 100,000 lines
  * Check file size before reading to prevent DoS attacks

MEDIUM Priority Fixes:
- Remove code duplication:
  * Move is_hostlist_expression() and looks_like_hostlist_range()
    from src/main.rs and src/app/nodes.rs to src/hostlist/mod.rs
  * Export functions as public API from hostlist module
  * Update all call sites to use the exported functions

- Remove unused variable:
  * Remove unused 'sign' variable from parse_number() in parser.rs

- Add overflow protection:
  * Use checked_mul() for cartesian product allocations in expander.rs
  * Return RangeTooLarge error if overflow would occur
  * Prevent integer overflow in intermediate calculations

All changes compile successfully and pass tests (cargo test, cargo clippy).
@inureyes inureyes self-assigned this Dec 16, 2025
@inureyes inureyes added type:refactor Code refactoring status:done Completed priority:medium Medium priority issue type:test Test related changes labels Dec 16, 2025
@inureyes inureyes marked this pull request as ready for review December 16, 2025 23:51
@inureyes inureyes merged commit 59ff34f into feature/issue-98-hostlist-expression Dec 16, 2025
1 check passed
@inureyes inureyes deleted the pr-107 branch December 16, 2025 23:52
inureyes added a commit that referenced this pull request Dec 16, 2025
* feat: Add pdsh-style hostlist expression support (#98)

Implement range expansion syntax for specifying multiple hosts:
- Simple ranges: node[1-5] -> node1, node2, node3, node4, node5
- Zero-padded: node[01-05] -> node01, node02, node03, node04, node05
- Comma-separated: node[1,3,5] -> node1, node3, node5
- Mixed: node[1-3,7] -> node1, node2, node3, node7
- Cartesian product: rack[1-2]-node[1-3] -> 6 hosts
- File input: ^/path/to/file reads hosts from file

Integrate with -H option, --filter, and --exclude options.
Add 52 unit tests for hostlist module.

* fix: Address PR review findings - resource limits and code cleanup (#108)

* fix: Address PR review findings - resource limits and code cleanup

HIGH Priority Fixes:
- Add resource exhaustion protection in parse_hostfile():
  * Maximum file size limit of 1 MB
  * Maximum line count limit of 100,000 lines
  * Check file size before reading to prevent DoS attacks

MEDIUM Priority Fixes:
- Remove code duplication:
  * Move is_hostlist_expression() and looks_like_hostlist_range()
    from src/main.rs and src/app/nodes.rs to src/hostlist/mod.rs
  * Export functions as public API from hostlist module
  * Update all call sites to use the exported functions

- Remove unused variable:
  * Remove unused 'sign' variable from parse_number() in parser.rs

- Add overflow protection:
  * Use checked_mul() for cartesian product allocations in expander.rs
  * Return RangeTooLarge error if overflow would occur
  * Prevent integer overflow in intermediate calculations

All changes compile successfully and pass tests (cargo test, cargo clippy).

* docs: Add HOSTLIST EXPRESSIONS section and update option docs in manpage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:medium Medium priority issue status:done Completed type:refactor Code refactoring type:test Test related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant