ServerSocket shall not throw (EOL) on accept returning nullptr; Simpl…

…ify accept. Have `ServerSocket::accept` determine whether to `Util::forcedExit(EX_SOFTWARE)` on unrecoverable errors: - Exit application if `::accept` returns `errno` other than EINTR || EAGAIN || EWOULDBLOCK. - tolerate `Socket` ctor exceptions - tolerate `::accept` EINTR || EAGAIN || EWOULDBLOCK - Note, the following are `no throw` - `::close` - `::inet_ntop` Also reject exceeding external connections right within `ServerSocket::accept`, avoiding creation of `Socket` objects - hence saving resources. Signed-off-by: Sven Göthel <[email protected]> Change-Id: I939df8e8462d2c29d19214a9b5fb70ad37dc7500
CollaboraOnline · Nov 1, 2024 · 641a07d · 641a07d
1 parent 57f6414
commit 641a07d
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 45 deletions.
diff --git a/net/ServerSocket.hpp b/net/ServerSocket.hpp
@@ -104,18 +104,11 @@ class ServerSocket : public Socket
         if (events & POLLIN)
         {
             std::shared_ptr<Socket> clientSocket = accept();
-            if (!clientSocket)
+            if (clientSocket)
             {
-                const std::string msg = "Failed to accept. (errno: ";
-                throw std::runtime_error(msg + std::strerror(errno) + ')');
-            }
-            const size_t extConnCount = StreamSocket::getExternalConnectionCount();
-            if( 0 == net::Defaults.maxExtConnections || extConnCount <= net::Defaults.maxExtConnections )
-            {
-                LOG_TRC("Accepted client #" << clientSocket->getFD());
+                LOGA_TRC(Socket, "Accepted client #" << clientSocket->getFD() << ", " << *clientSocket);
                 _clientPoller.insertNewSocket(std::move(clientSocket));
-            } else
-                LOG_WRN("Limiter rejected extConn[" << extConnCount << "/" << net::Defaults.maxExtConnections << "]: " << *clientSocket);
+            }
         }
     }
 

diff --git a/net/Socket.cpp b/net/Socket.cpp
@@ -15,6 +15,7 @@
 #include "TraceEvent.hpp"
 #include "Util.hpp"
 
+#include <cerrno>
 #include <chrono>
 #include <cstring>
 #include <cctype>
@@ -26,6 +27,7 @@
 #include <cstdio>
 #include <string>
 #include <unistd.h>
+#include <sysexits.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/un.h>
@@ -1161,50 +1163,66 @@ std::shared_ptr<Socket> ServerSocket::accept()
 #else
     const int rc = fakeSocketAccept4(getFD());
 #endif
-    LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
-    try
+    if (rc < 0)
     {
-        // Create a socket object using the factory.
-        if (rc != -1)
+        const int cause = errno;
+        if( cause != EINTR && cause != EAGAIN && cause != EWOULDBLOCK )
         {
-#if !MOBILEAPP
-            char addrstr[INET6_ADDRSTRLEN];
+            // Unrecoverable error, throw, which will close `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`
+            constexpr const char * messagePrefix = "Failed to accept. (errno: ";
+            LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
+            Util::forcedExit(EX_SOFTWARE);
+        }
+        return nullptr;
+    }
+    LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
 
-            Socket::Type type;
-            const void *inAddr;
-            if (clientInfo.sin6_family == AF_INET)
-            {
-                struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
-                inAddr = &(ipv4->sin_addr);
-                type = Socket::Type::IPv4;
-            }
-            else
-            {
-                struct sockaddr_in6 *ipv6 = &clientInfo;
-                inAddr = &(ipv6->sin6_addr);
-                type = Socket::Type::IPv6;
-            }
+#if !MOBILEAPP
+    char addrstr[INET6_ADDRSTRLEN];
 
-            std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
+    Socket::Type type;
+    const void *inAddr;
+    if (clientInfo.sin6_family == AF_INET)
+    {
+        struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
+        inAddr = &(ipv4->sin_addr);
+        type = Socket::Type::IPv4;
+    }
+    else
+    {
+        struct sockaddr_in6 *ipv6 = &clientInfo;
+        inAddr = &(ipv6->sin6_addr);
+        type = Socket::Type::IPv6;
+    }
+    ::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
 
-            ::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
-            _socket->setClientAddress(addrstr, clientInfo.sin6_port);
+    const size_t extConnCount = StreamSocket::getExternalConnectionCount();
+    if( 0 < net::Defaults.maxExtConnections && extConnCount >= net::Defaults.maxExtConnections )
+    {
+        LOG_WRN("Limiter rejected extConn[" << extConnCount << "/" << net::Defaults.maxExtConnections << "]: #"
+                << rc << " has family "
+                << clientInfo.sin6_family << ", address " << addrstr << ":" << clientInfo.sin6_port);
+        ::close(rc);
+        return nullptr;
+    }
+    try
+    {
+        // Create a socket object using the factory.
+        std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
+        _socket->setClientAddress(addrstr, clientInfo.sin6_port);
 
-            LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
-                                        << clientInfo.sin6_family << ", " << *_socket);
-#else
-            std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
-#endif
-            return _socket;
-        }
-        return std::shared_ptr<Socket>(nullptr);
+        LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
+                                    << clientInfo.sin6_family << ", " << *_socket);
+        return _socket;
     }
     catch (const std::exception& ex)
     {
         LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
     }
-
     return nullptr;
+#else
+    return createSocketFromAccept(rc, Socket::Type::Unix);
+#endif
 }
 
 #if !MOBILEAPP
@@ -1250,11 +1268,21 @@ bool Socket::isLocal() const
 std::shared_ptr<Socket> LocalServerSocket::accept()
 {
     const int rc = ::accept4(getFD(), nullptr, nullptr, SOCK_NONBLOCK | SOCK_CLOEXEC);
+    if (rc < 0)
+    {
+        const int cause = errno;
+        if( cause != EINTR && cause != EAGAIN && cause != EWOULDBLOCK )
+        {
+            // Unrecoverable error, throw, which will close `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`
+            constexpr const char * messagePrefix = "Failed to accept. (errno: ";
+            LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
+            Util::forcedExit(EX_SOFTWARE);
+        }
+        return nullptr;
+    }
     try
     {
         LOG_DBG("Accepted prisoner socket #" << rc << ", creating socket object.");
-        if (rc < 0)
-            return std::shared_ptr<Socket>(nullptr);
 
         std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
         // Sanity check this incoming socket
@@ -1304,8 +1332,8 @@ std::shared_ptr<Socket> LocalServerSocket::accept()
     catch (const std::exception& ex)
     {
         LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
-        return std::shared_ptr<Socket>(nullptr);
     }
+    return nullptr;
 }
 
 /// Returns true on success only.