Conversation
libpq accepts hostaddr as the address to dial while host keeps its meaning for server identity, such as SNI and certificate verification. pgx instead passed hostaddr to the server as a run-time parameter, which rejected it as an unrecognized configuration parameter. Parse hostaddr alongside host and port, and dial it directly instead of resolving the host name. As with port, a single address applies to every host and any other count must match the host count. An address given without a host name is a complete target: no default socket directory is substituted for the missing host, because it would become the TLS server name.
|
This PR works as expected for the example case from the issue. |
|
There are multiple cases it doesn't handle or handles differently than libpq.
|
…pass Follow-up to the review of this pull request. libpq resolves hostaddr with AI_NUMERICHOST and treats the hostaddr list as the authority on how many connections are described; the parsing here did neither. - hostaddr has no single-value form. The connection count follows the hostaddr list when it is present, as in pqConnectOptions2, and a mismatch against the host list is rejected instead of dialling one address for several hosts. - An empty element gives its slot no address, so hostaddr=,127.0.0.1 tries the default Unix-domain socket first and the supplied address second. - hostaddr must be a numeric address. A host name is no longer resolved and a socket directory is no longer dialled as a Unix domain socket; the affected slot reports an error instead. - The .pgpass search key is hostaddr when no host name was written. - PGHOSTADDR is read as the environment form of hostaddr.
|
Thanks for the detailed review. All five are addressed in two follow-up commits on this same
One limit I want to be explicit about rather than overstate: netip.ParseAddr is stricter than Each case has a test. go build ./..., go vet ./pgconn/ and gofumpt -extra -l pgconn/ are One more thing I should state plainly, per the contributing guide: this is an AI-assisted |
Summary
hostaddrnames the address to dial whilehostkeeps its meaning for server identity — SNI and certificate verification. pgx did neither: the value fell through to the run-time parameters, so the server rejected it withunrecognized configuration parameter "hostaddr".psql "postgresql://someuser@hostname.not.used:54320/somedb?sslmode=require&host=hostname.for.sni&hostaddr=192.168.1.100"This treats
hostaddras a connection string field alongsidehostandport, and dials it instead of resolving the host name.Changes
pgconn/config.go:hostaddris added tonotRuntimeParams, soParseConfigconsumes it instead of forwarding it to the server.ConfigandFallbackConfiggain aHostAddrfield.hostandhostaddrare parallel lists, andhostaddrdecides how many connections are described, as inpqConnectOptions2. Unlikeport, a single address is not broadcast over several hosts: when host names are supplied alongsidehostaddr, the two lists must have the same length, and any other count fails withcould not match N host names to M hostaddr valuesinstead of silently dialling one address for several hosts.hostoverridesPGHOSTand a host from the service file. That value supplies no host names, so it is not treated as a list and cannot produce the length error above.buildConnectOneConfigsdialshostaddrdirectly when it is set, skipping name resolution, and keepshostas theoriginalHostnameused for identity. A nonemptyhostaddris checked to be an IP literal first: a host name or a socket directory path is an error for that slot, not a fallback to name resolution or to a Unix domain socket.hostaddris always a TCP connection, so the Unix-domain-socket TLS exemption no longer applies when one is present.hostaddrelement means the slot carries no address, as inhostaddr=,127.0.0.1: the default host is tried first and the supplied address second..pgpasslookup useshostaddrwhen no host name was supplied, andPGHOSTADDRis read as the environment form ofhostaddr.hostaddrwithouthostis a complete target in libpq: there is no name to resolve and none to present. The default socket directory is therefore not substituted for the missing host name, which would otherwise become the TLS server name —Config.Hostis left empty.For the connection string above the parsed config is now:
Notes on libpq parity
Two limits, stated rather than glossed over:
hostaddrwithAI_NUMERICHOST, which on some platforms also accepts nonstandard IPv4 spellings: shorthand (127.1), hex (0x7f.0.0.1), and leading-zero forms (010.1.1.1, which macOS reads as10.1.1.1). Those are rejected here. The literal check is the conservative direction for a value documented as an address..pgpasspasswords for fallback hosts remain a pre-existing limitation, unchanged by this change.Tests
pgconn/config_test.go:TestParseConfigHostAddr(including the issue's URL),TestParseConfigHostAddrWithoutHost,TestParseConfigHostAddrWithHostFromEnvironment,TestParseConfigHostAddrFromEnvironment,TestParseConfigHostAddrWithEmptyElement,TestParseConfigHostAddrWithExplicitEmptyHost,TestParseConfigHostAddrUsesHostAddrForPassfile,TestParseConfigHostAddrCountMismatch.pgconn/pgconn_private_test.go:TestBuildConnectOneConfigsUsesHostAddrWithoutResolvingasserts the dial address, the retained host name, and thatLookupFuncis never called whenhostaddris set.TestBuildConnectOneConfigsRejectsNonNumericHostAddrcovers a host name and a socket directory path.Verification
The failures in
./pgconnare the ones that need a running PostgreSQL server, which this machine does not have. They are present onmasteras well, and the failing test names are identical before and after the change (96 top-level test names, 112 including subtests — the same set, counted two ways). That is not a substitute for a database-backed run or for CI, and the workflow run on this branch is still awaiting approval, so there is no CI result yet.AI Disclosure
Per the contributing guide, this is an AI-assisted proposal. The implementation, the tests, and the verification above were produced with AI coding agents — GLM-5.3 via the ZCode CLI, and GPT-6 Sol / GPT-6 Astra via ChatGPT — which also cross-reviewed each other's output. I drove the process (setting the goals and reviewing each round of output and the libpq comparison), but I am not yet able to answer detailed questions about the change unaided. Prompts and session logs are available on request.
Fixes #2655