Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use random ports for network client #124

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Use random ports for network client #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f2e7fa
Use random ports for connection
cyberodactyl Aug 17, 2024
c095789
Merge branch 'refs/heads/master' into issue-48-net-client-random-ports
cyberodactyl Aug 17, 2024
5103b1d
Merge with UPnP
cyberodactyl Aug 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 73 additions & 20 deletions src/C4Network2IO.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,28 +114,10 @@ bool C4Network2IO::Init(std::shared_ptr<spdlog::logger> logger, const std::uint1
}

// initialize net i/o classes: TCP first
pNetIO_TCP = CreateNetIO(this->logger, "TCP I/O", new C4NetIOTCP{}, iPortTCP, Thread);
if (pNetIO_TCP)
{
pNetIO_TCP->SetCallback(this);

if (Config.Network.EnableUPnP)
{
UPnP->AddMapping(P_TCP, iPortTCP, 0);
}
}
pNetIO_TCP = CreateSetupNetIO(P_TCP, "TCP I/O", new C4NetIOTCP{}, Thread, iPortTCP);

// then UDP
pNetIO_UDP = CreateNetIO(this->logger, "UDP I/O", new C4NetIOUDP{}, iPortUDP, Thread);
if (pNetIO_UDP)
{
pNetIO_UDP->SetCallback(this);

if (Config.Network.EnableUPnP)
{
UPnP->AddMapping(P_UDP, iPortUDP, 0);
}
}
pNetIO_UDP = CreateSetupNetIO(P_UDP, "UDP I/O", new C4NetIOUDP{}, Thread, iPortUDP);

// no protocols?
if (!pNetIO_TCP && !pNetIO_UDP)
Expand Down Expand Up @@ -473,6 +455,40 @@ bool C4Network2IO::IsPuncherAddr(const C4NetIO::addr_t &addr) const
(!PuncherAddrIPv6.IsNull() && PuncherAddrIPv6 == addr);
}

template<std::derived_from<C4NetIO> T>
T *C4Network2IO::CreateSetupNetIO(const C4Network2IOProtocol protocol, const char *const name, T *const io, C4InteractiveThread &thread, const std::uint16_t defaultPort)
{
uint16_t port;

if (defaultPort == NULL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never use NULL in C++ - defaultPort also isn't a pointer.

{
port = DiscoverFreePort();
}
else
{
port = defaultPort;
if (!CheckPortAvailability(port))
{
logger->info("Port {} is unavailable, discovering a free one", port);
port = DiscoverFreePort();
}
}

T *netIO = CreateNetIO(this->logger, name, io, port, thread);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary this->. Additionally, you should use const as the variable isn't changed, and brace-initialization unless the variable's type is auto:

Suggested change
T *netIO = CreateNetIO(this->logger, name, io, port, thread);
T *const netIO{CreateNetIO(logger, name, io, port, thread)};


if (netIO)
{
netIO->SetCallback(this);

if (Config.Network.EnableUPnP)
{
UPnP->AddMapping(protocol, port, 0);
}
}

return netIO;
}

// C4NetIO interface
bool C4Network2IO::OnConn(const C4NetIO::addr_t &PeerAddr, const C4NetIO::addr_t &ConnectAddr, const C4NetIO::addr_t *pOwnAddr, C4NetIO *pNetIO)
{
Expand Down Expand Up @@ -1248,6 +1264,43 @@ void C4Network2IO::SendConnPackets()
}
}

bool C4Network2IO::CheckPortAvailability(const uint16_t port)
{
const auto pNetIO = std::make_unique<C4NetIOUDP>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't check whether a TCP port is available by attempting to bind a UDP socket.


if (port <= 0)
{
return false;
}

if (pNetIO->Init(port))
{
pNetIO->Close();

return true;
}

return false;
}

std::uint16_t C4Network2IO::DiscoverFreePort(const uint16_t attempts)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

{
static constexpr uint16_t maxPort = 65535;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a constant and therefore should be spelled MaxPort; otherwise, same as above.

uint16_t port = NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above. Also, we use std:: for types wherever possible.

Suggested change
uint16_t port = NULL;
std::uint16_t port{0};


for (uint16_t i = 0; i < attempts; ++i)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

{
port = SafeRandom(maxPort);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can return 0, which isn't a valid port.


if (CheckPortAvailability(port))
{
break;
}
}

return port;
}

// *** C4Network2IOConnection

C4Network2IOConnection::C4Network2IOConnection()
Expand Down
6 changes: 6 additions & 0 deletions src/C4Network2IO.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ class C4Network2IO
bool IsReferenceNeeded();

protected:
// C4NetIO Setup
template<std::derived_from<C4NetIO> T>
T *CreateSetupNetIO(const C4Network2IOProtocol protocol, const char *name, T *io, C4InteractiveThread &thread, std::uint16_t defaultPort = NULL);
// *** callbacks
// C4NetIO-Callbacks
virtual bool OnConn(const C4NetIO::addr_t &addr, const C4NetIO::addr_t &AddrConnect, const C4NetIO::addr_t *pOwnAddr, C4NetIO *pNetIO) override;
Expand Down Expand Up @@ -198,6 +201,9 @@ class C4Network2IO
void CheckTimeout();
void GenerateStatistics(int iInterval);
void SendConnPackets();
// misc -- ports
static bool CheckPortAvailability(uint16_t port);
static std::uint16_t DiscoverFreePort(uint16_t attempts = 32);
};

enum C4Network2IOConnStatus
Expand Down