Skip to content

fix(postgres): use net.JoinHostPort to support IPv6 hosts#3052

Open
kaldown wants to merge 2 commits intogoogleapis:mainfrom
kaldown:fix/postgres-ipv6-host
Open

fix(postgres): use net.JoinHostPort to support IPv6 hosts#3052
kaldown wants to merge 2 commits intogoogleapis:mainfrom
kaldown:fix/postgres-ipv6-host

Conversation

@kaldown
Copy link
Copy Markdown

@kaldown kaldown commented Apr 14, 2026

Summary

The postgres source built its connection URL with fmt.Sprintf("%s:%s", host, port), which produces invalid authority for IPv6 literals. pgx then parsed the result as an 8-segment IPv6 with the port absorbed as a trailing hextet, producing dial errors such as:

dial tcp [fd00::1:5432]:5432: connect: no route to host

Replacing the concatenation with net.JoinHostPort wraps IPv6 literals in brackets as required by RFC 3986, and leaves IPv4 addresses and hostnames unchanged.

URL construction is extracted into a new BuildPostgresURL helper so it can be unit-tested without a live database.

Changes

  • internal/sources/postgres/postgres.go
    • Introduce BuildPostgresURL that uses net.JoinHostPort.
    • Encode query parameters with url.Values.Encode() (per gemini-code-assist review): proper escaping of &, =, spaces, and deterministic key order.
    • Remove the now-unused ConvertParamMapToRawQuery helper. It was only referenced from this file; internal/sources/cockroachdb has its own independent copy and is not affected.
  • internal/sources/postgres/postgres_test.go
    • Add TestBuildPostgresURL covering hostname, IPv4 (127.0.0.1), IPv6 loopback (::1), IPv6 documentation (2001:db8::1), query-parameter key ordering, and URL-encoding of values containing special characters. Each expected URL is also fed through pgx.ParseConfig to verify downstream compatibility.
    • Drop TestConvertParamMapToRawQuery alongside the removed function.

Backward compatibility

  • net.JoinHostPort behaves identically to fmt.Sprintf("%s:%s", host, port) for hostnames and IPv4 addresses — brackets are only added when the host contains a colon.
  • The query-string output order is now deterministic (sorted by key) rather than map-iteration order. The previous order was already non-deterministic, so any consumer that depended on a specific order was already broken.
  • queryParams values that previously contained special characters such as & or = would have produced malformed URLs; they are now correctly percent-encoded.

Test plan

  • go test ./internal/sources/postgres/... — all tests pass including new IPv6 and query-param encoding cases.
  • go vet ./internal/sources/postgres/... — clean.
  • End-to-end: built the patched toolbox and used it via --prebuilt postgres --stdio to connect to a real PostgreSQL instance reachable only over IPv6 (ULA address via a WireGuard-style tunnel). The MCP client listed tools and executed queries successfully, whereas the unpatched binary failed with the mangled dial error shown above.

@kaldown kaldown requested a review from a team as a code owner April 14, 2026 10:03
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 14, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@kaldown kaldown force-pushed the fix/postgres-ipv6-host branch from 08bb82c to a7565a1 Compare April 14, 2026 10:03
@kaldown kaldown force-pushed the fix/postgres-ipv6-host branch from a7565a1 to bfa74ba Compare April 14, 2026 10:03
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a BuildPostgresURL helper function in the Postgres source to correctly handle IPv6 addresses using net.JoinHostPort and adds corresponding unit tests. Feedback was provided to improve the robustness of URL generation by using url.Values for query parameter encoding instead of the manual concatenation method, ensuring special characters are correctly escaped and parameters are ordered deterministically.

Comment thread internal/sources/postgres/postgres.go
The postgres source built its connection URL with
`fmt.Sprintf("%s:%s", host, port)`, which produces invalid authority
for IPv6 literals. pgx then treated the port as a trailing IPv6
hextet, producing dial errors such as:

    dial tcp [fd00::1:5432]:5432: connect: no route to host

Replacing the concatenation with `net.JoinHostPort` wraps IPv6
literals in brackets as required by RFC 3986, and leaves IPv4
addresses and hostnames unchanged.

URL construction is extracted into `BuildPostgresURL` so it can be
unit-tested without a live database. While there, query parameters
are now encoded via `url.Values.Encode()` (gemini-code-assist review
feedback) — this properly escapes special characters such as `&`,
`=`, and spaces, and produces a deterministic key order. The
previous helper `ConvertParamMapToRawQuery`, which was
string-concatenation based and only called from this function, is
removed (cockroachdb has its own independent copy and is not
affected).

Tests cover hostname, IPv4, IPv6 loopback (::1), IPv6 documentation
(2001:db8::1), and query-parameter encoding (special characters and
key ordering).
@kaldown kaldown force-pushed the fix/postgres-ipv6-host branch from bfa74ba to f9bc4ab Compare April 14, 2026 10:51
@kaldown
Copy link
Copy Markdown
Author

kaldown commented Apr 14, 2026

@googlebot I signed it!

Lock in the link-local case (fe80::1%eth0). Go's net/url auto-encodes
the zone-separator '%' as '%25' per RFC 6874, and pgx.ParseConfig
accepts the result; the test asserts both the exact URL and the
downstream parser round-trip.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants