Skip to content

Commit

Permalink
merge bitcoin#24378: make bind() and listen() mockable/testable
Browse files Browse the repository at this point in the history
  • Loading branch information
kwvg committed Jun 11, 2024
1 parent be19868 commit c24804c
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 3 deletions.
5 changes: 2 additions & 3 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3246,8 +3246,7 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,
#endif
}

if (::bind(sock->Get(), (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR)
{
if (sock->Bind(reinterpret_cast<struct sockaddr*>(&sockaddr), len) == SOCKET_ERROR) {
int nErr = WSAGetLastError();
if (nErr == WSAEADDRINUSE)
strError = strprintf(_("Unable to bind to %s on this computer. %s is probably already running."), addrBind.ToString(), PACKAGE_NAME);
Expand All @@ -3259,7 +3258,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);
Expand Down
39 changes: 39 additions & 0 deletions src/test/fuzz/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,45 @@ int FuzzedSock::Connect(const sockaddr*, socklen_t) const
return 0;
}

int FuzzedSock::Bind(const sockaddr*, socklen_t) const
{
// Have a permanent error at bind_errnos[0] because when the fuzzed data is exhausted
// SetFuzzedErrNo() will always set the global errno to bind_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 bind_errnos{
EACCES,
EADDRINUSE,
EADDRNOTAVAIL,
EAGAIN,
};
if (m_fuzzed_data_provider.ConsumeBool()) {
SetFuzzedErrNo(m_fuzzed_data_provider, bind_errnos);
return -1;
}
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{
Expand Down
4 changes: 4 additions & 0 deletions src/test/fuzz/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ class FuzzedSock : public Sock

int Connect(const sockaddr*, socklen_t) const override;

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;
Expand Down
4 changes: 4 additions & 0 deletions src/test/util/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ class StaticContentsSock : public Sock

int Connect(const sockaddr*, socklen_t) const override { return 0; }

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) {
Expand Down
10 changes: 10 additions & 0 deletions src/util/sock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ int Sock::Connect(const sockaddr* addr, socklen_t addr_len) const
return connect(m_socket, addr, addr_len);
}

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
Expand Down
12 changes: 12 additions & 0 deletions src/util/sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,18 @@ class Sock
*/
[[nodiscard]] virtual int Connect(const sockaddr* addr, socklen_t addr_len) const;

/**
* bind(2) wrapper. Equivalent to `bind(this->Get(), addr, addr_len)`. Code that uses this
* wrapper can be unit tested if this method is overridden by a mock Sock implementation.
*/
[[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
Expand Down

0 comments on commit c24804c

Please sign in to comment.