From 661e6c9796ac88d9f69bb8b7dc32937c329df9d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= Date: Fri, 26 Jun 2026 16:49:08 +0200 Subject: [PATCH] clearer msg --- slsSupportLib/include/sls/string_utils.h | 2 + slsSupportLib/src/ClientSocket.cpp | 19 ++-- slsSupportLib/src/DataSocket.cpp | 4 +- slsSupportLib/src/string_utils.cpp | 7 ++ slsSupportLib/tests/test-Sockets.cpp | 110 +++++++++++++++++++++- slsSupportLib/tests/test-string_utils.cpp | 27 ++++++ 6 files changed, 159 insertions(+), 10 deletions(-) diff --git a/slsSupportLib/include/sls/string_utils.h b/slsSupportLib/include/sls/string_utils.h index dc1c037ac..abeed79a7 100644 --- a/slsSupportLib/include/sls/string_utils.h +++ b/slsSupportLib/include/sls/string_utils.h @@ -96,4 +96,6 @@ bool replace_first(std::string *s, const std::string &substr, std::pair ParseHostPort(const std::string &s); +std::string to_lower(const std::string &s); + } // namespace sls diff --git a/slsSupportLib/src/ClientSocket.cpp b/slsSupportLib/src/ClientSocket.cpp index e31a4e044..6dc6aac42 100644 --- a/slsSupportLib/src/ClientSocket.cpp +++ b/slsSupportLib/src/ClientSocket.cpp @@ -5,9 +5,11 @@ #include "sls/sls_detector_defs.h" #include "sls/sls_detector_exceptions.h" #include "sls/sls_detector_funcs.h" +#include "sls/string_utils.h" #include #include #include +#include #include #include #include @@ -24,8 +26,9 @@ ClientSocket::ClientSocket(std::string stype, const std::string &host, hints.ai_flags |= AI_CANONNAME; if (getaddrinfo(host.c_str(), nullptr, &hints, &result) != 0) { - std::string msg = socketType + " cannot decode host:" + host + - " on port " + std::to_string(port) + "\n"; + + + auto msg = fmt::format("Cannot resolve {} hostname: '{}'", to_lower(socketType), host); throwError(msg); } @@ -40,8 +43,10 @@ ClientSocket::ClientSocket(std::string stype, const std::string &host, if (::connect(getSocketId(), (struct sockaddr *)&serverAddr, sizeof(serverAddr)) != 0) { freeaddrinfo(result); - std::string msg = socketType + ": Cannot connect to " + host + - " on port " + std::to_string(port) + "\n"; + auto msg = fmt::format( + "Cannot connect to {} on {}:{}\n", + to_lower(socketType), host, port); + throwError(msg); } freeaddrinfo(result); @@ -53,8 +58,7 @@ ClientSocket::ClientSocket(std::string sType, struct sockaddr_in addr) if (::connect(getSocketId(), (struct sockaddr *)&addr, sizeof(addr)) != 0) { char address[INET_ADDRSTRLEN]; inet_ntop(AF_INET, &addr.sin_addr, address, INET_ADDRSTRLEN); - std::string msg = socketType + ": Cannot connect to " + address + - " on port " + std::to_string(addr.sin_port) + "\n"; + auto msg = fmt::format("Cannot connect to {} on {}:{}", to_lower(socketType), address, addr.sin_port); throwError(msg); } } @@ -95,7 +99,8 @@ void ClientSocket::readReply(int &ret, void *retval, size_t retval_size) { } // debugging catch (SocketError &e) { - throwError(socketType + " Socket Error: " + std::string(e.what())); + auto msg = fmt::format("While reading reply from {} {}", to_lower(socketType), e.what()); + throwError(msg); } } diff --git a/slsSupportLib/src/DataSocket.cpp b/slsSupportLib/src/DataSocket.cpp index 848dcca16..b0c01e875 100644 --- a/slsSupportLib/src/DataSocket.cpp +++ b/slsSupportLib/src/DataSocket.cpp @@ -150,7 +150,9 @@ int DataSocket::setTimeOut(int t_seconds) { void DataSocket::close() { if (sockfd_ > 0) { if (::close(sockfd_)) { - throw SocketError("could not close socket"); + std::stringstream ss; + ss << "could not close socket (fd: " << sockfd_ << ")"; + throw SocketError(ss.str()); } sockfd_ = -1; } else { diff --git a/slsSupportLib/src/string_utils.cpp b/slsSupportLib/src/string_utils.cpp index 1426049ff..68e389c99 100644 --- a/slsSupportLib/src/string_utils.cpp +++ b/slsSupportLib/src/string_utils.cpp @@ -75,4 +75,11 @@ std::pair ParseHostPort(const std::string &s) { return std::make_pair(host, port); } +std::string to_lower(const std::string &s) { + std::string result = s; + std::transform(result.begin(), result.end(), result.begin(), + [](unsigned char c) { return std::tolower(c); }); + return result; +} + }; // namespace sls \ No newline at end of file diff --git a/slsSupportLib/tests/test-Sockets.cpp b/slsSupportLib/tests/test-Sockets.cpp index c1826bcca..eb627aaad 100644 --- a/slsSupportLib/tests/test-Sockets.cpp +++ b/slsSupportLib/tests/test-Sockets.cpp @@ -3,6 +3,7 @@ #include "catch.hpp" #include "sls/ClientSocket.h" #include "sls/ServerSocket.h" +#include "sls/sls_detector_defs.h" #include "sls/sls_detector_exceptions.h" #include "sls/sls_detector_funcs.h" #include @@ -33,7 +34,7 @@ std::vector echo_server(uint16_t port, size_t bytes_to_send, auto fd = s.getSocketId(); setsockopt(fd, SOL_SOCKET, SO_LINGER, &ling, sizeof ling); - close(fd); + ::close(fd); return buffer; } @@ -48,6 +49,42 @@ std::vector echo_server(uint16_t port, size_t bytes_to_send, return buffer; } +// Minimal command server speaking the same protocol as sendCommandThenRead: +// accept a connection, read the function number and a single int argument, +// then reply via ServerInterface::sendResult with OK and (arg * 2). Returns +// the {fnum, arg} pair the server received so the test can verify them. +std::pair command_server(uint16_t port) { + auto server = ServerSocket(port); + auto s = server.accept(); + int fnum = -1; + int arg = 0; + s.Receive(&fnum, sizeof(fnum)); + s.Receive(&arg, sizeof(arg)); + int retval = arg * 2; + s.sendResult(slsDetectorDefs::OK, &retval, sizeof(retval)); + s.close(); + return {fnum, arg}; +} + +// Command server that replies with a truncated message: it reads the request, +// sends the OK return code, but then sends fewer retval bytes than the client +// expects before closing, so the client's Receive hits EOF mid-read. +void short_reply_server(uint16_t port, size_t retval_bytes_to_send) { + auto server = ServerSocket(port); + auto s = server.accept(); + int fnum = -1; + int arg = 0; + s.Receive(&fnum, sizeof(fnum)); + s.Receive(&arg, sizeof(arg)); + int ret = slsDetectorDefs::OK; + s.Send(&ret, sizeof(ret)); + if (retval_bytes_to_send > 0) { + std::vector partial(retval_bytes_to_send, '\0'); + s.Send(partial.data(), partial.size()); + } + s.close(); +} + TEST_CASE("The server recive the same message as we send", "[support]") { std::vector received_message(100, '\0'); std::vector sent_message(100, '\0'); @@ -70,6 +107,8 @@ TEST_CASE("The server recive the same message as we send", "[support]") { TEST_CASE("throws on no server", "[support]") { CHECK_THROWS(DetectorSocket("localhost", 1950)); + CHECK_THROWS(ReceiverSocket("localhost", 1950)); + CHECK_THROWS(GuiSocket("localhost", 1950)); } TEST_CASE("Receiving a too short message throws and reports EOF", "[support]") { @@ -154,11 +193,78 @@ TEST_CASE("Socket crash?", "[support]") { REQUIRE_THROWS(client.Receive(received_message.data(), received_message.size())); - // client.close(); + + //Now try to send more + // client.Send(sent_message.data(), sent_message.size()); } +TEST_CASE("ClientSocket throws on invalid hostname", "[support]") { + CHECK_THROWS(ReceiverSocket("invalidhostname", 1950)); + CHECK_THROWS(DetectorSocket("invalidhostname", 1950)); + CHECK_THROWS(GuiSocket("invalidhostname", 1950)); +} + + +TEST_CASE("Using DetectorSocket to talk to a Server Socket", "[support]") { + constexpr uint16_t port = 1961; + constexpr int fnum = F_GET_DETECTOR_TYPE; + constexpr int arg = 21; + + auto s = std::async(std::launch::async, command_server, port); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + auto client = DetectorSocket("localhost", port); + int retval = 0; + int ret = + client.sendCommandThenRead(fnum, &arg, sizeof(arg), &retval, + sizeof(retval)); + client.close(); + + auto server_received = s.get(); + + // Client got OK and the expected return value back from the server + CHECK(ret == slsDetectorDefs::OK); + CHECK(retval == arg * 2); + // Server received the function number and argument we sent + CHECK(server_received.first == fnum); + CHECK(server_received.second == arg); + // close() resets the underlying fd + CHECK(client.getSocketId() == -1); +} + +TEST_CASE("ServerSocket replies with a too short message", "[support]") { + constexpr uint16_t port = 1962; + constexpr int fnum = F_GET_DETECTOR_TYPE; + constexpr int arg = 21; + + // Server sends the OK return code, then only 1 of the 4 expected retval + // bytes before closing. + auto s = std::async(std::launch::async, short_reply_server, port, 1); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + auto client = DetectorSocket("localhost", port); + int retval = 0; + + std::string error_message; + try { + client.sendCommandThenRead(fnum, &arg, sizeof(arg), &retval, + sizeof(retval)); + FAIL("sendCommandThenRead should have thrown on a too short message"); + } catch (const DetectorError &e) { + error_message = e.what(); + } + client.close(); + s.get(); + + // The client read only 1 of the 4 expected retval bytes and then hit EOF. + CHECK_THAT(error_message, + Catch::Matchers::Contains("read 1 bytes instead of 4 bytes")); + CHECK_THAT(error_message, + Catch::Matchers::Contains("connection closed by peer (EOF)")); +} + } // namespace sls diff --git a/slsSupportLib/tests/test-string_utils.cpp b/slsSupportLib/tests/test-string_utils.cpp index 15dcb7dbc..21200eca9 100644 --- a/slsSupportLib/tests/test-string_utils.cpp +++ b/slsSupportLib/tests/test-string_utils.cpp @@ -123,6 +123,33 @@ TEST_CASE("port missing") { REQUIRE(res.second == 0); } +TEST_CASE("to_lower converts uppercase to lowercase") { + REQUIRE(to_lower("HELLO") == "hello"); + REQUIRE(to_lower("Hello World") == "hello world"); +} + +TEST_CASE("to_lower leaves an already lowercase string unchanged") { + REQUIRE(to_lower("already lower") == "already lower"); +} + +TEST_CASE("to_lower only affects alphabetic characters") { + REQUIRE(to_lower("ABC123!?_-XYZ") == "abc123!?_-xyz"); +} + +TEST_CASE("to_lower on an empty string returns an empty string") { + REQUIRE(to_lower("").empty()); +} + +TEST_CASE("to_lower does not modify the original string") { + std::string original = "MixedCase"; + auto result = to_lower(original); + REQUIRE(result == "mixedcase"); + // the source string must be untouched + REQUIRE(original == "MixedCase"); +} + + + // TEST_CASE("concat things not being strings") } // namespace sls