Improve scanner CLI stability and reporting#6
Improve scanner CLI stability and reporting#6abaskalov wants to merge 1 commit intoBeardedTek-com:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the rtspscanner CLI so it behaves more reliably in non-interactive runs, adds support for port ranges in --ports/RTSP_SCAN_PORTS, and expands scan output with a more detailed summary (also reflected in the README).
Changes:
- Add port range parsing (e.g.,
554,8554,10000-10010) and address counting for summary reporting. - Avoid non-interactive
sttyissues and make ffmpeg timeout handling/reporting more robust. - Expand CLI output with scan summary details and document the new output/port syntax in the README.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| rtspscanner.py | Adds port range normalization, scan summary reporting, and safer non-interactive behavior. |
| README.md | Documents port range syntax and includes the new “Scan Summary” output examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Paths tried per candidate: 2 | ||
| RTSP URL checks: 8 | ||
| Timeout per RTSP check: 10s | ||
| Timeout retries: 2 |
There was a problem hiding this comment.
The example output shows "Timeout retries: 2", but the CLI default for --timeoutretries is currently 3 (and main() uses the CLI value). Update the README example to match the actual default/output, or change the default in code to 2 for consistency.
| Timeout retries: 2 | |
| Timeout retries: 3 |
| Paths tried per candidate: 2 | ||
| RTSP URL checks: 2 | ||
| Timeout per RTSP check: 10s | ||
| Timeout retries: 2 |
There was a problem hiding this comment.
The example output shows "Timeout retries: 2", but the CLI default for --timeoutretries is currently 3 (and main() uses the CLI value). Update the README example to match the actual default/output, or change the default in code to 2 for consistency.
| Timeout retries: 2 | |
| Timeout retries: 3 |
| for value in self.splitCSV(ports): | ||
| value = value.strip() | ||
| if not value: | ||
| continue | ||
| if '-' not in value: | ||
| normalized.append(value) | ||
| continue | ||
|
|
||
| start, end = value.split('-', 1) | ||
| start = int(start.strip()) | ||
| end = int(end.strip()) |
There was a problem hiding this comment.
normalizePorts() calls int() on user-provided range endpoints without any error handling or validation. Inputs like --ports 554,abc or 1000- will raise ValueError and crash the CLI; consider catching parsing errors and surfacing a clear argparse/CLI error, and validating the allowed port range (1-65535).
| for value in self.splitCSV(ports): | |
| value = value.strip() | |
| if not value: | |
| continue | |
| if '-' not in value: | |
| normalized.append(value) | |
| continue | |
| start, end = value.split('-', 1) | |
| start = int(start.strip()) | |
| end = int(end.strip()) | |
| def parse_port(port_value, original_value): | |
| port_value = port_value.strip() | |
| if not port_value: | |
| raise ValueError(f"Invalid port specification '{original_value}': missing port value") | |
| try: | |
| port = int(port_value) | |
| except ValueError: | |
| raise ValueError(f"Invalid port specification '{original_value}': '{port_value}' is not a valid integer") | |
| if port < 1 or port > 65535: | |
| raise ValueError(f"Invalid port specification '{original_value}': port {port} is outside the allowed range 1-65535") | |
| return port | |
| for value in self.splitCSV(ports): | |
| value = value.strip() | |
| if not value: | |
| continue | |
| if '-' not in value: | |
| normalized.append(str(parse_port(value, value))) | |
| continue | |
| start, end = value.split('-', 1) | |
| start = parse_port(start, value) | |
| end = parse_port(end, value) |
| def normalizePorts(self, ports): | ||
| normalized = [] | ||
| for value in self.splitCSV(ports): | ||
| value = value.strip() | ||
| if not value: | ||
| continue | ||
| if '-' not in value: | ||
| normalized.append(value) | ||
| continue | ||
|
|
||
| start, end = value.split('-', 1) | ||
| start = int(start.strip()) | ||
| end = int(end.strip()) | ||
| if start > end: | ||
| start, end = end, start | ||
| normalized.extend(str(port) for port in range(start, end + 1)) | ||
|
|
||
| return ','.join(normalized) |
There was a problem hiding this comment.
normalizePorts() expands ranges into an explicit list of every port, which can allocate very large lists/strings (e.g., 1-65535 or worse) and make the CLI slow or OOM. Consider enforcing a reasonable maximum range size and/or passing ranges through without full expansion (if PortScan can accept ranges) to avoid accidental resource exhaustion.
| if flakyCount > 0: | ||
| CamDisp = "Cameras" if flakyCount > 1 else "Camera" | ||
| print(f"\n{len(scanner.scanResults['flaky'])} Flaky {CamDisp}:") | ||
| print(f"Potential {CamDisp.lower()} that cannot be verfied within {scanner.timeout} second timeout.") |
There was a problem hiding this comment.
Typo in user-facing output: "verfied" should be "verified".
| print(f"Potential {CamDisp.lower()} that cannot be verfied within {scanner.timeout} second timeout.") | |
| print(f"Potential {CamDisp.lower()} that cannot be verified within {scanner.timeout} second timeout.") |
Summary
rtspscannercan initialize and complete scans without crashing in non-interactive runs--portsandRTSP_SCAN_PORTS, including mixed CSV/range input such as554,8554,10000-10010Verification
tools/RTSPScanner/.venv/bin/python -m py_compile tools/RTSPScanner/rtspscanner.pytools/RTSPScanner/.venv/bin/rtspscanner -m scan -a 127.0.0.1 -c admin:104194411 -p 554,8554,10000-10002