[Libreoffice-commits] online.git: kit/ForKit.cpp test/httpwstest.cpp test/UnitFuzz.cpp wsd/Auth.cpp wsd/TileCache.cpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Sat Mar 11 19:50:26 UTC 2017


 kit/ForKit.cpp      |    2 -
 test/UnitFuzz.cpp   |    2 -
 test/httpwstest.cpp |    4 +-
 wsd/Auth.cpp        |   78 ++++++++++++++++++++++++++++------------------------
 wsd/TileCache.cpp   |   64 ++++++++++++++++++++++--------------------
 5 files changed, 79 insertions(+), 71 deletions(-)

New commits:
commit 57e7d22e28c574632631f6773affa1f600c0c2a7
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sat Mar 11 14:42:50 2017 -0500

    wsd: logging and formatting cleanup
    
    Change-Id: I5bfbd517c37b6df864d181abe7c70857815b9ece
    Reviewed-on: https://gerrit.libreoffice.org/35082
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp
index ca58e2a..fe81ac2 100644
--- a/kit/ForKit.cpp
+++ b/kit/ForKit.cpp
@@ -53,7 +53,7 @@ static bool NoCapsForKit = false;
 #endif
 static bool DisplayVersion = false;
 static std::string UnitTestLibrary;
-static std::atomic<unsigned> ForkCounter( 0 );
+static std::atomic<unsigned> ForkCounter(0);
 
 static std::map<Process::PID, std::string> childJails;
 
