Misc Improvements

common/Util.cpp

@@ -773,9 +773,9 @@ namespace Util
 
     std::string getValue(const std::map<std::string, std::string>& map, const std::string& subject)
     {
-        if (map.find(subject) != map.end())
+        if (const auto& it = map.find(subject); it != map.end())
         {
-            return map.at(subject);
+            return it->second;
         }
 
         // Not a perfect match, try regex.

net/HttpHelper.hpp

@@ -46,7 +46,7 @@ void sendFile(const std::shared_ptr<StreamSocket>& socket, const std::string& pa
 /// The idea is to only warn in release builds, but to help developers in debug builds.
 /// Returns false only in debug build.
 inline bool verifyWOPISrc(const std::string& uri, const std::string& wopiSrc,
-                          const std::shared_ptr<StreamSocket>& socket = {})
+                          [[maybe_unused]] const std::shared_ptr<StreamSocket>& socket = {})
 {
     // getQueryParameters(), which is used to extract wopiSrc, decodes the values.
     // Compare with the URI. WopiSrc is complex enough to require encoding.

net/Socket.cpp

@@ -1540,7 +1540,7 @@ bool StreamSocket::parseHeader(const char *clientName,
         const std::streamsize available = _inBuffer.size() - offset;
 
         LOG_INF("parseHeader: " << clientName << " HTTP Request: " << request.getMethod()
-                                << ", uri `" << request.getURI() << "` " << request.getVersion()
+                                << ", uri: [" << request.getURI() << "] " << request.getVersion()
                                 << ", sz[header " << map._headerSize << ", content "
                                 << contentLength << "], offset " << offset << ", chunked "
                                 << request.getChunkedTransferEncoding() << ", "

test/integration-http-server.cpp

@@ -265,6 +265,8 @@ void HTTPServerTest::testCoolPost()
     LOK_ASSERT(html.find("data-coolwsd-version = \"" + Util::getCoolVersion() + '"') !=
                std::string::npos);
     LOK_ASSERT(html.find("choMXq0rSMcsm0RoZZWDWsrgAcE5AHwc") != std::string::npos);
+    LOK_ASSERT(html.find("data-access-token = \"choMXq0rSMcsm0RoZZWDWsrgAcE5AHwc\"") !=
+               std::string::npos);
     LOK_ASSERT(html.find("data-access-token-ttl = \"0\"") != std::string::npos);
     LOK_ASSERT(html.find("data-access-header = \"\"") != std::string::npos);
     LOK_ASSERT(html.find("data-post-message-origin-ext = \"https://www.example.com:8080\"") != std::string::npos);

wsd/HostUtil.cpp

@@ -25,7 +25,7 @@ std::set<std::string> HostUtil::hostList;
 std::string HostUtil::FirstHost;
 bool HostUtil::WopiEnabled;
 
-void HostUtil::parseWopiHost(Poco::Util::LayeredConfiguration& conf)
+void HostUtil::parseWopiHost(const Poco::Util::LayeredConfiguration& conf)
 {
     // Parse the WOPI settings.
     WopiHosts.clear();
@@ -39,6 +39,7 @@ void HostUtil::parseWopiHost(Poco::Util::LayeredConfiguration& conf)
             {
                 break;
             }
+
             HostUtil::addWopiHost(conf.getString(path, ""),
                                       conf.getBool(path + "[@allow]", false));
         }
@@ -110,11 +111,12 @@ void HostUtil::parseAliases(Poco::Util::LayeredConfiguration& conf)
         {
             continue;
         }
-        bool allow = conf.getBool(path + ".host[@allow]", false);
+
+        const bool allow = conf.getBool(path + ".host[@allow]", false);
+        const Poco::URI realUri(uri);
 
         try
         {
-            const Poco::URI realUri(uri);
 #if ENABLE_FEATURE_LOCK
             CommandControl::LockManager::mapUnlockLink(realUri.getHost(), path);
 #endif
@@ -141,15 +143,14 @@ void HostUtil::parseAliases(Poco::Util::LayeredConfiguration& conf)
                 {
                     continue;
                 }
-                const std::string host = aliasUri.getHost();
 
-                std::vector<std::string> strVec = Util::splitStringToVector(host, '|');
-                const Poco::URI realUri(uri);
-                for (auto& x : strVec)
+                for (const std::string& x : Util::splitStringToVector(aliasUri.getHost(), '|'))
                 {
                     const Poco::URI aUri(aliasUri.getScheme() + "://" + x + ':' +
                                          std::to_string(aliasUri.getPort()));
-                    AliasHosts.insert({ aUri.getAuthority(), realUri.getAuthority() });
+                    LOG_DBG("Mapped URI alias [" << aUri.getAuthority() << "] to canonical URI ["
+                                                 << realUri.getAuthority() << ']');
+                    AliasHosts.emplace(aUri.getAuthority(), realUri.getAuthority());
 #if ENABLE_FEATURE_LOCK
                     CommandControl::LockManager::mapUnlockLink(aUri.getHost(), path);
 #endif
@@ -170,6 +171,7 @@ std::string HostUtil::getNewUri(const Poco::URI& uri)
     {
         return uri.getPath();
     }
+
     Poco::URI newUri(uri);
     const std::string value = Util::getValue(AliasHosts, newUri.getAuthority());
     if (!value.empty())
@@ -185,14 +187,17 @@ std::string HostUtil::getNewUri(const Poco::URI& uri)
         // the pair in AliasHosts
         if (val.compare(newUri.getHost()) != 0)
         {
-            AliasHosts.insert({ val, newUri.getHost() });
+            LOG_DBG("Mapped URI alias [" << val << "] to canonical URI [" << newUri.getHost()
+                                         << ']');
+            AliasHosts.emplace(val, newUri.getHost());
         }
     }
 
     if (newUri.getAuthority().empty())
     {
         return newUri.getPath();
     }
+
     return newUri.getScheme() + "://" + newUri.getHost() + ':' + std::to_string(newUri.getPort()) +
            newUri.getPath();
 }
@@ -205,9 +210,10 @@ const Poco::URI HostUtil::getNewLockedUri(const Poco::URI& uri)
     {
         newUri.setAuthority(value);
         LOG_WRN("The locked_host: " << uri.getAuthority() << " is alias of "
-                                    << newUri.getAuthority() << ",Applying "
+                                    << newUri.getAuthority() << ", Applying "
                                     << newUri.getAuthority() << " locked_host settings.");
     }
+
     return newUri;
 }
 

wsd/HostUtil.hpp

@@ -35,7 +35,7 @@ class HostUtil
 
 public:
     /// parse wopi.storage.host
-    static void parseWopiHost(Poco::Util::LayeredConfiguration& conf);
+    static void parseWopiHost(const Poco::Util::LayeredConfiguration& conf);
 
     /// parse wopi.storage.alias_groups.group
     static void parseAliases(Poco::Util::LayeredConfiguration& conf);

wsd/RequestDetails.cpp

@@ -309,6 +309,10 @@ std::string RequestDetails::getDocKey(const Poco::URI& uri)
     // resolve aliases
 #if !MOBILEAPP
     const std::string newUri = HostUtil::getNewUri(uri);
+    if (newUri != uri.toString())
+    {
+        LOG_TRC("Canonicalized URI [" << uri.toString() << "] to [" << newUri << ']');
+    }
 #else
     const std::string& newUri = uri.getPath();
 #endif

wsd/TileCache.hpp

@@ -205,7 +205,7 @@ struct TileData
             os << "deltas: ";
             for (size_t i = 0; i < _wids.size(); ++i)
             {
-                os << i << ": " << _wids[i] << " -> " << _offsets[i] << " ";
+                os << i << ": " << _wids[i] << " -> " << _offsets[i] << ' ';
             }
             os << (tooLarge() ? "too-large " : "");
         }

wsd/TileDesc.hpp

@@ -441,10 +441,11 @@ class TileCombined
             _tileWidth <= 0 ||
             _tileHeight <= 0)
         {
-            throw BadArgumentException("Invalid tilecombine descriptor. Elements: " +
-                    std::to_string(_part) + " " + std::to_string(_mode) + " " +
-                    std::to_string(_width) + " " + std::to_string(_height) + " " +
-                    std::to_string(_tileWidth) + " " + std::to_string(_tileHeight));
+            throw BadArgumentException(
+                "Invalid tilecombine descriptor. Elements: " + std::to_string(_part) + ' ' +
+                std::to_string(_mode) + ' ' + std::to_string(_width) + ' ' +
+                std::to_string(_height) + ' ' + std::to_string(_tileWidth) + ' ' +
+                std::to_string(_tileHeight));
         }
 
         StringVector positionXtokens(StringVector::tokenize(tilePositionsX, ','));

wsd/TraceFile.hpp

@@ -274,7 +274,7 @@ class TraceFileWriter
         if (_compress)
         {
             _deflater.write(&delim, 1);
-            _deflater << "+" << deltaT;
+            _deflater << '+' << deltaT;
             _deflater.write(&delim, 1);
             _deflater << id;
             _deflater.write(&delim, 1);
@@ -286,7 +286,7 @@ class TraceFileWriter
         else
         {
             _stream.write(&delim, 1);
-            _stream << "+" << deltaT;
+            _stream << '+' << deltaT;
             _stream.write(&delim, 1);
             _stream << id;
             _stream.write(&delim, 1);