Stop the browser lying about servers, and stop hosting from being taken away silently - #218
Merged
Conversation
Two ways the browser reported a server as absent when it was not. A player joined a server, opened the list while playing on it, and was told "no servers found, 2 did not respond" -- one of the two being the server they were at that moment standing in. browser_QueryServer skipped the server we are connected to, but the slot had already been marked AS_WAITINGFORREPLY by the caller. So it was not skipped, it was condemned: nothing was ever sent, so nothing could ever answer, and the row aged out on its own timeout and was counted as a failure. It is now queried like any other server. Its reply arrives from the same address as game traffic, so the client's packet loop tells them apart by whether we have an outstanding question for that sender -- the command alone will not do, because a game packet may begin with any bytes at all. The other way was quieter. A server running a different build is hidden by setting AS_INACTIVE, which is also what an empty slot looks like, so a server that answered us became indistinguishable from a slot nobody had used. The list simply got shorter. That is how one player insists a server exists while another cannot find it, and it produced exactly that report tonight about a Hexen session. Mismatches are now remembered and counted, and the footer says so. Both decisions moved into replyrouting_compute so they are covered rather than asserted: ten tests, including the case that a packet nobody asked for is never taken from the game parser, and that a wrong build is reported ahead of a timeout because it is the one a player can act on.
Deploying 0.1.1 stopped every server on the network verifying, and therefore stopped every one of them being listed. It was live for forty minutes. Two things were wrong, and only the first was the one I went looking for. The probe socket was written to and never read. recvfrom and select still polled the main socket alone, so a reply arriving on the probe socket was collected by nobody. Receiving is now split into a helper that either socket can be drained through, and both are in select's wait set. That alone would not have saved it. Servers refuse the request outright: they answer the registry they know, and SERVER_SERVERREGISTRY_IsAddress compares with NETADDRESS_s::Compare, which includes the port. A verification request from any other port of ours is dropped before it reaches a handler. Verification therefore CANNOT move off the announced port from the registry side alone -- it needs a change in the server, which would leave every server built before that change unlistable. So verification goes back to the main socket, with the reasoning written where the next person will be tempted. Honesty about reachability stays in the reach probe, which never needed the server's cooperation: the client listens on the port under test with a socket that has sent nothing, so no mapping exists for anything to arrive through. Tested end to end this time, against a real server rather than a passing build: local registry, real zandronum server, and the check is that it reaches the SERVER LIST, not merely the verification list. That distinction is the entire bug and no test covered it.
A download that stalls used to show a file name and a progress bar that stopped, with nothing to say whether the culprit was one bad mirror or the player's own connection. The console now names the source as each attempt begins, with its position in the candidate list, so a player watching it can see the search move on rather than guess whether anything is happening. Before it starts rather than after it succeeds, which is the whole point: the case that needs this is the one that never finishes. The host only, never the whole URL. A full URL wraps in the console and buries the one field being asked about, and a signed mirror URL can carry a token in its query string -- console logs get pasted into bug reports. DownloadSourceName strips the scheme, path, query and any credentials, and keeps the port, because two servers serving their own WADs from one address are told apart by nothing else. Five tests, including the credential and token cases.
Automatic port opening reported "your router will not open ports automatically" without the question having reached any router, and it would have said that to people whose routers were willing. Two faults, both found by running the search by hand next to the engine's. The SSDP search chose no interface, so it left by whichever one the routing table ranked first. On the machine this was written on that is never the LAN: five phantom Wi-Fi adapters on link-local addresses, a Bluetooth PAN and a Tailscale device all outrank it. An unbound search got ZERO replies where the identical search bound to the LAN address got dozens. It now binds and sets IP_MULTICAST_IF to the address that routes to the internet, which LocalAddressFor already works out without sending anything. A machine where that cannot be determined keeps the old behaviour rather than refusing to look. The first responder also won outright, and the first responder is not the router. A smart TV answers upnp:rootdevice repeatedly and eagerly; we took its description URL, found no WAN connection service in it, and concluded the network could not forward ports. Every responder is now collected and each is checked for a WAN service until one has it. Verified on this network: the search now finds a device, and it is not the gateway, which is precisely the case that used to end the search. Worth stating plainly, because it is the reason this went unnoticed: the router here offers neither UPnP nor NAT-PMP, so the message was true by accident while the code that produced it was wrong. Still outstanding: NAT-PMP is only ever aimed at a device that answered SSDP, so a router that speaks NAT-PMP and not UPnP remains invisible. Reaching it means reading the routing table on three platforms, and that is not something to write untested at the end of a night that already shipped one untested socket change.
The list looked like an aggressive cache. It was not: opening the browser already re-queries every listed server and asks the registry for new ones, keeping the rows on screen while it does. That is the right behaviour and it is completely invisible, which is indistinguishable from nothing happening -- so it read as stale data being served over and over. So the button does two jobs. It gives a way to ask, which was missing, and it REPORTS, which is what was actually wrong: an automatic refresh lights it up exactly as a pressed one does, reading CHECKING while any server is being re-queried or a registry query is outstanding. The complaint was never that the list was stale. It was that the work could not be seen. Bottom left, on the footer line, which is the one part of that row nothing else wants -- the footer text is centred. Its tooltip says what it does and, more usefully, that the list already refreshes itself when opened. Verified through the engine's own HUD dump: the button is painted at virtual (60, 336), bottom left, with the expected label. The click could NOT be exercised through the MCP bridge -- clicking an existing tab at known-good coordinates did not switch tabs either, only produced its hover tooltip, so the bridge is not delivering the release event this menu acts on. That is a harness limit, not a finding about this code: the hit test is the same shape as the tabs' and sits in the same responder, ahead of them. Worth a human press before it ships.
The IWAD picker searched Doomseeker's and GZDoom's folders only after the player had joined a server once, because that was the sole caller of RegisterKnownWadDirectories. The gap is invisible and it lands on exactly the wrong person: someone new, whose IWADs live in a Doomseeker library, with no Steam copy for the stock search to find. They get an empty picker on first launch, and then a full one later, because joining once writes those paths into their ini permanently. A list that appears to fix itself is worse than one that is simply wrong -- there is nothing to report and nothing to reproduce. So the directories are registered before FindIWAD runs as well. Registering in both places costs nothing: AddPathOnce is idempotent, and only directories that actually exist are ever added. Verified against a clean ini on a machine with Doomseeker installed and no prior join: both Doomseeker paths appear under [IWADSearch.Directories] after a single launch, and the two directories that do not exist here were correctly left out.
INTERNET now reads green when the probe arrived, red when the check ran and it did not, and plain white when we do not know. Untested, still running and FAILED all fall in that last group -- Failed especially, because it means the registry never answered, which is a fact about our service and not about the player's router. Painting their option red on the strength of our own outage would blame them for it, at the moment we are already the ones broken. Still selectable when red. The check can be wrong in a player's favour, and a form that refuses to let someone try their own network is worse than one that warns them. The running panel also loses two lines. The heading told the player they can run a server, which has stopped being news once theirs is up, so it is drawn on the form only. And the address line said 127.0.0.1 -- the one address that is useless to everyone except the person already looking at it, while inviting them to share it. Four tests on the display mapping, the load-bearing one being that Failed is not Unreachable.
Three ways to end a hosted server, none of which asked. The worst was joining your own. A host pressed JOIN on the row for the server they were already standing in, and the engine stopped that server, tore itself down for a WAD reload, and reconnected them to nothing. They asked to go where they already were and it cost them the server and their place in it. That now closes the menu and does nothing else, which is all "go where I already am" can honestly mean. Joining someone ELSE'S server while hosting, and starting a singleplayer game while hosting, both ask first. Neither did. Both disconnect whoever is playing on your server, which is not a thing to do silently on a keypress -- the HOST tab's STOP button has always asked, and these are the same act by another route. In both confirmations the stop runs BEFORE the thing the player asked for. Joining and starting a game each reload the engine and never return to the caller, so a stop deferred until afterwards never runs at all and the server outlives the menu that closed it. The choice is in joinintent_compute, with the ordering fixed by a test: AlreadyThere is checked ahead of the hosting warning, because both are true for a host on their own row, and asking whether to stop their server in order to go nowhere is the same bug wearing a dialog.
rc4l
enabled auto-merge
August 7, 2026 04:41
Every internet server in the browser showed "?" instead of a flag, and had done since the column existed. The GeoIP library is compiled in; a database never was. On Linux it could pick up /usr/share/GeoIP/GeoIP.dat if the distribution happened to install one, and everywhere else NETWORK_IsGeoIPAvailable was simply false, so NETWORK_GetCountryIndexFromAddress returned 0 for every address that was not on a LAN. The scoreboard kept working throughout and hid the problem: player flags arrive from the server as a country index over the wire, so they never needed a local database. The browser is the only place that geolocates for itself. Shipping the upstream answer was not available. MaxMind discontinued the legacy .dat format in 2019, so there is no current file to package even if the packaging step had remembered it. So the table is ours, built from DB-IP's free IP-to-Country data by tools/gen_geoip_table.py, and it lives as a lump INSIDE zandronum.pk3 rather than as a file beside the exe. That placement is the point: a loose data file next to the binary is exactly what a packaging step drops, which is the bug being fixed. The pk3 deflates it, so the download grows by about a megabyte rather than 1.8. Aggregating to /16 was measured first, because a 64 KB table would have been nearly free. It puts 6.8% of the address space in the wrong country and sends 1.1.1.1 to Thailand, so exact ranges it is. A real GeoIP database still wins when one is present. This is the fallback, not a replacement. Parsing is in geoiptable_compute with twelve tests, weighted towards refusing bad files rather than reading good ones: the count and the code count are read out of the file and then used to index, so a declared size larger than the file, one big enough to overflow the multiplication, a truncated file at every possible length, and an out-of-range per-entry code index are each covered. Verified against the shipped table: the two addresses from tonight's reports and the registry droplet all resolve to US, and 212.129.1.1 to FR, which the /16 version had in China.
The browser answers that question with twenty pixels of flag, which is not something anyone can check. When it was wrong for every server on the internet, there was no way to tell whether the lookup was broken, the database was missing, or the server had simply declined to say. fua_whereis prints the answer and, more usefully, says which source produced it: a system GeoIP database or the table shipped in zandronum.pk3. That distinction is the one that mattered here. Verified against the addresses from tonight: the reporter's own, the other player's, a French one, and a LAN address, all named correctly.
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
1 participant
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.
A night of bug reports from live play, each one traced to something we were doing rather than
something the network was doing.
The browser said servers were not there
A player opened the list while standing in a server and was told "no servers found, 2 did not
respond" -- one of the two being the server they were in.
browser_QueryServerskipped the serverwe are connected to, but the caller had already marked the slot as awaiting a reply, so it was not
skipped, it was condemned: nothing sent, nothing could answer, and it aged out as a failure.
Separately, servers running another build were hidden by setting
AS_INACTIVE, which is also whatan empty slot looks like. The list simply got shorter with no count and no explanation, which is how
one player insists a server exists while another cannot find it.
Hosting was taken away without asking
Joining your own server stopped it, reloaded the engine, and reconnected you to nothing. Joining
someone else's, or starting a singleplayer game, killed it silently. All three now behave: the first
just closes the menu, the other two ask first.
The registry told operators their port was open when it was not
Verification rode the NAT mapping the server's own announce had opened, so it arrived however closed
the port was. The attempted fix took the registry down for forty minutes, because servers reject a
request from any port but the announced one --
Compareincludes the port. That reasoning is nowrecorded where the next person will be tempted to redo it.
Automatic port opening never asked the router
The SSDP search chose no interface, so it left by whichever the routing table ranked first. On a
machine with a VPN or virtual adapters that is never the LAN: an unbound search got zero replies
where a bound one got dozens. And the first responder won outright, which on a home network is a
smart TV rather than the gateway.
Also
Downloads name their source as each attempt begins, host only, credentials and tokens stripped. The
IWAD picker now finds Doomseeker and GZDoom libraries on first launch instead of after your first
join. INTERNET reads green, red, or white, with a failed check deliberately white so our outage is
never drawn as the player's shut port. And a refresh button, which mostly exists to make visible the
refresh that was already happening silently.
Suite at 1392, with 27 new tests across four compute units.