From 9e4c59a44e2a326071a030d54908a5aaf2dbbc84 Mon Sep 17 00:00:00 2001
From: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
Date: Fri, 8 Nov 2024 21:49:41 +0300
Subject: [PATCH] netplay: more strict checks for error codes from socket
 operations

There isn't any guarantee that `getSockErr()`/`setSockErr()`
won't return `0` in all corner cases where something bad happens
with an underlying socket.

Provide more strict checks for socket write errors and for
checking the result of connection opening routines, allowing to
catch error conditions even in presence of error code == 0.

Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
---
 lib/netplay/netsocket.cpp     |  8 ++++----
 lib/netplay/netsocket.h       | 12 ++++++------
 src/screens/joiningscreen.cpp |  5 +++--
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/lib/netplay/netsocket.cpp b/lib/netplay/netsocket.cpp
index 47a45a053a0..b449acb9783 100644
--- a/lib/netplay/netsocket.cpp
+++ b/lib/netplay/netsocket.cpp
@@ -72,7 +72,7 @@ struct Socket
 
 	SOCKET fd[SOCK_COUNT];
 	bool ready;
-	std::error_code writeErrorCode = make_network_error_code(0);
+	nonstd::optional<std::error_code> writeErrorCode;
 	bool deleteLater;
 	char textAddress[40] = {};
 
@@ -650,9 +650,9 @@ net::result<ssize_t> writeAll(Socket& sock, const void *buf, size_t size, size_t
 		return tl::make_unexpected(make_network_error_code(EBADF));
 	}
 
-	if (sock.writeErrorCode)
+	if (sock.writeErrorCode.has_value())
 	{
-		return tl::make_unexpected(sock.writeErrorCode);
+		return tl::make_unexpected(sock.writeErrorCode.value());
 	}
 
 	if (size > 0)
@@ -731,7 +731,7 @@ void socketFlush(Socket& sock, uint8_t player, size_t *rawByteCount)
 		return;  // Not compressed, so don't mess with zlib.
 	}
 
-	ASSERT(!sock.writeErrorCode, "Socket write error?? (Player: %" PRIu8 "", player);
+	ASSERT(!sock.writeErrorCode.has_value(), "Socket write error?? (Player: %" PRIu8 "", player);
 
 	// Flush data out of zlib compression state.
 	do
diff --git a/lib/netplay/netsocket.h b/lib/netplay/netsocket.h
index 32fff7019d8..a660a744645 100644
--- a/lib/netplay/netsocket.h
+++ b/lib/netplay/netsocket.h
@@ -26,6 +26,7 @@
 #include <system_error>
 #include <vector>
 
+#include <nonstd/optional.hpp>
 #include <tl/expected.hpp>
 
 namespace net
@@ -137,7 +138,6 @@ int checkSockets(const SocketSet& set, unsigned int timeout); ///< Checks which
 // Higher-level functions for opening a connection / socket
 struct OpenConnectionResult
 {
-public:
 	OpenConnectionResult(std::error_code ec, std::string errorString)
 	: errorCode(ec)
 	, errorString(errorString)
@@ -146,19 +146,19 @@ struct OpenConnectionResult
 	OpenConnectionResult(Socket* open_socket)
 	: open_socket(open_socket)
 	{ }
-public:
-	bool hasError() const { return static_cast<bool>(errorCode); }
-public:
+
+	bool hasError() const { return errorCode.has_value(); }
+
 	OpenConnectionResult( const OpenConnectionResult& other ) = delete; // non construction-copyable
 	OpenConnectionResult& operator=( const OpenConnectionResult& ) = delete; // non copyable
 	OpenConnectionResult(OpenConnectionResult&&) = default;
 	OpenConnectionResult& operator=(OpenConnectionResult&&) = default;
-public:
+
 	struct SocketDeleter {
 		void operator()(Socket* b) { if (b) { socketClose(b); } }
 	};
 	std::unique_ptr<Socket, SocketDeleter> open_socket;
-	std::error_code errorCode;
+	nonstd::optional<std::error_code> errorCode;
 	std::string errorString;
 };
 typedef std::function<void (OpenConnectionResult&& result)> OpenConnectionToHostResultCallback;
diff --git a/src/screens/joiningscreen.cpp b/src/screens/joiningscreen.cpp
index c3ad785edb4..1f18c884aff 100644
--- a/src/screens/joiningscreen.cpp
+++ b/src/screens/joiningscreen.cpp
@@ -1149,8 +1149,9 @@ void WzJoiningGameScreen_HandlerRoot::processOpenConnectionResult(size_t connect
 		{
 			debug(LOG_ERROR, "%s", result.errorString.c_str());
 			// Done trying connections - all failed
-			const auto sockErrorMsg = result.errorCode.message();
-			auto localizedError = astringf(_("Failed to open connection: [%d] %s"), result.errorCode.value(), sockErrorMsg.c_str());
+			const auto errCode = result.errorCode.value();
+			const auto sockErrorMsg = errCode.message();
+			auto localizedError = astringf(_("Failed to open connection: [%d] %s"), errCode.value(), sockErrorMsg.c_str());
 			handleFailure(FailureDetails::makeFromInternalError(WzString::fromUtf8(localizedError)));
 		}
 		return;