fix(connection): a blank port means the driver's default, and the SQL Browser lookup finally reaches the Browser - #170
Merged
Conversation
… Browser lookup finally reaches the Browser A connection saved without a port was dialled verbatim. `profile.port` is a `u16`, a blank port field stores `0`, and every consumer read it literally: `build_url` wrote `postgres://host:0/db`, the Mongo fallback assembled `mongodb://host:0`, `tiberius` was handed `Config::port(0)`, and `ssh::open_tunnel` forwarded to a remote port of zero. The user got a connection refused naming a port they never typed. Nothing can listen on port zero, so the value is free to mean "no port was chosen" — which is what it already was, arriving from three places the dialog never touches: a `--port`-less CLI launch, an imported `.json`, and a profile synced from a shared origin. `Driver::default_port` and `ConnectionProfile::effective_port` resolve it at connect time; `effectivePort` is the frontend's copy, used by every surface that prints a port. Resolving rather than rewriting is the design decision. Filling the default into the field on save — HeidiSQL's behaviour — was rejected: `profiles.json` would record a number the user never entered, an origin would propagate it to every consumer, and the profile would freeze today's default if it ever moved. Two placements matter: the resolution happens *before* the SSH branch, because a tunnel's remote port is as real as the one a URL names, and inside `EndpointKey`, or a blank-port profile and a `5432` one would hold two budgets against one server. Chasing the same symptom on SQL Server turned up a second, larger bug. `tiberius`' `connect_named` sends its SSRP discovery datagram to `Config::get_addr()` — the config's *own* port — and only rewrites the address once the Browser has replied. `build_config` has always called `cfg.port(port)`, and must, because that port is what `Reach::Browser`'s fallback connects to. So every named-instance lookup was addressed to the instance's TCP port, where no UDP listener has ever been, timed out after `tiberius`' one second, and fell through. The SQL Browser path therefore never worked at all: instances that connected did so through the static-port fallback, and an instance on a dynamic port could not be reached by any route. The tell was a user typing `1434` — the Browser's own port — into the port field and watching the connection succeed, because that was the only value that made the datagram land where it was addressed. `discovery_config` clones the login config onto `SQL_BROWSER_PORT` for the lookup; the login keeps the typed port, so it still serves as the fallback target. A port equal to the default is still not treated as a static-port signal, blank or typed — retrying 1433 after the Browser went quiet buys a second timeout and nothing else. The dialog's port field now shows the driver's default as its placeholder, so an empty field reads as a choice with a known outcome, and `ConnectionRailRow` prints the resolved port instead of `SVRSQL3:0`. Recorded as ADR gotcha #89, because the `tiberius` behaviour is not discoverable from its documentation and will bite again. Authored by Alex López (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Authored by Alex López (Alexfp28)
<alexlopezdelafuente@gmail.com>.What
Two bugs that share a symptom: a connection that would not open because of a port the user never typed.
A blank port meant port zero
profile.portis au16and an empty port field stores0, which every consumer read literally —postgres://host:0/db,mongodb://host:0,Config::port(0), and an SSH tunnel forwarding to remote port zero. Nothing can listen on port zero, so the value is now free to mean what it already meant: no port was chosen.Driver::default_port/ConnectionProfile::effective_portresolve it at connect time,effectivePortdoes the same on the frontend for every surface that prints one.It is resolved, never written back. Filling the default into the field on save (HeidiSQL's behaviour) was considered and rejected:
profiles.jsonwould record a number the user never entered, a shared origin would propagate it to every consumer, and the profile would freeze today's default if it ever moved.Two placements are load-bearing: the resolution happens before the SSH branch, because a tunnel's remote port is as real as the one a URL names, and inside
EndpointKey, or a blank-port profile and a5432one would hold two separate budgets against the same server.The SQL Server Browser lookup never reached the Browser
Found while chasing the same symptom, and the larger of the two.
tiberius'connect_namedsends its SSRP discovery datagram toConfig::get_addr()— the config's own port — and only rewrites the address once the Browser replies.build_confighas always calledcfg.port(port), and must, because that port is whatReach::Browser's fallback connects to. So every named-instance lookup was addressed to the instance's TCP port, where no UDP listener has ever been, timed out aftertiberius' one second, and fell through.The SQL Browser path therefore never worked: named instances that connected did so through the static-port fallback, and an instance on a dynamic port could not be reached by any route. The tell was typing
1434— the Browser's own port — into the port field and watching the connection succeed, because that was the only value that made the datagram land where it was addressed.discovery_configclones the login config ontoSQL_BROWSER_PORTfor the lookup; the login keeps the typed port, so it still serves as the fallback target.Also
ConnectionRailRowprints the resolved port instead ofSVRSQL3:0.tiberiusbehaviour is not discoverable from its documentation and will bite again.Tests
cargo test --all-features: 796 passing, including new coverage foreffective_portper driver,EndpointKeysharing a budget across a blank and an explicit port, and the Browser lookup's destination (the_browser_lookup_goes_to_the_browser_not_to_the_instance).pnpm test: 1339 passing, includingeffectivePort.tsc --noEmit,cargo fmt,cargo clippy --all-features --all-targets: clean.