fix(connections): keep the username empty when a connection URL has none#1860
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55231d7b4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| host: coordinator.network.resolvedHost, | ||
| port: coordinator.network.resolvedPort, | ||
| database: coordinator.network.database, | ||
| username: PgpassReader.effectiveUsername(resolvedUsername) |
There was a problem hiding this comment.
Use the effective pgpass user when resolving passwords
This makes the form check ~/.pgpass with the OS username when the username field is blank, but the actual connection path still calls PgpassReader.resolve(... username: connection.username) in TablePro/Core/Database/DatabaseDriver.swift:547-552. For PostgreSQL connections using ~/.pgpass through an SSH/Cloudflare/Cloud SQL tunnel, the app must resolve the password itself against the original host; with a blank saved username it now reports a match in the form, then looks up an empty username during connect and passes no password, so the connection can fail despite the green status.
Useful? React with 👍 / 👎.
Problem
Reported by a user:
Import
mysql://localhost:3306/shopand the Username field ends up asroot. TablePlus leaves it empty.Root cause
The username you type is never what gets saved.
ConnectionFormCoordinatorrewrote a blank username to the literal"root"in two places,save()andtest(), whenever the plugin reportedrequiresAuthentication— which is the protocol default, so it fired for MySQL, PostgreSQL, ClickHouse and nearly everything else. The field's placeholder was also the hardcoded literalText("root"), and the model injected"root"as its init default and Codable fallback.Everything around this was already correct, which is the tell that the fabrication was the anomaly: the URL parser returns
""for a no-user URL (with a test asserting exactly that), the URL formatter omitsuser@when empty, and the deeplink exporter andpg_dumpbuilder both already skip an empty username.Why empty is the correct value to store
An empty username is not a workaround, it is the documented "use the default user" signal in every client library we link:
mysql_real_connect):NULLand""are identical.if (mysql->user && mysql->user[0]) ... else read_user_name(end)sends the macOS login name. It never transmits an empty user, so there is no anonymous-auth hazard.connectOptions2()replaces an emptypguserwithpg_fe_getauthname(), and does so before matching.pgpass. Our driver already omits theuser=keyword when empty.nil. Redis has no username before 6.0, and sendingdefaultexplicitly is known to hang some servers.Changes
ConnectionFormCoordinatorno longer substitutes"root". Bothsave()andtest()were re-deriving host/port/username inline, which is exactly why the bug existed twice, so the resolution now lives once on the view models:NetworkPaneViewModel.resolvedHost/.resolvedPortandAuthPaneViewModel.resolvedUsername.DatabaseConnectiondefaults the username to""in both its initializer and its Codable decode.GeneralPaneViewdrops therootplaceholder. On Postgres it told the user the opposite of what would actually happen.PgpassReader.effectiveUsername(_:)resolves a blank username toNSUserName(), matching libpq. Previously a blank field looked uprootin~/.pgpassand reported "no match" even when a real connection would have succeeded.Basic base64("user:pass")unconditionally, so a blank username would have gone out as:pass. NewClickHouseCredentialshelper resolves blank todefault, ClickHouse's documented default user. (It was previously sendingroot, which is wrong for ClickHouse regardless.)Existing saved connections are untouched. Anyone who already saved a blank username has
rooton disk and possibly a working connection. Rewriting that on their behalf would be a worse bug than the one being fixed.Tests
13 new tests across four files:
DatabaseConnectionUsernameTests— init default and decode-without-key both yield an empty username; empty round-trips through encoding.ConnectionURLImportUsernameTests— a MySQL/Postgres URL with no user info imports with an empty username and exports back to a URL with nouser@; a URL with user info still keeps it.PgpassReaderUsernameTests— blank resolves to the OS user, explicit is preserved.ClickHouseCredentialsTests— blank resolves todefault, and the Basic header decodes todefault:secret.No UI automation:
TableProUITestscurrently holds onlyTableProLaunchUITests.swift, so there is no connection-form harness to extend.Verification
swiftlint lint --strictclean on all changed files.ClickHouseDrivertarget builds. Manual check: importmysql://localhost:3306/somedb, the Username field is blank and stays blank through Save and reopen.