net: add new method Sock::Listen() that wraps listen()

vasild · vasild · commit b2733ab6a85b · 2022-05-18T16:40:13.000+02:00
This will help to increase `Sock` usage and make more code mockable.
diff --git a/src/net.cpp b/src/net.cpp
@@ -2408,7 +2408,7 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,
     LogPrintf("Bound to %s\n", addrBind.ToString());
 
     // Listen for incoming connections
-    if (listen(sock->Get(), SOMAXCONN) == SOCKET_ERROR)
+    if (sock->Listen(SOMAXCONN) == SOCKET_ERROR)
     {
         strError = strprintf(_("Error: Listening for incoming connections failed (listen returned error %s)"), NetworkErrorString(WSAGetLastError()));
         LogPrintf("%s\n", strError.original);
diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp
@@ -180,6 +180,25 @@ int FuzzedSock::Bind(const sockaddr*, socklen_t) const
     return 0;
 }
 
+int FuzzedSock::Listen(int) const
+{
+    // Have a permanent error at listen_errnos[0] because when the fuzzed data is exhausted
+    // SetFuzzedErrNo() will always set the global errno to listen_errnos[0]. We want to
+    // avoid this method returning -1 and setting errno to a temporary error (like EAGAIN)
+    // repeatedly because proper code should retry on temporary errors, leading to an
+    // infinite loop.
+    constexpr std::array listen_errnos{
+        EADDRINUSE,
+        EINVAL,
+        EOPNOTSUPP,
+    };
+    if (m_fuzzed_data_provider.ConsumeBool()) {
+        SetFuzzedErrNo(m_fuzzed_data_provider, listen_errnos);
+        return -1;
+    }
+    return 0;
+}
+
 std::unique_ptr<Sock> FuzzedSock::Accept(sockaddr* addr, socklen_t* addr_len) const
 {
     constexpr std::array accept_errnos{
diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h
@@ -66,6 +66,8 @@ class FuzzedSock : public Sock
 
     int Bind(const sockaddr*, socklen_t) const override;
 
+    int Listen(int backlog) const override;
+
     std::unique_ptr<Sock> Accept(sockaddr* addr, socklen_t* addr_len) const override;
 
     int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const override;
diff --git a/src/test/util/net.h b/src/test/util/net.h
@@ -129,6 +129,8 @@ class StaticContentsSock : public Sock
 
     int Bind(const sockaddr*, socklen_t) const override { return 0; }
 
+    int Listen(int) const override { return 0; }
+
     std::unique_ptr<Sock> Accept(sockaddr* addr, socklen_t* addr_len) const override
     {
         if (addr != nullptr) {
diff --git a/src/util/sock.cpp b/src/util/sock.cpp
@@ -79,6 +79,11 @@ int Sock::Bind(const sockaddr* addr, socklen_t addr_len) const
     return bind(m_socket, addr, addr_len);
 }
 
+int Sock::Listen(int backlog) const
+{
+    return listen(m_socket, backlog);
+}
+
 std::unique_ptr<Sock> Sock::Accept(sockaddr* addr, socklen_t* addr_len) const
 {
 #ifdef WIN32
diff --git a/src/util/sock.h b/src/util/sock.h
@@ -103,6 +103,12 @@ class Sock
      */
     [[nodiscard]] virtual int Bind(const sockaddr* addr, socklen_t addr_len) const;
 
+    /**
+     * listen(2) wrapper. Equivalent to `listen(this->Get(), backlog)`. Code that uses this
+     * wrapper can be unit tested if this method is overridden by a mock Sock implementation.
+     */
+    [[nodiscard]] virtual int Listen(int backlog) const;
+
     /**
      * accept(2) wrapper. Equivalent to `std::make_unique<Sock>(accept(this->Get(), addr, addr_len))`.
      * Code that uses this wrapper can be unit tested if this method is overridden by a mock Sock

Original file line number	Diff line number	Diff line change
`@@ -2408,7 +2408,7 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,`
`2408`	`2408`	`LogPrintf("Bound to %s\n", addrBind.ToString());`
`2409`	`2409`
`2410`	`2410`	`// Listen for incoming connections`
`2411`		`- if (listen(sock->Get(), SOMAXCONN) == SOCKET_ERROR)`
	`2411`	`+ if (sock->Listen(SOMAXCONN) == SOCKET_ERROR)`
`2412`	`2412`	`{`
`2413`	`2413`	`strError = strprintf(_("Error: Listening for incoming connections failed (listen returned error %s)"), NetworkErrorString(WSAGetLastError()));`
`2414`	`2414`	`LogPrintf("%s\n", strError.original);`
Original file line number	Diff line number	Diff line change
`@@ -129,6 +129,8 @@ class StaticContentsSock : public Sock`
`129`	`129`
`130`	`130`	`int Bind(const sockaddr*, socklen_t) const override { return 0; }`
`131`	`131`
	`132`	`+ int Listen(int) const override { return 0; }`
	`133`	`+`
`132`	`134`	`std::unique_ptr<Sock> Accept(sockaddr* addr, socklen_t* addr_len) const override`
`133`	`135`	`{`
`134`	`136`	`if (addr != nullptr) {`
Original file line number	Diff line number	Diff line change
`@@ -79,6 +79,11 @@ int Sock::Bind(const sockaddr* addr, socklen_t addr_len) const`
`79`	`79`	`return bind(m_socket, addr, addr_len);`
`80`	`80`	`}`
`81`	`81`
	`82`	`+int Sock::Listen(int backlog) const`
	`83`	`+{`
	`84`	`+ return listen(m_socket, backlog);`
	`85`	`+}`
	`86`	`+`
`82`	`87`	`std::unique_ptr<Sock> Sock::Accept(sockaddr* addr, socklen_t* addr_len) const`
`83`	`88`	`{`
`84`	`89`	`#ifdef WIN32`