refactor(connection): delegate URI parsing to tokio_postgres::Config#731
refactor(connection): delegate URI parsing to tokio_postgres::Config#731
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #731 +/- ##
=======================================
Coverage ? 68.75%
=======================================
Files ? 46
Lines ? 31061
Branches ? 0
=======================================
Hits ? 21354
Misses ? 9707
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
REV16/16 CI green including both Windows targets (second push fixed the unreachable-pattern warning from the C5 fix verifiedBefore: Code reviewScope is clean.
Test update — multi-host URI port assignment now matches real libpq: a portless host gets 5432, not the last explicitly-seen port. Verified with psql. 1702/1702 unit tests pass. |
Replace the hand-rolled parse_uri() implementation with delegation to tokio_postgres::Config::from_str(), which handles all standard libpq URI parameters including multi-host, Unix socket paths, percent-encoding, sslmode, connect_timeout, options, target_session_attrs, and query-string host=/port= params (fixing the C5 silent-drop bug). The only params tokio-postgres omits are the three TLS file params (sslcert, sslkey, sslrootcert) because it delegates TLS entirely to the caller. These are extracted from the raw URI via extract_ssl_file_params() before the sanitized copy is handed to tokio-postgres. rpg-extended sslmode values (allow, verify-ca, verify-full) and target_session_attrs values (primary, standby, prefer-standby) are also captured from the raw URI before sanitization. As a side effect, multi-host URI port assignment now matches real libpq: a host without an explicit port gets 5432, not the last explicitly-seen port. The test expectation is updated accordingly.
…-unix On Windows, tokio_postgres::config::Host has no Unix variant so the cfg(not(unix)) catch-all triggered an unreachable-patterns warning (promoted to error by -D warnings). Drop the dead arm entirely.
NikolayS
force-pushed
the
refactor/uri-use-tokio-config
branch
from
ec8f4f2 to
3d5884e
Compare
2 participants
Summary
Replaces the hand-rolled
parse_uri()implementation with delegation totokio_postgres::Config::from_str().Motivation
The custom URI parser had a bug (C5 from the #709 audit):
host=andport=in the URI query string fell through a_ => {}catch-all and were silently discarded. Rather than patch individual params, this replaces the parser wholesale with the battle-tested tokio-postgres implementation.What changes
New helpers:
extract_ssl_file_params()— extractssslcert,sslkey,sslrootcertfrom URI or connstring. These are the only params tokio-postgres omits (it delegates TLS entirely to the caller).sanitize_uri_for_tokio()— strips/rewrites params tokio-postgres doesn't recognise (sslcert,sslkey,sslrootcert) and maps rpg-extendedsslmode/target_session_attrsvalues to accepted equivalents before parsing.parse_uri()internals replaced:tokio_postgres::Config::from_str()on the sanitized URIUriParamsfrom the parsedConfigsslmode/target_session_attrsvalues from the raw URI before sanitizationUriParams struct and all
resolve_*callers: unchanged.Bug fixed
C5 (#709):
postgres://host/db?host=socket_dir&port=5433— the query-stringhostandportparams were silently dropped by the old parser. tokio-postgres handles these correctly.Behavior change
Multi-host URI port assignment now matches real libpq behavior: a host without an explicit port gets the default (5432), not the last explicitly-seen port. Verified with psql:
postgresql://localhost:5432,localhost:5433,localhost/postgrestries the third host on port 5432. Test updated accordingly.Tests
1702/1702 unit tests pass. Existing
parse_uritest coverage exercises the refactored path.Closes #709 (C5 item).