Fix a couple of endianness issues with plNetAddress
#1500
Merged
Conversation
This file contains 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
The srvAddr field is transmitted little-endian and the NetCli receiving logic converts it to host byte order (also little-endian usually), but SetHost expects a big-endian value. This change fixes auth server reconnects for servers that send Auth2Cli_ServerAddr messages (i. e. Cyan's server and MOSS).
fHost is always stored big-endian in memory, regardless of the host endianness. It's thus incorrect to read/write it using ReadLE32/WriteLE32, because then the byte order in the stream changes depending on the host byte order! On little-endian systems, fHost has always been read/written big-endian (because ReadLE32/WriteLE32 don't swap anything on little-endian systems). To make big-endian systems behave the same way, the field needs to be read/written as-is with no endianness conversion.
dpogue
reviewed
Oct 8, 2023
| @@ -1861,7 +1861,7 @@ bool RecvMsg<Auth2Cli_ServerAddr>(const uint8_t in[], unsigned, void*) | |||
| hsLockGuard(s_critsect); | |||
| if (s_active) { | |||
| s_active->token = msg.token; | |||
| s_active->addr.SetHost(msg.srvAddr); | |||
| s_active->addr.SetHost(hsToBE32(msg.srvAddr)); | |||
There was a problem hiding this comment.
I'm guessing we need to byteswap here because the server is (incorrectly) sending the address in LE rather than network order?
There was a problem hiding this comment.
Well, it's "correct" in the sense that it's consistent with the rest of the auth server protocol - IPv4 addresses are treated as integers, so they are transmitted little-endian. It's definitely a weird choice though.
An extra twist is that the NetCli receive machinery also does an implicit endianness conversion before the message gets here. So the IPv4 address is received little-endian, then implicitly converted to host byte order, and finally explicitly converted to big-endian. Fun!
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.
This fixes auth server reconnects for servers that send
Auth2Cli_ServerAddrmessages (i. e. Cyan's server and MOSS). Previously, the received IPv4 address was byte-swapped, making the reconnect always fail.