fix(remote): normalize localhost to 127.0.0.1 for IPv4-only remote servers#245
Merged
Conversation
…rvers rdc open --proxy localhost:39920 stalled indefinitely at "uploading: 0%" on dual-stack hosts: localhost resolves to ::1 first, but renderdoccmd remoteserver binds IPv4-only (0.0.0.0) and never listens on ::1, so the RenderDoc C++ client blocks forever with no error. Add _normalize_remote_host (literal "localhost", case-insensitive -> 127.0.0.1; ::1, IPv6 literals, and real hostnames untouched) and apply it at every seam where a user host string reaches a RenderDoc API: - top of _load_remote_replay (covers --proxy and android open; also fixes state.remote_url shown by rdc status; guards adb:// / protocol URLs) - parse_url (rdc remote connect/list/capture/setup) - build_conn_url (closes the saved-state ghost path: a pre-fix state file keyed on localhost is rewritten at URL assembly) - hardcoded literals in android.py and capture_core.py State files previously keyed on localhost become stale after upgrade; rdc remote disconnect && rdc remote connect 127.0.0.1:PORT cleans them up.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR implements a fix for ChangesLocalhost normalization for proxy connections
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
openspec/changes/2026-06-10-proxy-localhost-ipv4/proposal.md (2)
58-66: ⚡ Quick winAdd language identifier to fenced code block.
The code block should specify
pythonafter the opening backticks for proper syntax highlighting and to follow Markdown best practices.📝 Proposed fix
-``` +```python def _normalize_remote_host(host: str) -> str: if host.lower() == "localhost":🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/2026-06-10-proxy-localhost-ipv4/proposal.md` around lines 58 - 66, The fenced code block in proposal.md should include a language identifier for proper syntax highlighting; update the block that contains the function _normalize_remote_host(host: str) -> str to start with ```python instead of ``` so the snippet (including the function name _normalize_remote_host and its body) is treated as Python code by Markdown renderers.Source: Linters/SAST tools
80-80: ⚡ Quick winFix heading formatting.
Add a space after
#for proper ATX-style heading syntax.📝 Proposed fix
-**Option A** — normalize inside `parse_url` only, and also add a thin wrapper +# Option A + +Normalize inside `parse_url` only, and also add a thin wrapperOr if the bold formatting was intentional (not a heading):
-**Option A** — normalize inside `parse_url` only, and also add a thin wrapper +**Option A** — Normalize inside `parse_url` only, and also add a thin wrapper🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/2026-06-10-proxy-localhost-ipv4/proposal.md` at line 80, The line beginning "`#4`, `#5`). Applying normalization there covers those paths with one change." is being parsed as an ATX heading because there is no space after the hash; fix by either adding a space after the hash to make a proper heading (e.g., change "`#4`, `#5`)..." to "# 4, # 5)..." or, if the intent was not to create a heading, escape or reformat the tokens (wrap "`#4`, `#5`" in backticks or bold them) so they render as inline text rather than an ATX heading.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openspec/changes/2026-06-10-proxy-localhost-ipv4/proposal.md`:
- Around line 58-66: The fenced code block in proposal.md should include a
language identifier for proper syntax highlighting; update the block that
contains the function _normalize_remote_host(host: str) -> str to start with
```python instead of ``` so the snippet (including the function name
_normalize_remote_host and its body) is treated as Python code by Markdown
renderers.
- Line 80: The line beginning "`#4`, `#5`). Applying normalization there covers
those paths with one change." is being parsed as an ATX heading because there is
no space after the hash; fix by either adding a space after the hash to make a
proper heading (e.g., change "`#4`, `#5`)..." to "# 4, # 5)..." or, if the intent
was not to create a heading, escape or reformat the tokens (wrap "`#4`, `#5`" in
backticks or bold them) so they render as inline text rather than an ATX
heading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58b4c9a6-df88-4007-bd00-67812cd71542
📒 Files selected for processing (13)
openspec/changes/2026-06-10-proxy-localhost-ipv4/proposal.mdopenspec/changes/2026-06-10-proxy-localhost-ipv4/tasks.mdopenspec/changes/2026-06-10-proxy-localhost-ipv4/test-plan.mdsrc/rdc/capture_core.pysrc/rdc/commands/android.pysrc/rdc/commands/capture_control.pysrc/rdc/daemon_server.pysrc/rdc/remote_core.pytests/unit/test_android_commands.pytests/unit/test_capture_control.pytests/unit/test_capture_core.pytests/unit/test_daemon_server_unit.pytests/unit/test_remote_core.py
BANANASJIM
added a commit
that referenced
this pull request
Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rdc open <capture> --proxy localhost:39920stalled indefinitely atuploading: 0%on dual-stack hosts:localhostresolves to::1(IPv6) first, whilerenderdoccmd remoteserverbinds IPv4 only (verified viass -tlnp: listener on0.0.0.0:39920, none on::1). RenderDoc's native connection layer does not fall back to IPv4, so the upload never progresses and there is no error.This change normalizes the literal hostname
localhost(case-insensitive) to127.0.0.1everywhere a host string reaches a RenderDoc remote API.Seams covered
_normalize_remote_hosthelper inremote_core.py, applied inparse_urlandbuild_conn_url(the latter also closes a saved-state path: a stalelocalhost_<port>.jsonwritten by an older version would otherwise resurrect the un-normalized host throughrdc remote list/capture).daemon_server._load_remote_replaynormalizes the incoming--remote-urlup front (with anadb://guard), so bothCreateRemoteServerConnectionand therdc statusdisplay use the normalized value."localhost"literals replaced inandroid.py(remote-server connect,CreateTargetControl) andcapture_core.py(EnumerateRemoteTargets).capture_control._connectnormalizes--hostforattach/capture-trigger/capture-list/capture-copy(review-found gap; covers the default and explicit--host localhost).Untouched by design:
::1and bracketed IPv6 literals, real hostnames (mylocalhost,localhost.example.com),adb://URLs, and split-mode--connect(Python'ssocket.create_connectioniterates getaddrinfo with immediate ECONNREFUSED fallback, so it does not stall).Testing
test_remote_core.py,test_daemon_server_unit.py,test_android_commands.py,test_capture_core.py,test_capture_control.py— asserting the host actually passed to the (mocked) RenderDoc APIs, plus an 8-case normalization boundary matrix (LOCALHOST,mylocalhost,localhost.example.com,[::1], etc.).uploading: 0%stall with--proxy localhost:39920; with this fix the same command opens the session,rdc statusshows127.0.0.1:39920, remote draws and depth export work end-to-end; explicit--proxy 127.0.0.1unchanged; nolocalhost_*.jsonstate ghost created.Summary by CodeRabbit
Release Notes
Bug Fixes
rdchanging when using proxy URLs withlocalhost(e.g.,localhost:39920) on dual-stack hosts by normalizinglocalhostto127.0.0.1for IPv4 loopback resolution.Tests