Handle IPv6 literal parsing in network utilities#2303
Handle IPv6 literal parsing in network utilities#2303KristopherKubicki wants to merge 1 commit intostagingfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances IPv6 address parsing in network utilities to handle both bracketed and unbracketed IPv6 literals, with or without explicit ports. The implementation uses the ipaddress module to validate IPv6 addresses and applies heuristics to disambiguate IPv6 addresses from port numbers.
Key Changes:
- Add
ipaddressimport for IPv6 validation - Refactor
_parse_target()to handle bracketed IPv6 format[host]:port - Add heuristic parsing for unbracketed IPv6 addresses by validating if the host portion (before last colon) is a valid IPv6 address
- Introduce
_is_ipv6_literal()helper function to validate IPv6 addresses - Add test coverage for IPv6 literals without ports
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/utils/network.py | Refactored _parse_target() to support bracketed IPv6 notation and unbracketed IPv6 heuristics; added _is_ipv6_literal() helper function for validation |
| tests/test_network_helpers.py | Added test case for IPv6 literal without port to verify default port usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _parse_target(target: str, default_port: int) -> tuple[str, int]: | ||
| """Return ``(host, port)`` tuple for ``target``.""" |
There was a problem hiding this comment.
[nitpick] The docstring for _parse_target is minimal and doesn't document the complex IPv6 parsing logic. Consider expanding it to explain the supported formats and parsing rules, for example:
"""Return ``(host, port)`` tuple for ``target``.
Supports the following formats:
- hostname:port (e.g., "example.com:8080")
- hostname (uses default_port)
- [ipv6]:port (e.g., "[2001:db8::1]:8443")
- [ipv6] (uses default_port)
- ipv6:port (e.g., "2001:db8::1:8443", parsed heuristically)
- ipv6 (uses default_port, e.g., "2001:db8::1")
For unbracketed IPv6 addresses, the last colon-separated segment is treated
as a port if it's all digits and the remaining part is a valid IPv6 address.
"""This would help maintainers understand the parsing logic and expected behavior.
| try: | ||
| return host, int(port_str) | ||
| except ValueError: | ||
| return host, default_port | ||
| return host, default_port | ||
|
|
||
| if ":" in target: | ||
| host_candidate, port_candidate = target.rsplit(":", 1) | ||
| if port_candidate.isdigit(): | ||
| if ":" not in host_candidate or _is_ipv6_literal(host_candidate): | ||
| try: | ||
| return host_candidate, int(port_candidate) |
There was a problem hiding this comment.
The code doesn't validate that the parsed port number is within the valid range (0-65535). For example, _parse_target("example.com:99999", 80) would return ("example.com", 99999), which is an invalid port.
Consider adding validation:
port = int(port_str)
if 0 <= port <= 65535:
return host, port
return host, default_portThis validation should be applied at lines 29 and 39 where ports are parsed.
|
Stale pull request message |
1 participant
Summary
Testing
Codex Task