From 30e2d88f21185fac9040ca31855ce7c81a569957 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Thu, 14 Mar 2024 16:12:14 +0100 Subject: [PATCH 1/8] enh(Net): cleanup and simplification of UPDServer code and related files. --- Net/include/Poco/Net/MultiSocketPoller.h | 21 ++++--- Net/include/Poco/Net/SingleSocketPoller.h | 3 +- Net/include/Poco/Net/UDPClient.h | 1 - Net/include/Poco/Net/UDPHandler.h | 74 +++++++++++------------ Net/include/Poco/Net/UDPServer.h | 4 -- Net/include/Poco/Net/UDPSocketReader.h | 29 ++++----- Net/testsuite/src/UDPServerTest.cpp | 12 ++-- 7 files changed, 67 insertions(+), 77 deletions(-) diff --git a/Net/include/Poco/Net/MultiSocketPoller.h b/Net/include/Poco/Net/MultiSocketPoller.h index 807a2e7522..8dea80d540 100644 --- a/Net/include/Poco/Net/MultiSocketPoller.h +++ b/Net/include/Poco/Net/MultiSocketPoller.h @@ -19,8 +19,12 @@ #include "Poco/Net/Net.h" +#include "Poco/Net/DatagramSocket.h" #include "Poco/Net/Socket.h" +#include "Poco/Net/PollSet.h" #include "Poco/Net/UDPHandler.h" +#include "Poco/Net/UDPServerParams.h" +#include "Poco/Net/UDPSocketReader.h" namespace Poco { @@ -74,23 +78,20 @@ class MultiSocketPoller void poll() { - if (_reader.handlerStopped()) return; + if (_reader.handlerStopped()) + return; PollSet::SocketModeMap sm; - PollSet::SocketModeMap::iterator it; - PollSet::SocketModeMap::iterator end; sm = _pollSet.poll(_timeout); - it = sm.begin(); - end = sm.end(); - for (; it != end; ++it) + for (auto& se: sm) { - if (it->second & PollSet::POLL_READ) + if (se.second & PollSet::POLL_READ) { - DatagramSocket ds(it->first); + DatagramSocket ds(se.first); _reader.read(ds); } - else if (it->second & PollSet::POLL_ERROR) + else if (se.second & PollSet::POLL_ERROR) { - _reader.setError(it->first.impl()->sockfd()); + _reader.setError(se.first.impl()->sockfd()); } } } diff --git a/Net/include/Poco/Net/SingleSocketPoller.h b/Net/include/Poco/Net/SingleSocketPoller.h index be200c157c..5f10ce2ba1 100644 --- a/Net/include/Poco/Net/SingleSocketPoller.h +++ b/Net/include/Poco/Net/SingleSocketPoller.h @@ -21,7 +21,8 @@ #include "Poco/Net/Net.h" #include "Poco/Net/DatagramSocket.h" #include "Poco/Net/UDPHandler.h" -#include "Poco/Net/PollSet.h" +#include "Poco/Net/UDPServerParams.h" +#include "Poco/Net/UDPSocketReader.h" namespace Poco { diff --git a/Net/include/Poco/Net/UDPClient.h b/Net/include/Poco/Net/UDPClient.h index 98889e45d6..1bb99c034b 100644 --- a/Net/include/Poco/Net/UDPClient.h +++ b/Net/include/Poco/Net/UDPClient.h @@ -21,7 +21,6 @@ #include "Poco/Net/Net.h" #include "Poco/Net/SocketAddress.h" #include "Poco/Net/DatagramSocket.h" -#include "Poco/Timespan.h" #include "Poco/Runnable.h" #include "Poco/Thread.h" diff --git a/Net/include/Poco/Net/UDPHandler.h b/Net/include/Poco/Net/UDPHandler.h index 467cf798d5..dc3298c557 100644 --- a/Net/include/Poco/Net/UDPHandler.h +++ b/Net/include/Poco/Net/UDPHandler.h @@ -19,26 +19,27 @@ #include "Poco/Net/Net.h" +#include "Poco/Net/SocketAddress.h" +#include "Poco/Net/SocketDefs.h" #include "Poco/RefCountedObject.h" #include "Poco/AutoPtr.h" #include "Poco/Runnable.h" #include "Poco/Thread.h" #include "Poco/MemoryPool.h" #include "Poco/Event.h" -#include "Poco/Error.h" #include "Poco/Mutex.h" #include "Poco/StringTokenizer.h" #include #include #include - +#include namespace Poco { namespace Net { -typedef int UDPMsgSizeT; -#define POCO_UDP_BUF_SIZE 1472 + sizeof(UDPMsgSizeT) + SocketAddress::MAX_ADDRESS_LENGTH +using UDPMsgSizeT = int; +constexpr std::size_t POCO_UDP_BUF_SIZE = 1472 + sizeof(UDPMsgSizeT) + SocketAddress::MAX_ADDRESS_LENGTH; template @@ -53,15 +54,14 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject /// the inheriting class can call start() in the constructor. { public: - typedef UDPMsgSizeT MsgSizeT; - typedef AutoPtr Ptr; - typedef std::vector List; - typedef typename List::iterator Iterator; - typedef TMutex DFMutex; + using MsgSizeT = UDPMsgSizeT; + using Ptr = AutoPtr; + using List = std::vector; + using DFMutex = TMutex; - static const MsgSizeT BUF_STATUS_IDLE = 0; - static const MsgSizeT BUF_STATUS_BUSY = -1; - static const MsgSizeT BUF_STATUS_ERROR = -2; + static constexpr MsgSizeT BUF_STATUS_IDLE = 0; + static constexpr MsgSizeT BUF_STATUS_BUSY = -1; + static constexpr MsgSizeT BUF_STATUS_ERROR = -2; UDPHandlerImpl(std::size_t bufListSize = 1000, std::ostream* pErr = 0): _thread("UDPHandlerImpl"), @@ -101,11 +101,11 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject { makeNext(sock, &ret); } - else if (*reinterpret_cast(*_bufIt[sock]) != 0) // busy + else if (payloadSize(*_bufIt[sock]) != 0) // busy { makeNext(sock, &ret); } - else if (*reinterpret_cast(*_bufIt[sock]) == 0) // available + else if (payloadSize(*_bufIt[sock]) == 0) // available { setBusy(*_bufIt[sock]); ret = *_bufIt[sock]; @@ -116,11 +116,11 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject } else // last resort, full scan { - BufList::iterator it = _buffers[sock].begin(); - BufList::iterator end = _buffers[sock].end(); - for (; it != end; ++it) + auto& buffers { _buffers[sock] }; + auto it {buffers.begin()}; + for (; it != buffers.end(); ++it) { - if (*reinterpret_cast(*_bufIt[sock]) == 0) // available + if (payloadSize(*_bufIt[sock]) == 0) // available { setBusy(*it); ret = *it; @@ -132,7 +132,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject break; } } - if (it == end) makeNext(sock, &ret); + if (it == buffers.end()) makeNext(sock, &ret); } _mutex.unlock(); } @@ -156,23 +156,19 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject { if (!_stop) { - BufMap::iterator it = _buffers.begin(); - BufMap::iterator end = _buffers.end(); - for (; it != end; ++it) + for (auto& buf: _buffers) { - BufList::iterator lIt = it->second.begin(); - BufList::iterator lEnd = it->second.end(); - for (; lIt != lEnd; ++lIt) + for (auto* list: buf.second) { - if (hasData(*lIt)) + if (hasData(list)) { - processData(*lIt); + processData(list); --_dataBacklog; - setIdle(*lIt); + setIdle(list); } - else if (isError(*lIt)) + else if (isError(list)) { - processError(*lIt); + processError(list); ++_errorBacklog; } } @@ -245,7 +241,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject /// Returns true if buffer contains data. { typename DFMutex::ScopedLock l(_dfMutex); - return *reinterpret_cast(pBuf) > 0; + return payloadSize(pBuf) > 0; } bool isError(char*& pBuf) @@ -263,7 +259,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject static MsgSizeT payloadSize(char* buf) { - return *((MsgSizeT*) buf); + return *reinterpret_cast(buf); } static SocketAddress address(char* buf) @@ -297,7 +293,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject } static char* error(char* buf) - /// Returns pointer to the erro message payload. + /// Returns pointer to the error message payload. /// /// Total message size is S. /// @@ -336,11 +332,11 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject } private: - typedef std::deque BufList; - typedef std::map BufMap; - typedef typename BufList::iterator BLIt; - typedef std::map BufIt; - typedef Poco::FastMemoryPool MemPool; + using BufList = std::deque; + using BufMap = std::map; + using BLIt = typename BufList::iterator; + using BufIt = std::map; + using MemPool = Poco::FastMemoryPool; void setStatusImpl(char*& pBuf, MsgSizeT status) { @@ -378,7 +374,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject }; -typedef UDPHandlerImpl UDPHandler; +using UDPHandler = UDPHandlerImpl<>; } } // namespace Poco::Net diff --git a/Net/include/Poco/Net/UDPServer.h b/Net/include/Poco/Net/UDPServer.h index a65036745a..c97469e596 100644 --- a/Net/include/Poco/Net/UDPServer.h +++ b/Net/include/Poco/Net/UDPServer.h @@ -19,14 +19,10 @@ #include "Poco/Net/Net.h" -#include "Poco/Net/DatagramSocket.h" -#include "Poco/Net/PollSet.h" #include "Poco/Net/UDPHandler.h" #include "Poco/Net/UDPServerParams.h" -#include "Poco/Net/UDPSocketReader.h" #include "Poco/Net/SingleSocketPoller.h" #include "Poco/Net/MultiSocketPoller.h" -#include namespace Poco { diff --git a/Net/include/Poco/Net/UDPSocketReader.h b/Net/include/Poco/Net/UDPSocketReader.h index b6af9e0bd2..8a22fbebd2 100644 --- a/Net/include/Poco/Net/UDPSocketReader.h +++ b/Net/include/Poco/Net/UDPSocketReader.h @@ -20,7 +20,9 @@ #include "Poco/Net/Net.h" #include "Poco/Net/DatagramSocket.h" - +#include "Poco/Net/UDPHandler.h" +#include "Poco/Net/UDPServerParams.h" +#include "Poco/Error.h" namespace Poco { namespace Net { @@ -137,7 +139,7 @@ class UDPSocketReader } else return; } - catch (Poco::Exception& exc) + catch (const Poco::Exception& exc) { AtomicCounter::ValueType errors = setError(sock.impl()->sockfd(), p, exc.displayText()); if (_backlogThreshold > 0 && errors > _backlogThreshold && errors != _errorBacklog[sockfd] && pSA && pAL) @@ -154,27 +156,26 @@ class UDPSocketReader /// Returns true if all handlers are stopped. { bool stopped = true; - typename UDPHandlerImpl::List::iterator it = _handlers.begin(); - typename UDPHandlerImpl::List::iterator end = _handlers.end(); - for (; it != end; ++it) stopped = stopped && (*it)->stopped(); + for (auto& h: _handlers) + stopped = stopped && h->stopped(); + return stopped; } void stopHandler() /// Stops all handlers. { - typename UDPHandlerImpl::List::iterator it = _handlers.begin(); - typename UDPHandlerImpl::List::iterator end = _handlers.end(); - for (; it != end; ++it) (*it)->stop(); + for (auto& h: _handlers) + h->stop(); } bool handlerDone() const /// Returns true if all handlers are done processing data. { bool done = true; - typename UDPHandlerImpl::List::iterator it = _handlers.begin(); - typename UDPHandlerImpl::List::iterator end = _handlers.end(); - for (; it != end; ++it) done = done && (*it)->done(); + for (auto& h: _handlers) + done = done && h->done(); + return done; } @@ -204,9 +205,9 @@ class UDPSocketReader return **_handler; } - typedef typename UDPHandlerImpl::List HandlerList; - typedef typename UDPHandlerImpl::List::iterator HandlerIterator; - typedef std::map CounterMap; + using HandlerList = typename UDPHandlerImpl::List; + using HandlerIterator = typename HandlerList::iterator; + using CounterMap = std::map; HandlerList& _handlers; HandlerIterator _handler; diff --git a/Net/testsuite/src/UDPServerTest.cpp b/Net/testsuite/src/UDPServerTest.cpp index 1e82cc33ad..95c5bce639 100644 --- a/Net/testsuite/src/UDPServerTest.cpp +++ b/Net/testsuite/src/UDPServerTest.cpp @@ -124,11 +124,9 @@ namespace count = 0; errCount = 0; Poco::Thread::sleep(10); - Poco::Net::UDPHandler::Iterator it = handlers.begin(); - Poco::Net::UDPHandler::Iterator end = handlers.end(); - for (; it != end; ++it) + for (const auto& h: handlers) { - count += dynamic_cast(*(*it)).counter.value(); + count += dynamic_cast(*h).counter.value(); errCount += count / 10; } } while (count < i); @@ -138,11 +136,9 @@ namespace << ", received: " << count << std::endl; return false; } - Poco::Net::UDPHandler::Iterator it = handlers.begin(); - Poco::Net::UDPHandler::Iterator end = handlers.end(); - for (; it != end; ++it) + for (const auto& he: handlers) { - TestUDPHandler &h = dynamic_cast(*(*it)); + const auto &h = dynamic_cast(*he); count = h.counter.value(); errCount = h.errCounter.value(); if (errCount < count / 10) From 0fe4451eb257fec73b4eaa44a324df1f270f2f0e Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Tue, 19 Mar 2024 11:11:20 +0100 Subject: [PATCH 2/8] enh(Net): some C++ modernisation --- Net/include/Poco/Net/DatagramSocket.h | 2 +- Net/include/Poco/Net/MultiSocketPoller.h | 2 +- Net/include/Poco/Net/Socket.h | 3 ++- Net/include/Poco/Net/SocketDefs.h | 8 ++++---- Net/include/Poco/Net/SocketImpl.h | 2 +- Net/include/Poco/Net/StreamSocket.h | 2 +- Net/include/Poco/Net/UDPClient.h | 4 ++-- Net/include/Poco/Net/UDPHandler.h | 11 ++++++----- Net/include/Poco/Net/UDPServer.h | 4 ++-- Net/include/Poco/Net/UDPSocketReader.h | 12 ++++++------ Net/include/Poco/Net/WebSocket.h | 2 +- Net/src/SocketImpl.cpp | 2 +- Net/testsuite/src/UDPServerTest.cpp | 6 +++--- Net/testsuite/src/WebSocketTest.cpp | 4 ++-- 14 files changed, 33 insertions(+), 31 deletions(-) diff --git a/Net/include/Poco/Net/DatagramSocket.h b/Net/include/Poco/Net/DatagramSocket.h index 8c3d69aaf4..82b1b60c9b 100644 --- a/Net/include/Poco/Net/DatagramSocket.h +++ b/Net/include/Poco/Net/DatagramSocket.h @@ -67,7 +67,7 @@ class Net_API DatagramSocket: public Socket /// Creates the DatagramSocket with the SocketImpl /// from another socket. - ~DatagramSocket(); + ~DatagramSocket() override; /// Destroys the DatagramSocket. DatagramSocket& operator = (const Socket& socket); diff --git a/Net/include/Poco/Net/MultiSocketPoller.h b/Net/include/Poco/Net/MultiSocketPoller.h index 8dea80d540..91efeaf6b4 100644 --- a/Net/include/Poco/Net/MultiSocketPoller.h +++ b/Net/include/Poco/Net/MultiSocketPoller.h @@ -38,7 +38,7 @@ class MultiSocketPoller /// state, the reading/error handling actions are delegated to the reader. { public: - MultiSocketPoller(typename UDPHandlerImpl::List& handlers, const Poco::Net::SocketAddress& sa, int nSockets = 10, Poco::Timespan timeout = 250000): + MultiSocketPoller(typename UDPHandlerImpl::List& handlers, const Poco::Net::SocketAddress& sa, int nSockets = 10, const Poco::Timespan& timeout = 250000): _address(sa), _timeout(timeout), _reader(handlers) diff --git a/Net/include/Poco/Net/Socket.h b/Net/include/Poco/Net/Socket.h index 2817150413..51dc67d140 100644 --- a/Net/include/Poco/Net/Socket.h +++ b/Net/include/Poco/Net/Socket.h @@ -411,11 +411,12 @@ class FDCompare { public: FDCompare(int fd): _fd(fd) { } + FDCompare() = delete; + inline bool operator()(const Socket& socket) const { return socket.sockfd() == _fd; } private: - FDCompare(); int _fd; }; #endif diff --git a/Net/include/Poco/Net/SocketDefs.h b/Net/include/Poco/Net/SocketDefs.h index 8806e80bdc..7bbb7f761e 100644 --- a/Net/include/Poco/Net/SocketDefs.h +++ b/Net/include/Poco/Net/SocketDefs.h @@ -144,7 +144,7 @@ #define POCO_NO_DATA NO_DATA #elif defined(POCO_OS_FAMILY_UNIX) #include - #include + #include #include #include #include @@ -369,12 +369,12 @@ namespace Net { #if defined(POCO_OS_FAMILY_WINDOWS) - typedef WSABUF SocketBuf; + using SocketBuf = WSABUF; #elif defined(POCO_OS_FAMILY_UNIX) // TODO: may need more refinement - typedef iovec SocketBuf; + using SocketBuf = iovec; #endif -typedef std::vector SocketBufVec; +using SocketBufVec = std::vector; inline int SocketBufVecSize(const SocketBufVec& sbv) /// Returns total length of all SocketBufs in the vector. diff --git a/Net/include/Poco/Net/SocketImpl.h b/Net/include/Poco/Net/SocketImpl.h index 501488e052..b2e894aa3d 100644 --- a/Net/include/Poco/Net/SocketImpl.h +++ b/Net/include/Poco/Net/SocketImpl.h @@ -490,7 +490,7 @@ class Net_API SocketImpl: public Poco::RefCountedObject SocketImpl(poco_socket_t sockfd); /// Creates a SocketImpl using the given native socket. - virtual ~SocketImpl(); + ~SocketImpl() override; /// Destroys the SocketImpl. /// Closes the socket if it is still open. diff --git a/Net/include/Poco/Net/StreamSocket.h b/Net/include/Poco/Net/StreamSocket.h index a71b87acd5..f547d72964 100644 --- a/Net/include/Poco/Net/StreamSocket.h +++ b/Net/include/Poco/Net/StreamSocket.h @@ -69,7 +69,7 @@ class Net_API StreamSocket: public Socket /// Creates the StreamSocket with the SocketImpl /// from another socket. - virtual ~StreamSocket(); + ~StreamSocket() override; /// Destroys the StreamSocket. StreamSocket& operator = (const Socket& socket); diff --git a/Net/include/Poco/Net/UDPClient.h b/Net/include/Poco/Net/UDPClient.h index 1bb99c034b..bfe184c904 100644 --- a/Net/include/Poco/Net/UDPClient.h +++ b/Net/include/Poco/Net/UDPClient.h @@ -44,10 +44,10 @@ class Net_API UDPClient : public Poco::Runnable /// If listen is true, a thread is launched where client can receive /// responses rom the server. - virtual ~UDPClient(); + ~UDPClient() override; /// Destroys UDPClient. - void run(); + void run() override; /// Runs listener (typically invoked internally, in separate thread). SocketAddress address() const; diff --git a/Net/include/Poco/Net/UDPHandler.h b/Net/include/Poco/Net/UDPHandler.h index dc3298c557..6d371057d7 100644 --- a/Net/include/Poco/Net/UDPHandler.h +++ b/Net/include/Poco/Net/UDPHandler.h @@ -63,7 +63,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject static constexpr MsgSizeT BUF_STATUS_BUSY = -1; static constexpr MsgSizeT BUF_STATUS_ERROR = -2; - UDPHandlerImpl(std::size_t bufListSize = 1000, std::ostream* pErr = 0): + UDPHandlerImpl(std::size_t bufListSize = 1000, std::ostream* pErr = nullptr): _thread("UDPHandlerImpl"), _stop(false), _done(false), @@ -76,7 +76,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject { } - ~UDPHandlerImpl() + ~UDPHandlerImpl() override /// Destroys the UDPHandlerImpl. { stop(); @@ -94,7 +94,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject /// the pointers to the newly created guard/buffer. /// If mutex lock times out, returns null pointer. { - char* ret = 0; + char* ret = nullptr; if (_mutex.tryLock(10)) { if (_buffers[sock].size() < _bufListSize) // building buffer list @@ -145,7 +145,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject _dataReady.set(); } - void run() + void run() override /// Does the work. { while (!_stop) @@ -336,7 +336,8 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject using BufMap = std::map; using BLIt = typename BufList::iterator; using BufIt = std::map; - using MemPool = Poco::FastMemoryPool; + using BufArray = std::array; + using MemPool = Poco::FastMemoryPool; void setStatusImpl(char*& pBuf, MsgSizeT status) { diff --git a/Net/include/Poco/Net/UDPServer.h b/Net/include/Poco/Net/UDPServer.h index c97469e596..10e135eec4 100644 --- a/Net/include/Poco/Net/UDPServer.h +++ b/Net/include/Poco/Net/UDPServer.h @@ -57,7 +57,7 @@ class UDPServerImpl: public Poco::Runnable _thread.start(*this); } - ~UDPServerImpl() + ~UDPServerImpl() override /// Destroys the UDPServer. { _stop = true; @@ -79,7 +79,7 @@ class UDPServerImpl: public Poco::Runnable return _poller.address(); } - void run() + void run() override /// Does the work. { while (!_stop) _poller.poll(); diff --git a/Net/include/Poco/Net/UDPSocketReader.h b/Net/include/Poco/Net/UDPSocketReader.h index 8a22fbebd2..62331ceeaa 100644 --- a/Net/include/Poco/Net/UDPSocketReader.h +++ b/Net/include/Poco/Net/UDPSocketReader.h @@ -101,10 +101,10 @@ class UDPSocketReader /// for replying to sender and data or error backlog threshold is /// exceeded, sender is notified of the current backlog size. { - typedef typename UDPHandlerImpl::MsgSizeT RT; - char* p = 0; - struct sockaddr* pSA = 0; - poco_socklen_t* pAL = 0; + using RT = typename UDPHandlerImpl::MsgSizeT; + char* p = nullptr; + struct sockaddr* pSA = nullptr; + poco_socklen_t* pAL = nullptr; poco_socket_t sockfd = sock.impl()->sockfd(); nextHandler(); try @@ -179,10 +179,10 @@ class UDPSocketReader return done; } - AtomicCounter::ValueType setError(poco_socket_t sock, char* buf = 0, const std::string& err = "") + AtomicCounter::ValueType setError(poco_socket_t sock, char* buf = nullptr, const std::string& err = "") /// Sets error to the provided buffer buf. If the buffer is null, a new buffer is obtained /// from handler. - /// If successful, returns the handler's eror backlog size, otherwise returns zero. + /// If successful, returns the handler's error backlog size, otherwise returns zero. { if (!buf) buf = handler().next(sock); if (buf) return handler().setError(buf, err.empty() ? Error::getMessage(Error::last()) : err); diff --git a/Net/include/Poco/Net/WebSocket.h b/Net/include/Poco/Net/WebSocket.h index e6cf927462..cf62da32c3 100644 --- a/Net/include/Poco/Net/WebSocket.h +++ b/Net/include/Poco/Net/WebSocket.h @@ -179,7 +179,7 @@ class Net_API WebSocket: public StreamSocket WebSocket(const WebSocket& socket); /// Creates a WebSocket from another WebSocket. - virtual ~WebSocket(); + ~WebSocket() override; /// Destroys the WebSocket. WebSocket& operator = (const Socket& socket); diff --git a/Net/src/SocketImpl.cpp b/Net/src/SocketImpl.cpp index 34cad7d074..a3d85f824f 100644 --- a/Net/src/SocketImpl.cpp +++ b/Net/src/SocketImpl.cpp @@ -133,7 +133,7 @@ SocketImpl* SocketImpl::acceptConnection(SocketAddress& clientAddr) if (_sockfd == POCO_INVALID_SOCKET) throw InvalidSocketException(); sockaddr_storage buffer; - struct sockaddr* pSA = reinterpret_cast(&buffer); + auto* pSA = reinterpret_cast(&buffer); poco_socklen_t saLen = sizeof(buffer); poco_socket_t sd; do diff --git a/Net/testsuite/src/UDPServerTest.cpp b/Net/testsuite/src/UDPServerTest.cpp index 95c5bce639..00b121b45d 100644 --- a/Net/testsuite/src/UDPServerTest.cpp +++ b/Net/testsuite/src/UDPServerTest.cpp @@ -50,7 +50,7 @@ namespace start(); } - void processData(char *buf) + void processData(char *buf) override { if (!addr.empty() && addr != address(buf).toString()) ++errors; addr = address(buf).toString(); @@ -68,7 +68,7 @@ namespace } } - void processError(char *buf) + void processError(char *buf) override { if (std::string(error(buf)) != "error") ++errors; std::memset(buf, 0, blockSize()); @@ -100,7 +100,7 @@ namespace const char *str = "hello\n"; for (; i < reps; ++i) { - data.push_back(str); + data.emplace_back(str); if (data.size() == 230 || i == reps - 1) { data.back().append(1, '\0'); diff --git a/Net/testsuite/src/WebSocketTest.cpp b/Net/testsuite/src/WebSocketTest.cpp index 3cd312ed1f..bf1bbac6e4 100644 --- a/Net/testsuite/src/WebSocketTest.cpp +++ b/Net/testsuite/src/WebSocketTest.cpp @@ -47,7 +47,7 @@ namespace { } - void handleRequest(HTTPServerRequest& request, HTTPServerResponse& response) + void handleRequest(HTTPServerRequest& request, HTTPServerResponse& response) override { try { @@ -97,7 +97,7 @@ namespace { } - Poco::Net::HTTPRequestHandler* createRequestHandler(const HTTPServerRequest& request) + Poco::Net::HTTPRequestHandler* createRequestHandler(const HTTPServerRequest& request) override { return new WebSocketRequestHandler(_bufSize); } From 3a46ae2e323159329b16e65051fcf4d32fa3d82e Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Tue, 19 Mar 2024 13:03:45 +0100 Subject: [PATCH 3/8] test(UDPServer): add more unit tests --- Net/testsuite/src/UDPServerTest.cpp | 58 +++++++++++++++++++++++------ Net/testsuite/src/UDPServerTest.h | 6 ++- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/Net/testsuite/src/UDPServerTest.cpp b/Net/testsuite/src/UDPServerTest.cpp index 00b121b45d..79894f0295 100644 --- a/Net/testsuite/src/UDPServerTest.cpp +++ b/Net/testsuite/src/UDPServerTest.cpp @@ -82,17 +82,17 @@ namespace AtomicCounter TestUDPHandler::errors; - template - bool server(int handlerCount, int reps, int port = 0) + template + bool server(int handlerCount, int reps, int port) { Poco::Net::UDPHandler::List handlers; for (int i = 0; i < handlerCount; ++i) - handlers.push_back(new TestUDPHandler()); + handlers.push_back(new HNDLRT()); - S server(handlers, Poco::Net::SocketAddress("127.0.0.1", port)); + SRVT server(handlers, Poco::Net::SocketAddress("127.0.0.1", port)); Poco::Thread::sleep(100); - Poco::Net::UDPClient client("127.0.0.1", server.port(), true); + CLTT client("127.0.0.1", server.port(), true); Poco::Thread::sleep(10); std::vector data; @@ -112,7 +112,7 @@ namespace << ", sent: " << sent << std::endl; return false; } - Poco::Thread::sleep(10); + Poco::Thread::sleep(5); data.clear(); } } @@ -126,7 +126,7 @@ namespace Poco::Thread::sleep(10); for (const auto& h: handlers) { - count += dynamic_cast(*h).counter.value(); + count += dynamic_cast(*h).counter.value(); errCount += count / 10; } } while (count < i); @@ -138,7 +138,7 @@ namespace } for (const auto& he: handlers) { - const auto &h = dynamic_cast(*he); + const auto &h = dynamic_cast(*he); count = h.counter.value(); errCount = h.errCounter.value(); if (errCount < count / 10) @@ -170,11 +170,42 @@ UDPServerTest::~UDPServerTest() } -void UDPServerTest::testServer() +void UDPServerTest::testUDPSingleSocket() { + TestUDPHandler::errors = 0; int msgs = 10000; - assertTrue (server(1, msgs)); - assertTrue (server(10, msgs, 22080)); + auto tf = server; + assertTrue( tf(1, msgs, 0) ); + assertTrue (TestUDPHandler::errors == 0); +} + + +void UDPServerTest::testUDPMultiSocket() +{ + TestUDPHandler::errors = 0; + int msgs = 10000; + auto tf = server; + assertTrue( tf(1, msgs, 22080) ); + assertTrue (TestUDPHandler::errors == 0); +} + + +void UDPServerTest::testUDPSingleSocketMultipleHandlers() +{ + TestUDPHandler::errors = 0; + int msgs = 10000; + auto tf = server; + assertTrue( tf(10, msgs, 0) ); + assertTrue (TestUDPHandler::errors == 0); +} + + +void UDPServerTest::testUDPMultiSocketMultipleHandlers() +{ + TestUDPHandler::errors = 0; + int msgs = 10000; + auto tf = server; + assertTrue( tf(10, msgs, 22080) ); assertTrue (TestUDPHandler::errors == 0); } @@ -193,7 +224,10 @@ CppUnit::Test* UDPServerTest::suite() { CppUnit::TestSuite* pSuite = new CppUnit::TestSuite("UDPServerTest"); - CppUnit_addTest(pSuite, UDPServerTest, testServer); + CppUnit_addTest(pSuite, UDPServerTest, testUDPSingleSocket); + CppUnit_addTest(pSuite, UDPServerTest, testUDPMultiSocket); + CppUnit_addTest(pSuite, UDPServerTest, testUDPSingleSocketMultipleHandlers); + CppUnit_addTest(pSuite, UDPServerTest, testUDPMultiSocketMultipleHandlers); return pSuite; } diff --git a/Net/testsuite/src/UDPServerTest.h b/Net/testsuite/src/UDPServerTest.h index 7c1714ab01..26d05aaa8d 100644 --- a/Net/testsuite/src/UDPServerTest.h +++ b/Net/testsuite/src/UDPServerTest.h @@ -16,7 +16,6 @@ #include "Poco/Net/Net.h" #include "CppUnit/TestCase.h" -#include "Poco/Thread.h" class UDPServerTest: public CppUnit::TestCase { @@ -24,7 +23,10 @@ class UDPServerTest: public CppUnit::TestCase UDPServerTest(const std::string& name); ~UDPServerTest(); - void testServer(); + void testUDPSingleSocket(); + void testUDPMultiSocket(); + void testUDPSingleSocketMultipleHandlers(); + void testUDPMultiSocketMultipleHandlers(); void setUp(); void tearDown(); From a0c43be135204ae60a6fafad328ad95cead62538 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Tue, 19 Mar 2024 13:11:46 +0100 Subject: [PATCH 4/8] test(Net): rename UDPServerTestSuite to AsyncServerTestSuite --- Net/testsuite/Makefile | 2 +- Net/testsuite/TestSuite_vs160.vcxproj | 4 ++-- Net/testsuite/TestSuite_vs160.vcxproj.filters | 4 ++-- Net/testsuite/TestSuite_vs170.vcxproj | 4 ++-- Net/testsuite/TestSuite_vs170.vcxproj.filters | 4 ++-- Net/testsuite/TestSuite_vs90.vcproj | 4 ++-- ...PServerTestSuite.cpp => AsyncServerTestSuite.cpp} | 8 ++++---- .../{UDPServerTestSuite.h => AsyncServerTestSuite.h} | 12 ++++++------ Net/testsuite/src/NetTestSuite.cpp | 4 ++-- 9 files changed, 23 insertions(+), 23 deletions(-) rename Net/testsuite/src/{UDPServerTestSuite.cpp => AsyncServerTestSuite.cpp} (56%) rename Net/testsuite/src/{UDPServerTestSuite.h => AsyncServerTestSuite.h} (51%) diff --git a/Net/testsuite/Makefile b/Net/testsuite/Makefile index 0070233e96..5af297c702 100644 --- a/Net/testsuite/Makefile +++ b/Net/testsuite/Makefile @@ -28,7 +28,7 @@ objects = \ WebSocketTest WebSocketTestSuite \ SyslogTest \ OAuth10CredentialsTest OAuth20CredentialsTest OAuthTestSuite \ - PollSetTest UDPServerTest UDPServerTestSuite \ + PollSetTest UDPServerTest AsyncServerTestSuite \ NTLMCredentialsTest target = testrunner diff --git a/Net/testsuite/TestSuite_vs160.vcxproj b/Net/testsuite/TestSuite_vs160.vcxproj index 4ef8a2e51d..30e3aec1ff 100644 --- a/Net/testsuite/TestSuite_vs160.vcxproj +++ b/Net/testsuite/TestSuite_vs160.vcxproj @@ -710,7 +710,7 @@ - + @@ -1035,7 +1035,7 @@ stdcpp17 stdc11 - + true stdcpp17 stdc11 diff --git a/Net/testsuite/TestSuite_vs160.vcxproj.filters b/Net/testsuite/TestSuite_vs160.vcxproj.filters index 076729cfea..4d02c94f5b 100644 --- a/Net/testsuite/TestSuite_vs160.vcxproj.filters +++ b/Net/testsuite/TestSuite_vs160.vcxproj.filters @@ -366,7 +366,7 @@ UDP\Header Files - + UDP\Header Files @@ -569,7 +569,7 @@ UDP\Source Files - + UDP\Source Files diff --git a/Net/testsuite/TestSuite_vs170.vcxproj b/Net/testsuite/TestSuite_vs170.vcxproj index 03051fa8af..843d2698d2 100644 --- a/Net/testsuite/TestSuite_vs170.vcxproj +++ b/Net/testsuite/TestSuite_vs170.vcxproj @@ -1025,7 +1025,7 @@ - + @@ -1350,7 +1350,7 @@ stdcpp17 stdc11 - + true stdcpp17 stdc11 diff --git a/Net/testsuite/TestSuite_vs170.vcxproj.filters b/Net/testsuite/TestSuite_vs170.vcxproj.filters index 242c27c5ff..2e28a0337a 100644 --- a/Net/testsuite/TestSuite_vs170.vcxproj.filters +++ b/Net/testsuite/TestSuite_vs170.vcxproj.filters @@ -366,7 +366,7 @@ UDP\Header Files - + UDP\Header Files @@ -569,7 +569,7 @@ UDP\Source Files - + UDP\Source Files diff --git a/Net/testsuite/TestSuite_vs90.vcproj b/Net/testsuite/TestSuite_vs90.vcproj index 68d782f5e3..abd919b2e1 100644 --- a/Net/testsuite/TestSuite_vs90.vcproj +++ b/Net/testsuite/TestSuite_vs90.vcproj @@ -1295,7 +1295,7 @@ > @@ -1307,7 +1307,7 @@ > diff --git a/Net/testsuite/src/UDPServerTestSuite.cpp b/Net/testsuite/src/AsyncServerTestSuite.cpp similarity index 56% rename from Net/testsuite/src/UDPServerTestSuite.cpp rename to Net/testsuite/src/AsyncServerTestSuite.cpp index 455ae58801..de8abfae05 100644 --- a/Net/testsuite/src/UDPServerTestSuite.cpp +++ b/Net/testsuite/src/AsyncServerTestSuite.cpp @@ -1,5 +1,5 @@ // -// UDPServerTestSuite.cpp +// AsyncServerTestSuite.cpp // // Copyright (c) 2005-2006, Applied Informatics Software Engineering GmbH. // and Contributors. @@ -8,13 +8,13 @@ // -#include "UDPServerTestSuite.h" +#include "AsyncServerTestSuite.h" #include "UDPServerTest.h" -CppUnit::Test* UDPServerTestSuite::suite() +CppUnit::Test* AsyncServerTestSuite::suite() { - CppUnit::TestSuite* pSuite = new CppUnit::TestSuite("UDPServerTestSuite"); + CppUnit::TestSuite* pSuite = new CppUnit::TestSuite("AsyncServerTestSuite"); pSuite->addTest(UDPServerTest::suite()); diff --git a/Net/testsuite/src/UDPServerTestSuite.h b/Net/testsuite/src/AsyncServerTestSuite.h similarity index 51% rename from Net/testsuite/src/UDPServerTestSuite.h rename to Net/testsuite/src/AsyncServerTestSuite.h index b54aa0eebc..794f6d1245 100644 --- a/Net/testsuite/src/UDPServerTestSuite.h +++ b/Net/testsuite/src/AsyncServerTestSuite.h @@ -1,7 +1,7 @@ // -// UDPServerTestSuite.h +// AsyncServerTestSuite.h // -// Definition of the UDPServerTestSuite class. +// Definition of the AsyncServerTestSuite class. // // Copyright (c) 2005-2006, Applied Informatics Software Engineering GmbH. // and Contributors. @@ -10,18 +10,18 @@ // -#ifndef UDPServerTestSuite_INCLUDED -#define UDPServerTestSuite_INCLUDED +#ifndef AsyncServerTestSuite_INCLUDED +#define AsyncServerTestSuite_INCLUDED #include "CppUnit/TestSuite.h" -class UDPServerTestSuite +class AsyncServerTestSuite { public: static CppUnit::Test* suite(); }; -#endif // UDPServerTestSuite_INCLUDED +#endif // AsyncServerTestSuite_INCLUDED diff --git a/Net/testsuite/src/NetTestSuite.cpp b/Net/testsuite/src/NetTestSuite.cpp index 29419ab577..50ff5e10ec 100644 --- a/Net/testsuite/src/NetTestSuite.cpp +++ b/Net/testsuite/src/NetTestSuite.cpp @@ -15,7 +15,7 @@ #include "HTTPTestSuite.h" #include "HTTPClientTestSuite.h" #include "TCPServerTestSuite.h" -#include "UDPServerTestSuite.h" +#include "AsyncServerTestSuite.h" #include "HTTPServerTestSuite.h" #include "HTMLTestSuite.h" #include "ReactorTestSuite.h" @@ -38,7 +38,7 @@ CppUnit::Test* NetTestSuite::suite() pSuite->addTest(HTTPTestSuite::suite()); pSuite->addTest(HTTPClientTestSuite::suite()); pSuite->addTest(TCPServerTestSuite::suite()); - pSuite->addTest(UDPServerTestSuite::suite()); + pSuite->addTest(AsyncServerTestSuite::suite()); pSuite->addTest(HTTPServerTestSuite::suite()); pSuite->addTest(HTMLTestSuite::suite()); pSuite->addTest(ReactorTestSuite::suite()); From 71f8b421cad14988279a0739fc38261cb1699c4c Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Tue, 19 Mar 2024 14:09:12 +0100 Subject: [PATCH 5/8] enh(UDPHandler): use const arguments where reasonable. --- Net/include/Poco/Net/UDPHandler.h | 36 +++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Net/include/Poco/Net/UDPHandler.h b/Net/include/Poco/Net/UDPHandler.h index 6d371057d7..49940db053 100644 --- a/Net/include/Poco/Net/UDPHandler.h +++ b/Net/include/Poco/Net/UDPHandler.h @@ -156,7 +156,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject { if (!_stop) { - for (auto& buf: _buffers) + for (const auto& buf: _buffers) { for (auto* list: buf.second) { @@ -200,14 +200,14 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject return _done; } - void setBusy(char*& pBuf) + void setBusy(char* pBuf) /// Flags the buffer as busy (usually done before buffer /// is passed to the reader. { setStatus(pBuf, BUF_STATUS_BUSY); } - void setIdle(char*& pBuf) + void setIdle(char* pBuf) /// Flags the buffer as idle, ie. not used by reader or /// waiting to be processed, so ready to be reused for /// reading. @@ -215,14 +215,14 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject setStatus(pBuf, BUF_STATUS_IDLE); } - AtomicCounter::ValueType setData(char*& pBuf, MsgSizeT sz) + AtomicCounter::ValueType setData(char* pBuf, MsgSizeT sz) /// Flags the buffer as containing data. { setStatus(pBuf, sz); return ++_dataBacklog; } - AtomicCounter::ValueType setError(char*& pBuf, const std::string& err) + AtomicCounter::ValueType setError(char* pBuf, const std::string& err) /// Sets the error into the buffer. { std::size_t availLen = S - sizeof(MsgSizeT); @@ -237,18 +237,18 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject return --_errorBacklog; } - bool hasData(char*& pBuf) + bool hasData(const char* pBuf) /// Returns true if buffer contains data. { typename DFMutex::ScopedLock l(_dfMutex); return payloadSize(pBuf) > 0; } - bool isError(char*& pBuf) + bool isError(const char* pBuf) /// Returns true if buffer contains error. { typename DFMutex::ScopedLock l(_dfMutex); - return *reinterpret_cast(pBuf) == BUF_STATUS_ERROR; + return *reinterpret_cast(pBuf) == BUF_STATUS_ERROR; } static Poco::UInt16 offset() @@ -257,19 +257,19 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject return sizeof(MsgSizeT) + sizeof(poco_socklen_t) + SocketAddress::MAX_ADDRESS_LENGTH; } - static MsgSizeT payloadSize(char* buf) + static MsgSizeT payloadSize(const char* buf) { - return *reinterpret_cast(buf); + return *reinterpret_cast(buf); } - static SocketAddress address(char* buf) + static SocketAddress address(const char* buf) { - poco_socklen_t* len = reinterpret_cast(buf + sizeof(MsgSizeT)); - struct sockaddr* pSA = reinterpret_cast(buf + sizeof(MsgSizeT) + sizeof(poco_socklen_t)); + const auto* len = reinterpret_cast(buf + sizeof(MsgSizeT)); + const auto* pSA = reinterpret_cast(buf + sizeof(MsgSizeT) + sizeof(poco_socklen_t)); return SocketAddress(pSA, *len); } - static char* payload(char* buf) + static const char* payload(const char* buf) /// Returns pointer to payload. /// /// Total message size is S. @@ -283,7 +283,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject return buf + offset(); } - static Poco::StringTokenizer payload(char* buf, char delimiter) + static Poco::StringTokenizer payload(const char* buf, char delimiter) /// Returns tokenized payload. /// Used when multiple logical messages are contained in a /// single physical message. Messages must be ASCII, as well as @@ -292,7 +292,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject return Poco::StringTokenizer(payload(buf), std::string(1, delimiter), StringTokenizer::TOK_IGNORE_EMPTY); } - static char* error(char* buf) + static const char* error(const char* buf) /// Returns pointer to the error message payload. /// /// Total message size is S. @@ -339,12 +339,12 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject using BufArray = std::array; using MemPool = Poco::FastMemoryPool; - void setStatusImpl(char*& pBuf, MsgSizeT status) + void setStatusImpl(char* pBuf, MsgSizeT status) { *reinterpret_cast(pBuf) = status; } - void setStatus(char*& pBuf, MsgSizeT status) + void setStatus(char* pBuf, MsgSizeT status) { typename DFMutex::ScopedLock l(_dfMutex); setStatusImpl(pBuf, status); From 25f99ca031955f0c726a80c584e4d180c487c1d1 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Tue, 19 Mar 2024 15:31:42 +0100 Subject: [PATCH 6/8] test(UDPServer): add more unit tests with different parameters --- Net/include/Poco/Net/UDPClient.h | 5 +- Net/include/Poco/Net/UDPServer.h | 5 +- Net/include/Poco/Net/UDPServerParams.h | 2 +- Net/src/UDPServerParams.cpp | 2 +- Net/testsuite/src/UDPServerTest.cpp | 90 +++++++++++++++++++++----- Net/testsuite/src/UDPServerTest.h | 3 + 6 files changed, 87 insertions(+), 20 deletions(-) diff --git a/Net/include/Poco/Net/UDPClient.h b/Net/include/Poco/Net/UDPClient.h index bfe184c904..d9570560da 100644 --- a/Net/include/Poco/Net/UDPClient.h +++ b/Net/include/Poco/Net/UDPClient.h @@ -22,6 +22,7 @@ #include "Poco/Net/SocketAddress.h" #include "Poco/Net/DatagramSocket.h" #include "Poco/Runnable.h" +#include "Poco/RefCountedObject.h" #include "Poco/Thread.h" @@ -29,7 +30,7 @@ namespace Poco { namespace Net { -class Net_API UDPClient : public Poco::Runnable +class Net_API UDPClient : public Poco::Runnable, public RefCountedObject /// UDP client can either send, or send/receive UDP packets. /// The mode of operation is specified at construction time. /// If receiving functionality is enabled, it will run in a @@ -42,7 +43,7 @@ class Net_API UDPClient : public Poco::Runnable UDPClient(const std::string& address, Poco::UInt16 port, bool listen = false); /// Creates UDP client and connects it to specified address/port. /// If listen is true, a thread is launched where client can receive - /// responses rom the server. + /// responses from the server. ~UDPClient() override; /// Destroys UDPClient. diff --git a/Net/include/Poco/Net/UDPServer.h b/Net/include/Poco/Net/UDPServer.h index 10e135eec4..46947a11a4 100644 --- a/Net/include/Poco/Net/UDPServer.h +++ b/Net/include/Poco/Net/UDPServer.h @@ -37,6 +37,9 @@ class UDPServerImpl: public Poco::Runnable /// MultipleSocketPoller for more information. { public: + + using ServerParams = UDPServerParams; + UDPServerImpl(typename UDPHandlerImpl::List& handlers, const Poco::Net::SocketAddress& sa): _poller(handlers, sa), _thread("UDPServer"), @@ -47,7 +50,7 @@ class UDPServerImpl: public Poco::Runnable _thread.start(*this); } - UDPServerImpl(typename UDPHandlerImpl::List& handlers, const UDPServerParams& params): + UDPServerImpl(typename UDPHandlerImpl::List& handlers, const ServerParams& params): _poller(handlers, params), _thread("UDPServer"), _stop(false) diff --git a/Net/include/Poco/Net/UDPServerParams.h b/Net/include/Poco/Net/UDPServerParams.h index e734d56647..cfe8059ef4 100644 --- a/Net/include/Poco/Net/UDPServerParams.h +++ b/Net/include/Poco/Net/UDPServerParams.h @@ -33,7 +33,7 @@ class Net_API UDPServerParams public: UDPServerParams(const Poco::Net::SocketAddress& sa, int nSockets = 10, - Poco::Timespan timeout = 250000, + const Poco::Timespan& timeout = 250000, std::size_t handlerBufListSize = 1000, bool notifySender = false, int backlogThreshold = 10); diff --git a/Net/src/UDPServerParams.cpp b/Net/src/UDPServerParams.cpp index e7a0c6edcf..50c5f43973 100644 --- a/Net/src/UDPServerParams.cpp +++ b/Net/src/UDPServerParams.cpp @@ -21,7 +21,7 @@ namespace Net { UDPServerParams::UDPServerParams(const Poco::Net::SocketAddress& sa, int nSockets, - Poco::Timespan timeout, + const Timespan& timeout, std::size_t handlerBufListSize, bool notifySender, int backlogThreshold): _sa(sa), diff --git a/Net/testsuite/src/UDPServerTest.cpp b/Net/testsuite/src/UDPServerTest.cpp index 79894f0295..617f052041 100644 --- a/Net/testsuite/src/UDPServerTest.cpp +++ b/Net/testsuite/src/UDPServerTest.cpp @@ -12,6 +12,7 @@ #include "CppUnit/TestCaller.h" #include "CppUnit/TestSuite.h" #include "Poco/Net/UDPServer.h" +#include "Poco/Net/UDPServerParams.h" #include "Poco/Net/UDPClient.h" #include "Poco/Net/UDPHandler.h" #include "Poco/Net/DatagramSocket.h" @@ -83,16 +84,20 @@ namespace AtomicCounter TestUDPHandler::errors; template - bool server(int handlerCount, int reps, int port) + bool server(int handlerCount, int clientCount, int reps, const typename SRVT::ServerParams& params) { Poco::Net::UDPHandler::List handlers; for (int i = 0; i < handlerCount; ++i) handlers.push_back(new HNDLRT()); - SRVT server(handlers, Poco::Net::SocketAddress("127.0.0.1", port)); + SRVT server(handlers, params); Poco::Thread::sleep(100); - CLTT client("127.0.0.1", server.port(), true); + using ClientPtr = Poco::AutoPtr; + std::vector clients; + for (int i = 0; i < clientCount; i++) + clients.push_back( new CLTT("127.0.0.1", server.port(), true) ); + Poco::Thread::sleep(10); std::vector data; @@ -105,7 +110,7 @@ namespace { data.back().append(1, '\0'); std::size_t sz = (data.size() * strlen(str)) + 1; - int sent = client.send(Poco::Net::Socket::makeBufVec(data)); + int sent = clients[i % clientCount]->send(Poco::Net::Socket::makeBufVec(data)); if (sz != sent) { std::cerr << "Send count mismatch, expected: " << sz @@ -136,6 +141,7 @@ namespace << ", received: " << count << std::endl; return false; } + const auto address = clients[0]->address().toString(); for (const auto& he: handlers) { const auto &h = dynamic_cast(*he); @@ -148,12 +154,13 @@ namespace return false; } - if (h.counter && (h.addr.empty() || h.addr != client.address().toString())) - { - std::cerr << "Address mismatch, expected: " << client.address().toString() - << ", received: " << h.addr << std::endl; - return false; - } + if (clientCount == 1) + if (h.counter && (h.addr.empty() || h.addr != address)) + { + std::cerr << "Address mismatch, expected: " << address + << ", received: " << h.addr << std::endl; + return false; + } } return true; } @@ -173,9 +180,11 @@ UDPServerTest::~UDPServerTest() void UDPServerTest::testUDPSingleSocket() { TestUDPHandler::errors = 0; - int msgs = 10000; + int msgs = 10000; + Poco::Net::UDPServerParams params(Poco::Net::SocketAddress("127.0.0.1", 0)); + auto tf = server; - assertTrue( tf(1, msgs, 0) ); + assertTrue( tf(1, 1, msgs, params) ); assertTrue (TestUDPHandler::errors == 0); } @@ -184,8 +193,10 @@ void UDPServerTest::testUDPMultiSocket() { TestUDPHandler::errors = 0; int msgs = 10000; + Poco::Net::UDPServerParams params(Poco::Net::SocketAddress("127.0.0.1", 22080)); + auto tf = server; - assertTrue( tf(1, msgs, 22080) ); + assertTrue( tf(1, 1, msgs, params) ); assertTrue (TestUDPHandler::errors == 0); } @@ -194,8 +205,10 @@ void UDPServerTest::testUDPSingleSocketMultipleHandlers() { TestUDPHandler::errors = 0; int msgs = 10000; + Poco::Net::UDPServerParams params(Poco::Net::SocketAddress("127.0.0.1", 0)); + auto tf = server; - assertTrue( tf(10, msgs, 0) ); + assertTrue( tf(10, 1, msgs, params) ); assertTrue (TestUDPHandler::errors == 0); } @@ -204,8 +217,52 @@ void UDPServerTest::testUDPMultiSocketMultipleHandlers() { TestUDPHandler::errors = 0; int msgs = 10000; + Poco::Net::UDPServerParams params(Poco::Net::SocketAddress("127.0.0.1", 22080)); + + auto tf = server; + assertTrue( tf(10, 1, msgs, params) ); + assertTrue (TestUDPHandler::errors == 0); +} + + +void UDPServerTest::testUDPMultiSocketMultipleHandlersLessSockets() +{ + TestUDPHandler::errors = 0; + int msgs = 10000; + Poco::Net::UDPServerParams params( + Poco::Net::SocketAddress("127.0.0.1", 22080), + 2 + ); + auto tf = server; - assertTrue( tf(10, msgs, 22080) ); + assertTrue( tf(10, 1, msgs, params) ); + assertTrue (TestUDPHandler::errors == 0); +} + + +void UDPServerTest::testUDPMultiSocketMultipleHandlersMoreSockets() +{ + TestUDPHandler::errors = 0; + int msgs = 10000; + Poco::Net::UDPServerParams params( + Poco::Net::SocketAddress("127.0.0.1", 22080), + 10 + ); + + auto tf = server; + assertTrue( tf(2, 1, msgs, params) ); + assertTrue (TestUDPHandler::errors == 0); +} + + +void UDPServerTest::testUDPMultiSocketMultipleHandlersMultipleClients() +{ + TestUDPHandler::errors = 0; + int msgs = 50000; + Poco::Net::UDPServerParams params(Poco::Net::SocketAddress("127.0.0.1", 0)); + + auto tf = server; + assertTrue( tf(10, 5, msgs, params) ); assertTrue (TestUDPHandler::errors == 0); } @@ -228,6 +285,9 @@ CppUnit::Test* UDPServerTest::suite() CppUnit_addTest(pSuite, UDPServerTest, testUDPMultiSocket); CppUnit_addTest(pSuite, UDPServerTest, testUDPSingleSocketMultipleHandlers); CppUnit_addTest(pSuite, UDPServerTest, testUDPMultiSocketMultipleHandlers); + CppUnit_addTest(pSuite, UDPServerTest, testUDPMultiSocketMultipleHandlersLessSockets); + CppUnit_addTest(pSuite, UDPServerTest, testUDPMultiSocketMultipleHandlersMoreSockets); + CppUnit_addTest(pSuite, UDPServerTest, testUDPMultiSocketMultipleHandlersMultipleClients); return pSuite; } diff --git a/Net/testsuite/src/UDPServerTest.h b/Net/testsuite/src/UDPServerTest.h index 26d05aaa8d..37e041733e 100644 --- a/Net/testsuite/src/UDPServerTest.h +++ b/Net/testsuite/src/UDPServerTest.h @@ -27,6 +27,9 @@ class UDPServerTest: public CppUnit::TestCase void testUDPMultiSocket(); void testUDPSingleSocketMultipleHandlers(); void testUDPMultiSocketMultipleHandlers(); + void testUDPMultiSocketMultipleHandlersLessSockets(); + void testUDPMultiSocketMultipleHandlersMoreSockets(); + void testUDPMultiSocketMultipleHandlersMultipleClients(); void setUp(); void tearDown(); From 173997964c976de6a11b4b9e10132f2f72c988db Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Tue, 19 Mar 2024 19:52:31 +0100 Subject: [PATCH 7/8] enh(UDPHandler): helper functions for data buffer member offsets --- Net/include/Poco/Net/UDPHandler.h | 36 ++++++++++++++++++++------ Net/include/Poco/Net/UDPSocketReader.h | 6 ++--- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/Net/include/Poco/Net/UDPHandler.h b/Net/include/Poco/Net/UDPHandler.h index 49940db053..faf294aaae 100644 --- a/Net/include/Poco/Net/UDPHandler.h +++ b/Net/include/Poco/Net/UDPHandler.h @@ -225,13 +225,13 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject AtomicCounter::ValueType setError(char* pBuf, const std::string& err) /// Sets the error into the buffer. { - std::size_t availLen = S - sizeof(MsgSizeT); - std::memset(pBuf + sizeof(MsgSizeT), 0, availLen); + std::size_t availLen = S - errorOffset(); + std::memset(pBuf + errorOffset(), 0, availLen); std::size_t msgLen = err.length(); if (msgLen) { if (msgLen >= availLen) msgLen = availLen; - std::memcpy(pBuf + sizeof(MsgSizeT), err.data(), msgLen); + std::memcpy(pBuf + errorOffset(), err.data(), msgLen); } setStatus(pBuf, BUF_STATUS_ERROR); return --_errorBacklog; @@ -251,10 +251,28 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject return *reinterpret_cast(pBuf) == BUF_STATUS_ERROR; } - static Poco::UInt16 offset() + static constexpr Poco::UInt16 errorOffset() + /// Returns offset of address length. + { + return sizeof(MsgSizeT); + } + + static constexpr Poco::UInt16 addressLengthOffset() + /// Returns offset of address length. + { + return sizeof(MsgSizeT); + } + + static constexpr Poco::UInt16 addressOffset() + /// Returns offset of address. + { + return addressLengthOffset() + sizeof(poco_socklen_t); + } + + static constexpr Poco::UInt16 payloadOffset() /// Returns buffer data offset. { - return sizeof(MsgSizeT) + sizeof(poco_socklen_t) + SocketAddress::MAX_ADDRESS_LENGTH; + return addressOffset() + SocketAddress::MAX_ADDRESS_LENGTH; } static MsgSizeT payloadSize(const char* buf) @@ -264,8 +282,10 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject static SocketAddress address(const char* buf) { - const auto* len = reinterpret_cast(buf + sizeof(MsgSizeT)); - const auto* pSA = reinterpret_cast(buf + sizeof(MsgSizeT) + sizeof(poco_socklen_t)); + const auto* len = reinterpret_cast(buf + addressLengthOffset()); + const auto* pSA = reinterpret_cast(buf + addressOffset()); + poco_assert(*len <= SocketAddress::MAX_ADDRESS_LENGTH); + return SocketAddress(pSA, *len); } @@ -280,7 +300,7 @@ class UDPHandlerImpl: public Runnable, public RefCountedObject /// | sizeof(MsgSizeT) bytes | sizeof(poco_socklen_t) | SocketAddress::MAX_ADDRESS_LENGTH | payload | /// +------------------------+------------------------+-----------------------------------+--------- ~ ---+ { - return buf + offset(); + return buf + payloadOffset(); } static Poco::StringTokenizer payload(const char* buf, char delimiter) diff --git a/Net/include/Poco/Net/UDPSocketReader.h b/Net/include/Poco/Net/UDPSocketReader.h index 62331ceeaa..b1105cfbde 100644 --- a/Net/include/Poco/Net/UDPSocketReader.h +++ b/Net/include/Poco/Net/UDPSocketReader.h @@ -112,10 +112,10 @@ class UDPSocketReader p = handler().next(sockfd); if (p) { - Poco::UInt16 off = handler().offset(); - poco_socklen_t* pAL = reinterpret_cast(p + sizeof(RT)); + Poco::UInt16 off = handler().payloadOffset(); + auto* pAL = reinterpret_cast(p + handler().addressLengthOffset()); *pAL = SocketAddress::MAX_ADDRESS_LENGTH; - struct sockaddr* pSA = reinterpret_cast(p + sizeof(RT) + sizeof(poco_socklen_t)); + struct sockaddr* pSA = reinterpret_cast(p + handler().addressOffset()); RT ret = sock.receiveFrom(p + off, S - off - 1, &pSA, &pAL); if (ret < 0) { From 32b2f720b34bb415e1e52bbef6cab213f58db4a7 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Tue, 19 Mar 2024 21:14:57 +0100 Subject: [PATCH 8/8] fix(UDPSocketReader): resolve security warnings reported by CodeQL --- Net/include/Poco/Net/UDPSocketReader.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Net/include/Poco/Net/UDPSocketReader.h b/Net/include/Poco/Net/UDPSocketReader.h index b1105cfbde..e177e3efbf 100644 --- a/Net/include/Poco/Net/UDPSocketReader.h +++ b/Net/include/Poco/Net/UDPSocketReader.h @@ -113,9 +113,9 @@ class UDPSocketReader if (p) { Poco::UInt16 off = handler().payloadOffset(); - auto* pAL = reinterpret_cast(p + handler().addressLengthOffset()); + pAL = reinterpret_cast(p + handler().addressLengthOffset()); *pAL = SocketAddress::MAX_ADDRESS_LENGTH; - struct sockaddr* pSA = reinterpret_cast(p + handler().addressOffset()); + pSA = reinterpret_cast(p + handler().addressOffset()); RT ret = sock.receiveFrom(p + off, S - off - 1, &pSA, &pAL); if (ret < 0) {