diff --git a/test/UnitFuzz.cpp b/test/UnitFuzz.cpp
index 18ee753..460fcf1 100644
--- a/test/UnitFuzz.cpp
+++ b/test/UnitFuzz.cpp
@@ -43,7 +43,7 @@ public:
     std::string corruptString(const std::string &str)
     {
         std::string ret;
-        for ( auto it = str.begin(); it != str.end(); ++it)
+        for (auto it = str.begin(); it != str.end(); ++it)
         {
             int op = _dist(_mt);
             if (op < 10) {
diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index cd5e156..9e9df3e 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -1229,7 +1229,7 @@ void HTTPWSTest::testMaxColumn()
                 CPPUNIT_ASSERT(docHeight >= 0);
 
                 const std::string text = "key type=input char=0 key=1027";
-                while ( cursorX <= docWidth )
+                while (cursorX <= docWidth)
                 {
                     sendTextFrame(socket, text);
                     cursorX += cursorWidth;
@@ -1268,7 +1268,7 @@ void HTTPWSTest::testMaxRow()
                 CPPUNIT_ASSERT(docHeight >= 0);
 
                 const std::string text = "key type=input char=0 key=1024";
-                while ( cursorY <= docHeight )
+                while (cursorY <= docHeight)
                 {
                     sendTextFrame(socket, text);
                     cursorY += cursorHeight;
diff --git a/wsd/Auth.cpp b/wsd/Auth.cpp
index 96765c9..0def0fe 100644
--- a/wsd/Auth.cpp
+++ b/wsd/Auth.cpp
@@ -48,18 +48,18 @@ const std::string JWTAuth::getAccessToken()
     // trim '=' from end of encoded payload
     encodedPayload.erase(std::find_if(encodedPayload.rbegin(), encodedPayload.rend(),
                                       [](char& ch)->bool { return ch != '='; }).base(), encodedPayload.end());
-    Log::info("Encoded JWT header: " + encodedHeader);
-    Log::info("Encoded JWT payload: " + encodedPayload);
+    LOG_INF("Encoded JWT header: " << encodedHeader);
+    LOG_INF("Encoded JWT payload: " << encodedPayload);
 
     // Convert to a URL and filename safe variant:
     // Replace '+' with '-' && '/' with '_'
-    std::replace(encodedHeader.begin(), encodedHeader.end(), '+','-');
-    std::replace(encodedHeader.begin(), encodedHeader.end(), '/','_');
+    std::replace(encodedHeader.begin(), encodedHeader.end(), '+', '-');
+    std::replace(encodedHeader.begin(), encodedHeader.end(), '/', '_');
 
-    std::replace(encodedPayload.begin(), encodedPayload.end(), '+','-');
-    std::replace(encodedPayload.begin(), encodedPayload.end(), '/','_');
+    std::replace(encodedPayload.begin(), encodedPayload.end(), '+', '-');
+    std::replace(encodedPayload.begin(), encodedPayload.end(), '/', '_');
 
-    std::string encodedBody = encodedHeader  + "." +  encodedPayload;
+    const std::string encodedBody = encodedHeader + '.' +  encodedPayload;
 
     // sign the encoded body
     _digestEngine.update(encodedBody.c_str(), static_cast<unsigned>(encodedBody.length()));
@@ -79,13 +79,13 @@ const std::string JWTAuth::getAccessToken()
                                   [](char& ch)->bool { return ch != '='; }).base(), encodedSig.end());
 
     // Be URL and filename safe
-    std::replace(encodedSig.begin(), encodedSig.end(), '+','-');
-    std::replace(encodedSig.begin(), encodedSig.end(), '/','_');
+    std::replace(encodedSig.begin(), encodedSig.end(), '+', '-');
+    std::replace(encodedSig.begin(), encodedSig.end(), '/', '_');
 
-    Log::info("Sig generated is : " + encodedSig);
+    LOG_INF("Sig generated is : " << encodedSig);
 
-    const std::string jwtToken = encodedBody + "." + encodedSig;
-    Log::info("JWT token generated: " + jwtToken);
+    const std::string jwtToken = encodedBody + '.' + encodedSig;
+    LOG_INF("JWT token generated: " << jwtToken);
 
     return jwtToken;
 }
@@ -96,7 +96,7 @@ bool JWTAuth::verify(const std::string& accessToken)
 
     try
     {
-        std::string encodedBody = tokens[0] + "." + tokens[1];
+        const std::string encodedBody = tokens[0] + '.' + tokens[1];
         _digestEngine.update(encodedBody.c_str(), static_cast<unsigned>(encodedBody.length()));
         Poco::Crypto::DigestEngine::Digest digest = _digestEngine.signature();
 
@@ -118,7 +118,7 @@ bool JWTAuth::verify(const std::string& accessToken)
 
         if (encodedSig != tokens[2])
         {
-            Log::info("JWTAuth: verification failed; Expected: " + encodedSig + ", Received: " + tokens[2]);
+            LOG_INF("JWTAuth: verification failed; Expected: " << encodedSig << ", Received: " << tokens[2]);
             return false;
         }
 
@@ -127,7 +127,7 @@ bool JWTAuth::verify(const std::string& accessToken)
         Base64Decoder decoder(istr);
         decoder >> decodedPayload;
 
-        Log::info("JWTAuth:verify: decoded payload: " + decodedPayload);
+        LOG_INF("JWTAuth:verify: decoded payload: " << decodedPayload);
 
         // Verify if the token is not already expired
         Poco::JSON::Parser parser;
@@ -138,13 +138,13 @@ bool JWTAuth::verify(const std::string& accessToken)
         std::time_t curtime = Poco::Timestamp().epochTime();
         if (curtime > decodedExptime)
         {
-            Log::info("JWTAuth:verify: JWT expired; curtime:" + std::to_string(curtime) + ", exp:" + std::to_string(decodedExptime));
+            LOG_INF("JWTAuth:verify: JWT expired; curtime:" << curtime << ", exp:" << decodedExptime);
             return false;
         }
     }
     catch(Poco::Exception& exc)
     {
-        Log::warn("JWTAuth:verify: Exception: " + exc.displayText());
+        LOG_WRN("JWTAuth:verify: Exception: " << exc.displayText());
         return false;
     }
 
@@ -154,9 +154,9 @@ bool JWTAuth::verify(const std::string& accessToken)
 const std::string JWTAuth::createHeader()
 {
     // TODO: Some sane code to represent JSON objects
-    std::string header = "{\"alg\":\""+_alg+"\",\"typ\":\""+_typ+"\"}";
+    const std::string header = "{\"alg\":\"" + _alg + "\",\"typ\":\"" + _typ + "\"}";
 
-    Log::info("JWT Header: " + header);
+    LOG_INF("JWT Header: " << header);
     std::ostringstream ostr;
     OutputLineEndingConverter lineEndingConv(ostr, "");
     Base64Encoder encoder(lineEndingConv);
@@ -168,13 +168,15 @@ const std::string JWTAuth::createHeader()
 
 const std::string JWTAuth::createPayload()
 {
-    std::time_t curtime = Poco::Timestamp().epochTime();
-    std::string exptime = std::to_string(curtime + 1800);
+    const std::time_t curtime = Poco::Timestamp().epochTime();
+    const std::string exptime = std::to_string(curtime + 1800);
 
     // TODO: Some sane code to represent JSON objects
-    std::string payload = "{\"iss\":\""+_iss+"\",\"sub\":\""+_sub+"\",\"aud\":\""+_aud+"\",\"nme\":\""+_name+"\",\"exp\":\""+exptime+"\"}";
+    const std::string payload = "{\"iss\":\"" + _iss + "\",\"sub\":\"" + _sub
+                              + "\",\"aud\":\"" + _aud + "\",\"nme\":\"" + _name
+                              + "\",\"exp\":\"" + exptime + "\"}";
 
-    Log::info("JWT Payload: " + payload);
+    LOG_INF("JWT Payload: " << payload);
     std::ostringstream ostr;
     OutputLineEndingConverter lineEndingConv(ostr, "");
     Base64Encoder encoder(lineEndingConv);
@@ -187,22 +189,24 @@ const std::string JWTAuth::createPayload()
 //TODO: This MUST be done over TLS to protect the token.
 const std::string OAuth::getAccessToken()
 {
-    std::string url = _tokenEndPoint
-        + "?client_id=" + _clientId
-        + "&client_secret=" + _clientSecret
-        + "&grant_type=authorization_code"
-        + "&code=" + _authorizationCode;
-    // + "&redirect_uri="
+    const std::string url = _tokenEndPoint
+                          + "?client_id=" + _clientId
+                          + "&client_secret=" + _clientSecret
+                          + "&grant_type=authorization_code"
+                          + "&code=" + _authorizationCode;
+                        // + "&redirect_uri="
 
     Poco::URI uri(url);
     Poco::Net::HTTPClientSession session(uri.getHost(), uri.getPort());
     Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, url, Poco::Net::HTTPMessage::HTTP_1_1);
     Poco::Net::HTTPResponse response;
     session.sendRequest(request);
+
     std::istream& rs = session.receiveResponse(response);
-    Log::info() << "Status: " <<  response.getStatus() << " " << response.getReason() << Log::end;
-    std::string reply(std::istreambuf_iterator<char>(rs), {});
-    Log::info("Response: " + reply);
+    LOG_INF("Status: " <<  response.getStatus() << ' ' << response.getReason());
+
+    const std::string reply(std::istreambuf_iterator<char>(rs), {});
+    LOG_INF("Response: " << reply);
     //TODO: Parse the token.
 
     return std::string();
@@ -211,16 +215,18 @@ const std::string OAuth::getAccessToken()
 bool OAuth::verify(const std::string& token)
 {
     const std::string url = _authVerifyUrl + token;
-    Log::debug("Verifying authorization token from: " + url);
+    LOG_DBG("Verifying authorization token from: " << url);
     Poco::URI uri(url);
     Poco::Net::HTTPClientSession session(uri.getHost(), uri.getPort());
     Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, url, Poco::Net::HTTPMessage::HTTP_1_1);
     Poco::Net::HTTPResponse response;
     session.sendRequest(request);
+
     std::istream& rs = session.receiveResponse(response);
-    Log::info() << "Status: " <<  response.getStatus() << " " << response.getReason() << Log::end;
-    std::string reply(std::istreambuf_iterator<char>(rs), {});
-    Log::info("Response: " + reply);
+    LOG_INF("Status: " <<  response.getStatus() << ' ' << response.getReason());
+
+    const std::string reply(std::istreambuf_iterator<char>(rs), {});
+    LOG_INF("Response: " << reply);
 
     //TODO: Parse the response.
     /*
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index c8cb0a6..0607d1e 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -52,9 +52,9 @@ TileCache::TileCache(const std::string& docURL,
     _docURL(docURL),
     _cacheDir(cacheDir)
 {
-    Log::info() << "TileCache ctor for uri [" << _docURL
-                << "] modifiedTime=" << (modifiedTime.raw()/1000000)
-                << " getLastModified()=" << (getLastModified().raw()/1000000) << Log::end;
+    LOG_INF("TileCache ctor for uri [" << _docURL <<
+            "] modifiedTime=" << (modifiedTime.raw()/1000000) <<
+            " getLastModified()=" << (getLastModified().raw()/1000000));
     File directory(_cacheDir);
     std::string unsaved;
     if (directory.exists() &&
@@ -63,7 +63,7 @@ TileCache::TileCache(const std::string& docURL,
     {
         // Document changed externally or modifications were not saved after all. Cache not useful.
         FileUtil::removeFile(_cacheDir, true);
-        Log::info("Completely cleared tile cache: " + _cacheDir);
+        LOG_INF("Completely cleared tile cache: " << _cacheDir);
     }
 
     File(_cacheDir).createDirectories();
@@ -73,7 +73,7 @@ TileCache::TileCache(const std::string& docURL,
 
 TileCache::~TileCache()
 {
-    Log::info("~TileCache dtor for uri [" + _docURL + "].");
+    LOG_INF("~TileCache dtor for uri [" << _docURL << "].");
 }
 
 /// Tracks the rendering of a given tile
@@ -134,7 +134,7 @@ std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile)
 
     if (result && result->is_open())
     {
-        Log::trace("Found cache tile: " + fileName);
+        LOG_TRC("Found cache tile: " << fileName);
         return result;
     }
 
@@ -156,7 +156,7 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
     const auto fileName = _cacheDir + "/" + cachedName;
     if (FileUtil::saveDataToFileSafely(fileName, data, size))
     {
-        Log::trace() << "Saved cache tile: " << fileName << Log::end;
+        LOG_TRC("Saved cache tile: " << fileName);
     }
 
     // Notify subscribers, if any.
@@ -166,7 +166,7 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
         if (subscriberCount > 0)
         {
             std::string response = tile.serialize("tile:");
-            LOG_DBG("Sending tile message to " << subscriberCount << " subscribers: " + response);
+            LOG_DBG("Sending tile message to " << subscriberCount << " subscribers: " << response);
 
             // Send to first subscriber as-is (without cache marker).
             auto payload = std::make_shared<Message>(response,
@@ -207,20 +207,20 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
         }
         else
         {
-            Log::debug("No subscribers for: " + cachedName);
+            LOG_DBG("No subscribers for: " << cachedName);
         }
 
         // Remove subscriptions.
         if (tileBeingRendered->getVersion() <= tile.getVersion())
         {
-            Log::debug() << "STATISTICS: tile " << tile.getVersion() << " internal roundtrip "
-                         << tileBeingRendered->getElapsedTimeMs() << " ms." << Log::end;
+            LOG_DBG("STATISTICS: tile " << tile.getVersion() << " internal roundtrip " <<
+                    tileBeingRendered->getElapsedTimeMs() << " ms.");
             _tilesBeingRendered.erase(cachedName);
         }
     }
     else
     {
-        Log::debug("No subscribers for: " + cachedName);
+        LOG_DBG("No subscribers for: " << cachedName);
     }
 }
 
@@ -231,7 +231,7 @@ bool TileCache::getTextFile(const std::string& fileName, std::string& content)
     std::fstream textStream(fullFileName, std::ios::in);
     if (!textStream.is_open())
     {
-        Log::info("Could not open " + fullFileName);
+        LOG_INF("Could not open " << fullFileName);
         return false;
     }
 
@@ -247,7 +247,8 @@ bool TileCache::getTextFile(const std::string& fileName, std::string& content)
         buffer.pop_back();
 
     content = std::string(buffer.data(), buffer.size());
-    Log::info("Read '" + LOOLProtocol::getAbbreviatedMessage(content.c_str(), content.size()) + "' from " + fullFileName);
+    LOG_INF("Read '" << LOOLProtocol::getAbbreviatedMessage(content.c_str(), content.size()) <<
+            "' from " << fullFileName);
 
     return true;
 }
@@ -259,12 +260,13 @@ void TileCache::saveTextFile(const std::string& text, const std::string& fileNam
 
     if (!textStream.is_open())
     {
-        Log::error("Could not save '" + text + "' to " + fullFileName);
+        LOG_ERR("Could not save '" << text << "' to " << fullFileName);
         return;
     }
     else
     {
-        Log::info("Saving '" + LOOLProtocol::getAbbreviatedMessage(text.c_str(), text.size()) + "' to " + fullFileName);
+        LOG_INF("Saving '" << LOOLProtocol::getAbbreviatedMessage(text.c_str(), text.size()) <<
+                "' to " << fullFileName);
     }
 
     textStream << text << std::endl;
@@ -308,10 +310,10 @@ std::unique_ptr<std::fstream> TileCache::lookupCachedFile(const std::string& nam
 
 void TileCache::invalidateTiles(int part, int x, int y, int width, int height)
 {
-    Log::trace() << "Removing invalidated tiles: part: " << part
-                 << ", x: " << x << ", y: " << y
-                 << ", width: " << width
-                 << ", height: " << height << Log::end;
+    LOG_TRC("Removing invalidated tiles: part: " << part <<
+            ", x: " << x << ", y: " << y <<
+            ", width: " << width <<
+            ", height: " << height);
 
     File dir(_cacheDir);
 
@@ -325,7 +327,7 @@ void TileCache::invalidateTiles(int part, int x, int y, int width, int height)
             const std::string fileName = tileIterator.path().getFileName();
             if (intersectsTile(fileName, part, x, y, width, height))
             {
-                Log::debug("Removing tile: " + tileIterator.path().toString());
+                LOG_DBG("Removing tile: " << tileIterator.path().toString());
                 FileUtil::removeFile(tileIterator.path());
             }
         }
@@ -375,7 +377,7 @@ void TileCache::removeFile(const std::string& fileName)
     const std::string fullFileName = _cacheDir + "/" + fileName;
 
     if (std::remove(fullFileName.c_str()) == 0)
-        Log::info("Removed file: " + fullFileName);
+        LOG_INF("Removed file: " << fullFileName);
 }
 
 std::string TileCache::cacheFileName(const TileDesc& tile)
@@ -450,15 +452,14 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
         {
             if (s.lock().get() == subscriber.get())
             {
-                Log::debug("Redundant request to subscribe on tile " + name);
+                LOG_DBG("Redundant request to subscribe on tile " << name);
                 tileBeingRendered->setVersion(tile.getVersion());
                 return;
             }
         }
 
-        Log::debug() << "Subscribing " << subscriber->getName() << " to tile " << name << " which has "
-                     << tileBeingRendered->_subscribers.size()
-                     << " subscribers already." << Log::end;
+        LOG_DBG("Subscribing " << subscriber->getName() << " to tile " << name << " which has " <<
+                tileBeingRendered->_subscribers.size() << " subscribers already.");
         tileBeingRendered->_subscribers.push_back(subscriber);
 
         const auto duration = (std::chrono::steady_clock::now() - tileBeingRendered->getStartTime());
@@ -470,8 +471,8 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
     }
     else
     {
-        Log::debug() << "Subscribing " << subscriber->getName() << " to tile " << name
-                     << " ver=" << tile.getVersion() << " which has no subscribers." << Log::end;
+        LOG_DBG("Subscribing " << subscriber->getName() << " to tile " << name <<
+                " ver=" << tile.getVersion() << " which has no subscribers.");
 
         const std::string cachedName = cacheFileName(tile);
 
@@ -486,7 +487,7 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
 std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscriber)
 {
     assert(subscriber && "cancelTiles expects valid subscriber");
-    Log::trace("Cancelling tiles for " + subscriber->getName());
+    LOG_TRC("Cancelling tiles for " << subscriber->getName());
 
     std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
 
@@ -504,13 +505,14 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri
         }
 
         auto& subscribers = it->second->_subscribers;
-        Log::trace("Tile " + it->first + " has " + std::to_string(subscribers.size()) + " subscribers.");
+        LOG_TRC("Tile " << it->first << " has " << subscribers.size() << " subscribers.");
 
         const auto itRem = std::find_if(subscribers.begin(), subscribers.end(),
                                         [sub](std::weak_ptr<ClientSession>& ptr){ return ptr.lock().get() == sub; });
         if (itRem != subscribers.end())
         {
-            Log::trace("Tile " + it->first + " has " + std::to_string(subscribers.size()) + " subscribers. Removing " + subscriber->getName() + ".");
+            LOG_TRC("Tile " << it->first << " has " << subscribers.size() <<
+                    " subscribers. Removing " << subscriber->getName() << ".");
             subscribers.erase(itRem, itRem + 1);
             if (subscribers.empty())
             {


More information about the Libreoffice-commits mailing list