From f1e108db29251c73ca0108c9a6fcbb7b74a08df0 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sat, 28 Dec 2024 08:33:11 -0500 Subject: [PATCH 1/9] wsd: cosmetics Change-Id: Ib1c72b73e892f2416c2493e4bf03736e07a459d1 Signed-off-by: Ashod Nakashian --- net/Socket.cpp | 1 + net/Socket.hpp | 3 ++- net/SslSocket.hpp | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/net/Socket.cpp b/net/Socket.cpp index b67dcb93cbbb..f7bf2de01102 100644 --- a/net/Socket.cpp +++ b/net/Socket.cpp @@ -1234,6 +1234,7 @@ std::shared_ptr ServerSocket::accept() ::close(rc); return nullptr; } + try { // Create a socket object using the factory. diff --git a/net/Socket.hpp b/net/Socket.hpp index eb1cf73661ed..c0bed52d022f 100644 --- a/net/Socket.hpp +++ b/net/Socket.hpp @@ -454,7 +454,8 @@ class Socket _noShutdown = false; _sendBufferSize = DefaultSendBufferSize; _owner = std::this_thread::get_id(); - LOG_TRC("Created socket. Thread affinity set to " << Log::to_string(_owner) << ", " << toStringImpl()); + LOG_DBG("Created socket. Thread affinity set to " << Log::to_string(_owner) << ", " + << toStringImpl()); if constexpr (!Util::isMobileApp()) { diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp index fd428c96c887..536e3de1fecd 100644 --- a/net/SslSocket.hpp +++ b/net/SslSocket.hpp @@ -368,7 +368,7 @@ class SslStreamSocket final : public StreamSocket return rc; } - // Fallthrough... + [[fallthrough]]; default: { // Effectively an EAGAIN error at the BIO layer From 116f1abb805ad193d289bcbb98289a6759e2c843 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Fri, 3 Jan 2025 06:29:23 -0500 Subject: [PATCH 2/9] wsd: use non-deprecated SSL_get1_peer_certificate SSL_get1_peer_certificate doesn't auto-release the x509 instance, and lets us handle the lifetime, which is what we need. The now-deprecated SSL_get_peer_certificate was already mapped to SSL_get1_peer_certificate. Change-Id: Ic57586d114ec66bedb48005eb8cdf304565d8a6a Signed-off-by: Ashod Nakashian --- net/Socket.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/Socket.cpp b/net/Socket.cpp index f7bf2de01102..eba8819422c2 100644 --- a/net/Socket.cpp +++ b/net/Socket.cpp @@ -230,7 +230,7 @@ bool SslStreamSocket::verifyCertificate() } LOG_TRC("Verifying certificate of [" << hostname() << ']'); - X509* x509 = SSL_get_peer_certificate(_ssl); + X509* x509 = SSL_get1_peer_certificate(_ssl); if (x509) { // Dump cert info, for debugging only. @@ -272,7 +272,7 @@ bool SslStreamSocket::verifyCertificate() std::string SslStreamSocket::getSslCert(std::string& subjectHash) { std::ostringstream strstream; - if (X509* x509 = SSL_get_peer_certificate(_ssl)) + if (X509* x509 = SSL_get1_peer_certificate(_ssl)) { Poco::Net::X509Certificate cert(x509); cert.save(strstream); From 85fba3d8d509cdfce97c4549bd472f963a127943 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Fri, 3 Jan 2025 06:49:10 -0500 Subject: [PATCH 3/9] wsd: string -> string_view Change-Id: Ic6c0621d009c18a6789527add65c2fd2ebb701d9 Signed-off-by: Ashod Nakashian --- net/SslSocket.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp index 536e3de1fecd..590ec58775f4 100644 --- a/net/SslSocket.hpp +++ b/net/SslSocket.hpp @@ -473,7 +473,7 @@ class SslStreamSocket final : public StreamSocket auto cb = [](const char* str, size_t len, void* u) -> int { std::ostringstream& os = *reinterpret_cast(u); - os << '\n' << std::string(str, len); + os << '\n' << std::string_view(str, len); return 1; // Apparently 0 means failure here. }; From 66b22fa9760144f9aa6f33b39eec7844e96f9fe8 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sat, 4 Jan 2025 08:27:40 -0500 Subject: [PATCH 4/9] wsd: include cleanup Change-Id: I97cda4267af25791fa54e18c05958a7c131f15c0 Signed-off-by: Ashod Nakashian --- net/Ssl.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/net/Ssl.cpp b/net/Ssl.cpp index 288933e6acee..cf362c42a428 100644 --- a/net/Ssl.cpp +++ b/net/Ssl.cpp @@ -11,17 +11,18 @@ #include -#include #include "Ssl.hpp" +#include +#include + +#include +#include + #ifdef __FreeBSD__ #include #endif -#include -#include -#include - extern "C" { // Multithreading support for OpenSSL. From 91a82a9c08b525813d7c0c0fe9c091129e6ec4bf Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Fri, 3 Jan 2025 07:29:46 -0500 Subject: [PATCH 5/9] wsd: log the OpenSSL version Change-Id: Ibd5eb673b81db4aa56439c6463ccae61e7c8cf6a Signed-off-by: Ashod Nakashian --- net/Ssl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/Ssl.cpp b/net/Ssl.cpp index cf362c42a428..78bd0d9dbcd9 100644 --- a/net/Ssl.cpp +++ b/net/Ssl.cpp @@ -109,6 +109,8 @@ SslContext::SslContext(const std::string& certFilePath, const std::string& keyFi : _ctx(nullptr) , _verification(verification) { + LOG_INF("Initializing " << OPENSSL_VERSION_TEXT); + const std::vector rand = Util::rng::getBytes(512); RAND_seed(rand.data(), rand.size()); From a3d969d0d5e8efa603423e933f9bce38dff3d549 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sat, 4 Jan 2025 17:33:26 -0500 Subject: [PATCH 6/9] wsd: as of C++17 [[fallthrough]] is standard Change-Id: I16f4639feb6978f847a762a88a4de579db5476b3 Signed-off-by: Ashod Nakashian --- common/SpookyV2.cpp | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/common/SpookyV2.cpp b/common/SpookyV2.cpp index 4eff9a5afba9..a7aa9e093871 100644 --- a/common/SpookyV2.cpp +++ b/common/SpookyV2.cpp @@ -16,13 +16,6 @@ #define ALLOW_UNALIGNED_READS true -#if defined __clang__ -#define FALLTHROUGH [[clang::fallthrough]] -#elif defined __GNUC__ && __GNUC__ >= 7 -#define FALLTHROUGH [[fallthrough]] -#else -#define FALLTHROUGH -#endif // // short hash ... it could be used on any message, // but it's used by Spooky just for short messages. @@ -86,48 +79,48 @@ void SpookyHash::Short( switch (remainder) { case 15: - d += ((uint64)u.p8[14]) << 48; - FALLTHROUGH; + d += ((uint64)u.p8[14]) << 48; + [[fallthrough]]; case 14: d += ((uint64)u.p8[13]) << 40; - FALLTHROUGH; + [[fallthrough]]; case 13: d += ((uint64)u.p8[12]) << 32; - FALLTHROUGH; + [[fallthrough]]; case 12: d += u.p32[2]; c += u.p64[0]; break; case 11: d += ((uint64)u.p8[10]) << 16; - FALLTHROUGH; + [[fallthrough]]; case 10: d += ((uint64)u.p8[9]) << 8; - FALLTHROUGH; + [[fallthrough]]; case 9: d += (uint64)u.p8[8]; - FALLTHROUGH; + [[fallthrough]]; case 8: c += u.p64[0]; break; case 7: c += ((uint64)u.p8[6]) << 48; - FALLTHROUGH; + [[fallthrough]]; case 6: c += ((uint64)u.p8[5]) << 40; - FALLTHROUGH; + [[fallthrough]]; case 5: c += ((uint64)u.p8[4]) << 32; - FALLTHROUGH; + [[fallthrough]]; case 4: c += u.p32[0]; break; case 3: c += ((uint64)u.p8[2]) << 16; - FALLTHROUGH; + [[fallthrough]]; case 2: c += ((uint64)u.p8[1]) << 8; - FALLTHROUGH; + [[fallthrough]]; case 1: c += (uint64)u.p8[0]; break; From 8538a559b2589ab81ef882c85c1433a597fc7f3b Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sun, 5 Jan 2025 11:35:10 -0500 Subject: [PATCH 7/9] wsd: constexpr string_view Change-Id: I9b333b4e9588c418e7160b843dfcd10b5542198f Signed-off-by: Ashod Nakashian --- common/Util.cpp | 6 +++--- common/Util.hpp | 2 +- kit/ChildSession.cpp | 9 +++++---- kit/Kit.cpp | 2 +- net/Socket.cpp | 2 +- tools/WebSocketDump.cpp | 2 +- wsd/ClientSession.cpp | 2 +- wsd/DocumentBroker.cpp | 4 ++-- wsd/Storage.hpp | 3 ++- wsd/wopi/StorageConnectionManager.hpp | 2 +- 10 files changed, 18 insertions(+), 16 deletions(-) diff --git a/common/Util.cpp b/common/Util.cpp index ee5a10460efb..3ba7cba73eac 100644 --- a/common/Util.cpp +++ b/common/Util.cpp @@ -217,7 +217,7 @@ namespace Util return result; } - std::string replaceAllOf(const std::string &str, const std::string& match, const std::string& repl) + std::string replaceAllOf(std::string_view str, std::string_view match, std::string_view repl) { std::ostringstream os; @@ -240,8 +240,8 @@ namespace Util std::string cleanupFilename(const std::string &filename) { - static const std::string mtch(",/?:@&=+$#'\""); - static const std::string repl("------------"); + constexpr std::string_view mtch(",/?:@&=+$#'\""); + constexpr std::string_view repl("------------"); return replaceAllOf(filename, mtch, repl); } diff --git a/common/Util.hpp b/common/Util.hpp index fb16b5c153bb..cff834bc8c2a 100644 --- a/common/Util.hpp +++ b/common/Util.hpp @@ -370,7 +370,7 @@ namespace Util std::string replace(std::string s, const std::string& a, const std::string& b); /// Replace any characters in @a matching characters in @b with replacement chars in @c and return - std::string replaceAllOf(const std::string &str, const std::string& match, const std::string& repl); + std::string replaceAllOf(std::string_view str, std::string_view match, std::string_view repl); std::string formatLinesForLog(const std::string& s); diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp index eea5aadfb77d..57af3ab4f1fc 100644 --- a/kit/ChildSession.cpp +++ b/kit/ChildSession.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #define LOK_USE_UNSTABLE_API #include @@ -2143,8 +2144,8 @@ bool ChildSession::renderSearchResult(const char* buffer, int length, const Stri if (Png::encodeBufferToPNG(bitmapBuffer, width, height, output, eTileMode)) { - static const std::string header = "rendersearchresult:\n"; - size_t responseSize = header.size() + output.size(); + constexpr std::string_view header = "rendersearchresult:\n"; + const size_t responseSize = header.size() + output.size(); std::vector response(responseSize); std::copy(header.begin(), header.end(), response.begin()); std::copy(output.begin(), output.end(), response.begin() + header.size()); @@ -3111,8 +3112,8 @@ bool ChildSession::renderShapeSelection(const StringVector& tokens) const std::size_t outputSize = getLOKitDocument()->renderShapeSelection(&output); if (output != nullptr && outputSize > 0) { - static const std::string header = "shapeselectioncontent:\n"; - size_t responseSize = header.size() + outputSize; + constexpr std::string_view header = "shapeselectioncontent:\n"; + const size_t responseSize = header.size() + outputSize; std::unique_ptr response(new char[responseSize]); std::memcpy(response.get(), header.data(), header.size()); std::memcpy(response.get() + header.size(), output, outputSize); diff --git a/kit/Kit.cpp b/kit/Kit.cpp index 96b76462f556..9e14c92980c6 100644 --- a/kit/Kit.cpp +++ b/kit/Kit.cpp @@ -2103,7 +2103,7 @@ bool Document::forwardToChild(const std::string& prefix, const std::vector { std::shared_ptr session = it->second; - static const std::string disconnect("disconnect"); + constexpr std::string_view disconnect("disconnect"); if (size == disconnect.size() && strncmp(data, disconnect.data(), disconnect.size()) == 0) { diff --git a/net/Socket.cpp b/net/Socket.cpp index eba8819422c2..bceb3dee885d 100644 --- a/net/Socket.cpp +++ b/net/Socket.cpp @@ -1519,7 +1519,7 @@ bool StreamSocket::parseHeader(const char *clientName, std::chrono::duration delayMs = now - lastHTTPHeader; // Find the end of the header, if any. - static const std::string marker("\r\n\r\n"); + constexpr std::string_view marker("\r\n\r\n"); auto itBody = std::search(_inBuffer.begin(), _inBuffer.end(), marker.begin(), marker.end()); if (itBody == _inBuffer.end()) { diff --git a/tools/WebSocketDump.cpp b/tools/WebSocketDump.cpp index fa2c7e675d96..c44515bed728 100644 --- a/tools/WebSocketDump.cpp +++ b/tools/WebSocketDump.cpp @@ -85,7 +85,7 @@ class ClientRequestDispatcher final : public SimpleSocketHandler LOG_TRC('#' << socket->getFD() << " handling incoming " << in.size() << " bytes."); // Find the end of the header, if any. - static const std::string marker("\r\n\r\n"); + constexpr std::string_view marker("\r\n\r\n"); auto itBody = std::search(in.begin(), in.end(), marker.begin(), marker.end()); if (itBody == in.end()) diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index ac32e3ad8ef3..a450073cc16e 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -3151,7 +3151,7 @@ std::string ClientSession::processSVGContent(const std::string& svg) std::string::size_type pos = 0; for (;;) { - static const std::string prefix = "src=\"file:///tmp/"; + constexpr std::string_view prefix = "src=\"file:///tmp/"; const auto start = svg.find(prefix, pos); if (start == std::string::npos) { diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index b52c193d2244..61e3aa35eca6 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -4677,8 +4677,8 @@ void DocumentBroker::onUrpMessage(const char* data, size_t len) const auto session = getWriteableSession(); if (session) { - static const std::string header = "urp: "; - size_t responseSize = header.size() + len; + constexpr std::string_view header = "urp: "; + const size_t responseSize = header.size() + len; std::vector response(responseSize); std::memcpy(response.data(), header.data(), header.size()); std::memcpy(response.data() + header.size(), data, len); diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp index 29f7f565e8a6..8912d10d2e5d 100644 --- a/wsd/Storage.hpp +++ b/wsd/Storage.hpp @@ -23,6 +23,7 @@ #include #include +#include #include class LockContext; @@ -454,7 +455,7 @@ class StorageBase /// Sanitize a URI by removing authorization tokens. void sanitizeUri(Poco::URI& uri) { - static const std::string access_token("access_token"); + constexpr std::string_view access_token("access_token"); Poco::URI::QueryParameters queryParams = uri.getQueryParameters(); for (auto& param : queryParams) diff --git a/wsd/wopi/StorageConnectionManager.hpp b/wsd/wopi/StorageConnectionManager.hpp index 25356080a417..ad0ea19ec51c 100644 --- a/wsd/wopi/StorageConnectionManager.hpp +++ b/wsd/wopi/StorageConnectionManager.hpp @@ -58,7 +58,7 @@ class StorageConnectionManager final /// Sanitize a URI by removing authorization tokens. static Poco::URI sanitizeUri(Poco::URI uri) { - static const std::string access_token("access_token"); + constexpr std::string_view access_token("access_token"); Poco::URI::QueryParameters queryParams = uri.getQueryParameters(); for (auto& param : queryParams) From 4a409643c0e804cf6d26fd51608025cc77df2519 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sun, 5 Jan 2025 21:38:53 -0500 Subject: [PATCH 8/9] wsd: rename right-trimming helper to rtrim Change-Id: Ia9a7a6a856b09d8679fa46ee2dd3dde685baad75 Signed-off-by: Ashod Nakashian --- common/JailUtil.cpp | 4 ++-- common/Util.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/JailUtil.cpp b/common/JailUtil.cpp index 4a0514db0216..6d39f6c842ef 100644 --- a/common/JailUtil.cpp +++ b/common/JailUtil.cpp @@ -119,8 +119,8 @@ bool enterUserNS(uid_t uid, gid_t gid) bool coolmount(const std::string& arg, std::string source, std::string target, bool silent = false) { - source = Util::trim(source, '/'); - target = Util::trim(target, '/'); + source = Util::rtrim(source, '/'); + target = Util::rtrim(target, '/'); if (isMountNamespacesEnabled()) { diff --git a/common/Util.hpp b/common/Util.hpp index cff834bc8c2a..8265e34abc06 100644 --- a/common/Util.hpp +++ b/common/Util.hpp @@ -608,7 +608,7 @@ namespace Util size_t findInVector(const std::vector& tokens, const char *cstring, std::size_t offset = 0); /// Trim trailing characters (on the right). - inline std::string_view trim(const std::string_view s, const char ch) + inline std::string_view rtrim(const std::string_view s, const char ch) { const size_t last = s.find_last_not_of(ch); if (last != std::string::npos) From 1aa51e6c8432f72823483516ec0b8b0d2fbfcb5a Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sat, 4 Jan 2025 10:20:26 -0500 Subject: [PATCH 9/9] wsd: net: include cleanup Change-Id: Ia10ae28a0cf9f4e5c5345dbc60da71a341d63e26 Signed-off-by: Ashod Nakashian --- net/SslSocket.hpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp index 590ec58775f4..8b1e6553fd2e 100644 --- a/net/SslSocket.hpp +++ b/net/SslSocket.hpp @@ -11,11 +11,10 @@ #pragma once -#include "Ssl.hpp" -#include "Socket.hpp" +#include +#include #include -#include #include #include