Skip to content

Fix IPv6 authority parsing: Host()/Port() return garbage, scheme-less IPv6 breaks #1

Description

@prorochestvo

Summary

Parse mangles every IPv6 authority. The instance splitter in parseAuthority
uses strings.SplitN(instance, ":", 2), which splits on the first colon — and
an IPv6 literal is nothing but colons. As a result Host() and Port() return
garbage for bracketed IPv6 hosts, and scheme-less IPv6 is force-dumped into
Database().

A detailed, executable implementation plan lives in the repo at
plans/004-ipv6-authority-parsing.md.
This issue tracks the work; the Open Questions below must be settled before
implementation starts.

Current behavior (verified against HEAD)

Input Host() Port() Database() Addr() Marshal
redis://[::1]:6379 [ :1]:6379 [::1]:6379 redis://[::1]:6379
redis://[::1]:6379/0 [ :1]:6379 0 [::1]:6379 redis://[::1]:6379/0
redis://[::1] [ :1] [::1] redis://[::1]
postgres://u:p@[2001:db8::1]:5432/db [2001 db8::1]:5432 db [2001:db8::1]:5432 round-trips (string)
http://[fe80::1%25eth0]:8080 [fe80 :1%25eth0]:8080 [fe80::1%25eth0]:8080 round-trips
[::1]:6379 (scheme-less) /[::1]:6379 ////[::1]:6379
[2001:db8::1]:5432/db (scheme-less) /[2001:db8::1]:5432/db ////[2001:db8::1]:5432/db
redis://127.0.0.1:6379 (IPv4 baseline) 127.0.0.1 6379 127.0.0.1:6379 ok

Two distinct failure classes:

  1. Authority form (redis://[::1]:6379): Host()/Port() are garbage, but
    Addr()/Marshal come out correct by accident — the wrong split
    (host="[", port=":1]:6379") is exactly reversed by Addr()'s
    host + ":" + port rejoin (the same mechanism as the Telegram token trick). The
    broken observable surface is Host()/Port().
  2. Scheme-less form ([::1]:6379): no accidental save — the classifier sees the
    first colon (inside the brackets), fails the colon-then-digit test, and routes
    the whole literal into Database().

Root cause

  • parseAuthority: strings.SplitN(instance, ":", 2) is bracket-naive.
  • Parse scheme-less classifier: the first-colon heuristic is bracket-naive.
  • Addr(): host + ":" + port does not bracket an IPv6 host.

Proposed approach (surgical, zero-dependency)

Fix in place; do not adopt net/url for this (that is a separate, larger
decision — see OQ-5). Roughly ~15 lines plus a test table:

  1. splitHostPort (new unexported helper): a leading [ marks an IPv6 literal —
    the host runs to the matching ], with an optional :port after it. Without a
    leading [ it keeps the exact current first-colon split, so the Telegram trick
    and all existing cases stay byte-for-byte unchanged.
  2. Addr(): bracket the host when it contains a : and is not already
    bracketed. assembleDSN / Marshal / Redacted route through Addr(), so they
    need no change (one home for the bracketing decision).
  3. Classifier: one new case for a leading [ → reshape to the // authority
    form and reuse splitHostPort.
  4. Tests: full IPv6 truth table (compressed/full/mixed forms, with/without port,
    zone id, IPv4-mapped, /db, ?opts, credentials, scheme-less) + round-trip
    assertions + regression set (existing cases + Telegram trick + IPv4).

net.SplitHostPort / net.JoinHostPort were considered and rejected: the former
rejects a missing port and the Telegram first-colon trick; the latter mandates a
port and would emit [::1]: for the host-only form.

Backward-compatibility (hard constraints)

  • All existing TestParse success cases pass unchanged.
  • The Telegram Addr()-as-token trick stays intact (bracket logic fires only on a
    leading [).
  • No new dependencies, no CGO, regex preserved.
  • Host()/Port() moving from garbage to correct is a behavior change; no known
    consumer feeds an IPv6 DSN today, but classify the commit fix: and flag it.

Open questions (decide before implementing)

  • OQ-1 (headline): store the host unbracketed (Host()::1, matches
    net/url.URL.Hostname(); requires the Addr() bracketing edit) vs bracketed
    (Host()[::1]; smaller diff, but a leaky accessor a caller must strip before
    net.Dial). Lean: unbracketed — the accessor is the public contract.
  • OQ-2: zone ids ([fe80::1%eth0] / %25) and IPv4-mapped ([::ffff:1.2.3.4]):
    preserve the literal, percent-decode, or validate? Lean: preserve literal, no
    decode, no validation.
  • OQ-3: Addr() with no port → [::1] or ::1? Near-forced [::1], else
    Marshal produces the invalid redis://::1.
  • OQ-4: also normalize the host on SetHost for a path-independent Host()?
    Lean: yes, but optional under the unbracketed baseline.
  • OQ-5: keep this surgical now, or fold IPv6 into the net/url engine decision?
    Lean: fix IPv6 surgically now; let the net/url migration proceed on its own
    timeline.

Full task breakdown, acceptance criteria, and the complete 13-row truth table are
in plans/004-ipv6-authority-parsing.md.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions