fix: handle IPv6 bracketed hosts in strip_ssh_port#310
Conversation
The split_once(':') call split inside the IPv6 literal [::1], producing
a corrupt URL. Now split on "]:" for bracketed hosts and rsplit_once(':')
otherwise.
Closes #309
Co-Authored-By: Sachin Iyer <siyer@detail.dev>
Original prompt from Sachin
|
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| // IPv6 bracketed host, e.g. `[::1]:22` — split on `]:`. | ||
| host_port.split_once("]:").map(|(h, _)| format!("{h}]"))? | ||
| } else { | ||
| host_port.rsplit_once(':').map(|(h, _)| h.to_string())? |
There was a problem hiding this comment.
📝 Info: Behavioral change from split_once to rsplit_once in non-IPv6 path
Line 50 changes from split_once(':') to rsplit_once(':'). For valid SSH authority strings like github.com:22, these are equivalent since there is exactly one colon. The only difference would arise for malformed authorities with multiple colons (e.g. host:foo:22), where split_once would yield host=host while rsplit_once yields host=host:foo. Since such authorities are not valid URL forms and cannot match GitHub's hostname anyway, this is a safe change — but it is a subtle semantic difference worth noting.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch on the semantic difference. As noted, for any well-formed SSH authority there's exactly one colon separating host from port, so split_once and rsplit_once are equivalent. The switch to rsplit_once is intentional — it's the more defensive choice for the non-IPv6 path, mirroring RFC 3986's rule that the port is the segment after the last colon in the host info. Agreed this is safe.
## Summary Patch release bump `0.2.5` → `0.2.6`. Includes fixes merged since v0.2.5: - `#313` serialize SHELL env var mutation in completions tests - `#312` validate `$SHELL` before extracting shell name in completions - `#311` warn on config parse errors during auth login - `#310` handle IPv6 bracketed hosts in `strip_ssh_port` Once merged, the `release.yml` workflow will automatically tag `v0.2.6`, build platform artifacts via `cargo-dist`, and publish a GitHub Release. Link to Devin session: https://app.devin.ai/sessions/02dfe916725247d6955ba0d4a49460df Requested by: @sachiniyer <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/usedetail/cli/pull/315" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open in Devin Review"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Release v0.2.6 to ship fixes for shell detection, SSH host parsing, and clearer auth warnings, plus more reliable completion tests. - **Bug Fixes** - Serialize `SHELL` env var mutation in completions tests to prevent flakiness. - Validate `$SHELL` before extracting the shell name in completions. - Warn on config parse errors during `auth login`. - Handle IPv6 bracketed hosts in `strip_ssh_port`. <sup>Written for commit db9999c. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/usedetail/cli/pull/315?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Sachin Iyer <siyer@detail.dev>
Summary
strip_ssh_portusedsplit_once(':')to separate host from port, which splits inside an IPv6 literal like[::1]:22— producing a corruptssh://git@[/owner/repo.git.Fix: for bracketed hosts, split on
"]:"; otherwisersplit_once(':'):Adds
strip_ssh_port_handles_ipv6test.Closes #309
Link to Devin session: https://app.devin.ai/sessions/e9a8b1c51a604a08b3642f119c1c7384
Requested by: @sachiniyer
Summary by cubic
Fixes URL parsing for SSH URLs with IPv6 bracketed hosts by correctly stripping the port without corrupting the host. Prevents malformed URLs like ssh://git@[/owner/repo.git when the input includes
[::1]:22.strip_ssh_port.strip_ssh_port_handles_ipv6.Written for commit 7eebc4e. Summary will update on new commits.