Skip to content

Studio: leave a bracketed IPv6 host unchanged in dial_host - #12733

Merged
Lyxot merged 3 commits into
unslothai:mainfrom
drakeo338:claude/12719-fix
Oct 8, 2026
Merged

Lyxot merged 3 commits into
unslothai:mainfrom
drakeo338:claude/12719-fix

Conversation

@drakeo338

Copy link
Copy Markdown
Contributor

Fixes #12719.

dial_host("[::1]") returned "[[::1]]", which is not a valid URL authority, so a bracketed IPv6 host would break the self-call URLs built from it. published_url_host already returns bracketed input unchanged. dial_host now does the same, and a test covers it.

Ran the host policy test file on the committed HEAD: 3 passed.

@danielhanchen

Copy link
Copy Markdown
Member

Confirmed dial_host in studio/backend/utils/host_policy.py still turns "[::1]" into "[[::1]]" on main, unlike published_url_host just above it, and this mirrors that guard. Will get this reviewed.

@Lyxot Lyxot self-assigned this Oct 8, 2026
Lyxot added 2 commits October 8, 2026 14:35
… formatters

published_url_host and dial_host each carried their own copy of the bracket rule, and the copies had drifted: only one left an already-bracketed literal alone. Both now call one helper, so they differ only in whether the zone id is percent-escaped. Tests pin the zone id staying literal for dial_host and bracketed input with a zone id for published_url_host.
@Lyxot

Lyxot commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Pushed a small follow-up on top of the original fix and merged the latest main (the branch was 281 commits behind).

What changed:

  • published_url_host and dial_host now share one private helper for the bracket rule instead of each carrying a copy. published_url_host behaves exactly as before; the two differ only in whether the zone id is percent-escaped.
  • The dial_host test gained zone-id cases (fe80::1%en0 stays literal, bracketed or not), and published_url_host gained a bracketed-plus-zone-id case.

Note for reviewers: I traced both callers and no current path hands dial_host a bracketed host (the value comes from the bound socket address, the ASGI server pair, or the 127.0.0.1 fallback, and --host "[::1]" fails to bind), so this is hardening that keeps the two formatters from drifting rather than a fix for a failure users can hit.

cd studio/backend && python -m pytest tests/test_bind_host_policy.py -q   # 68 passed

@Lyxot

Lyxot commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Review and CI summary for head e21422e6cb, checked on a staging mirror of the same commit.

Codex review: converged — "Didn't find any major issues".

CI: 40 passed, 2 skipped, 1 failed (run). The one failure is (Python 3.13, a-k), with 7 tests in tests/test_hub_token_caller_identity.py. It is not caused by this PR: the same 7 tests fail in the same job on main at the commit this branch merged (main run), and this branch differs from that commit only in utils/host_policy.py and tests/test_bind_host_policy.py.

What changed during this pass:

  • Merged the latest main (the branch was 281 commits behind, no conflicts).
  • published_url_host and dial_host now share one helper for the bracket rule; published_url_host behaves as before.
  • Added zone-id cases to the dial_host test and a bracketed-plus-zone-id case to the published_url_host test.

There were no unresolved review threads to address.

@Lyxot Lyxot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The two host formatters now share one bracket rule, and the change is covered by tests. Codex and CI were checked on a staging mirror of the same head; the one failing CI job fails identically on main.

@Lyxot
Lyxot merged commit adf23cc into unslothai:main Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants