[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-cd-3-2' - common/Util.cpp common/Util.hpp kit/ChildSession.cpp kit/ChildSession.hpp kit/Kit.cpp test/WhiteBoxTests.cpp wsd/DocumentBroker.cpp wsd/LOOLWSD.cpp wsd/Storage.cpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Tue Jul 17 18:05:58 UTC 2018


 common/Util.cpp        |   80 +++++++++++++++----------
 common/Util.hpp        |   43 +++++++++++--
 kit/ChildSession.cpp   |    3 
 kit/ChildSession.hpp   |    2 
 kit/Kit.cpp            |   10 ++-
 test/WhiteBoxTests.cpp |  152 +++++++++++++++++++++++++++++++++++++++++++++++++
 wsd/DocumentBroker.cpp |    5 +
 wsd/LOOLWSD.cpp        |   25 +++++---
 wsd/Storage.cpp        |   13 +++-
 9 files changed, 281 insertions(+), 52 deletions(-)

New commits:
commit 9fee82aebadfdc59ece0693bdcba558b63cdb818
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Jul 17 02:01:05 2018 -0400

    wsd: anonymization improvements and unittests
    
    Also support anonymization of downloadas documents
    and renaming of documents.
    
    Change-Id: I81a80e6290217659987d73f625e5f0fb81cb7ef2
    Reviewed-on: https://gerrit.libreoffice.org/57541
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>

diff --git a/common/Util.cpp b/common/Util.cpp
index 788757e1c..cfd636d4b 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -544,6 +544,42 @@ namespace Util
         return true;
     }
 
+    /// Split a string in two at the delimeter and give the delimiter to the first.
+    static
+    std::pair<std::string, std::string> splitLast2(const char* s, const int length, const char delimeter = ' ')
+    {
+        if (s != nullptr && length > 0)
+        {
+            const int pos = getLastDelimiterPosition(s, length, delimeter);
+            if (pos < length)
+                return std::make_pair(std::string(s, pos + 1), std::string(s + pos + 1));
+        }
+
+        // Not found; return in first.
+        return std::make_pair(std::string(s, length), std::string());
+    }
+
+    std::tuple<std::string, std::string, std::string, std::string> splitUrl(const std::string& url)
+    {
+        // In case we have a URL that has parameters.
+        std::string base;
+        std::string params;
+        std::tie(base, params) = Util::split(url, '?', false);
+
+        std::string filename;
+        std::tie(base, filename) = Util::splitLast2(base.c_str(), base.size(), '/');
+        if (filename.empty())
+        {
+            // If no '/', then it's only filename.
+            std::swap(base, filename);
+        }
+
+        std::string ext;
+        std::tie(filename, ext) = Util::splitLast(filename, '.', false);
+
+        return std::make_tuple(base, filename, ext, params);
+    }
+
     static std::map<std::string, std::string> AnonymizedStrings;
     static std::atomic<unsigned> AnonymizationSalt(0);
     static std::mutex AnonymizedMutex;
@@ -586,45 +622,25 @@ namespace Util
         return res;
     }
 
-    static std::string anonymizeFilename(const std::string& filename)
+    std::string getFilenameFromURL(const std::string& url)
     {
-        // Preserve the extension.
-        std::string basename;
+        std::string base;
+        std::string filename;
         std::string ext;
-        const std::size_t mid = filename.find_last_of('.');
-        if (mid != std::string::npos)
-        {
-            basename = filename.substr(0, mid);
-            ext = filename.substr(mid);
-        }
-        else
-            basename = filename;
-
-        return Util::anonymize(basename) + ext;
-    }
-
-    std::string getFilenameFromPath(const std::string& path)
-    {
-        const std::size_t mid = path.find_last_of('/');
-        if (mid != std::string::npos)
-            return path.substr(mid + 1);
-
-        // No path, treat as filename only.
-        return path;
+        std::string params;
+        std::tie(base, filename, ext, params) = Util::splitUrl(url);
+        return filename;
     }
 
     std::string anonymizeUrl(const std::string& url)
     {
-        const std::size_t mid = url.find_last_of('/');
-        if (mid != std::string::npos)
-        {
-            const std::string path = url.substr(0, mid + 1);
-            const std::string filename = url.substr(mid + 1);
-            return path + Util::anonymizeFilename(filename);
-        }
+        std::string base;
+        std::string filename;
+        std::string ext;
+        std::string params;
+        std::tie(base, filename, ext, params) = Util::splitUrl(url);
 
-        // No path, treat as filename only.
-        return Util::anonymizeFilename(url);
+        return base + Util::anonymize(filename) + ext + params;
     }
 }
 
diff --git a/common/Util.hpp b/common/Util.hpp
index 3d457997f..719a1081c 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -242,6 +242,18 @@ namespace Util
         return false;
     }
 
+    inline size_t getLastDelimiterPosition(const char* message, const int length, const char delim)
+    {
+        if (message && length > 0)
+        {
+            const char *founddelim = static_cast<const char *>(memrchr(message, delim, length));
+            const auto size = (founddelim == nullptr ? length : founddelim - message);
+            return size;
+        }
+
+        return 0;
+    }
+
     inline size_t getDelimiterPosition(const char* message, const int length, const char delim)
     {
         if (message && length > 0)
@@ -263,19 +275,38 @@ namespace Util
 
     /// Split a string in two at the delimeter, removing it.
     inline
-    std::pair<std::string, std::string> split(const char* s, const int length, const char delimeter = ' ')
+    std::pair<std::string, std::string> split(const char* s, const int length, const char delimeter = ' ', bool removeDelim = true)
     {
         const auto size = getDelimiterPosition(s, length, delimeter);
-        return std::make_pair(std::string(s, size), std::string(s+size+1));
+        return std::make_pair(std::string(s, size), std::string(s+size+removeDelim));
     }
 
     /// Split a string in two at the delimeter, removing it.
     inline
-    std::pair<std::string, std::string> split(const std::string& s, const char delimeter = ' ')
+    std::pair<std::string, std::string> split(const std::string& s, const char delimeter = ' ', bool removeDelim = true)
     {
-        return split(s.c_str(), s.size(), delimeter);
+        return split(s.c_str(), s.size(), delimeter, removeDelim);
     }
 
+    /// Split a string in two at the delimeter.
+    inline
+    std::pair<std::string, std::string> splitLast(const char* s, const int length, const char delimeter = ' ', bool removeDelim = true)
+    {
+        const auto size = getLastDelimiterPosition(s, length, delimeter);
+        return std::make_pair(std::string(s, size), std::string(s+size+removeDelim));
+    }
+
+    /// Split a string in two at the delimeter, removing it.
+    inline
+    std::pair<std::string, std::string> splitLast(const std::string& s, const char delimeter = ' ', bool removeDelim = true)
+    {
+        return splitLast(s.c_str(), s.size(), delimeter, removeDelim);
+    }
+
+    /// Splits a URL into path (with protocol), filename, extension, parameters.
+    /// All components are optional, depending on what the URL represents (can be a unix path).
+    std::tuple<std::string, std::string, std::string, std::string> splitUrl(const std::string& url);
+
     /// Check for the URI scheme validity.
     /// For now just a basic sanity check, can be extended if necessary.
     bool isValidURIScheme(const std::string& scheme);
@@ -295,8 +326,8 @@ namespace Util
     /// Anonymize the basename of filenames only, preserving the path and extension.
     std::string anonymizeUrl(const std::string& url);
 
-    /// Extract and return the filename given a path (i.e. the token after last '/').
-    std::string getFilenameFromPath(const std::string& path);
+    /// Extract and return the filename given a url or path.
+    std::string getFilenameFromURL(const std::string& url);
 
     /// Given one or more patterns to allow, and one or more to deny,
     /// the match member will return true if, and only if, the subject
diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp
index b273c86f0..a7a1349ff 100644
--- a/kit/ChildSession.cpp
+++ b/kit/ChildSession.cpp
@@ -649,6 +649,9 @@ bool ChildSession::downloadAs(const char* /*buffer*/, int /*length*/, const std:
         return false;
     }
 
+    // Obfuscate the new name.
+    Util::mapAnonymized(name, _docManager.getObfuscatedFileId());
+
     getTokenString(tokens[3], "format", format);
 
     if (getTokenString(tokens[4], "options", filterOptions))
diff --git a/kit/ChildSession.hpp b/kit/ChildSession.hpp
index a5390dff7..853366993 100644
--- a/kit/ChildSession.hpp
+++ b/kit/ChildSession.hpp
@@ -70,6 +70,8 @@ public:
     /// setting a view followed by a tile render, etc.
     virtual std::mutex& getDocumentMutex() = 0;
 
+    virtual std::string getObfuscatedFileId() = 0;
+
     virtual std::shared_ptr<TileQueue>& getTileQueue() = 0;
 
     virtual bool sendFrame(const char* buffer, int length, int flags = Poco::Net::WebSocket::FRAME_TEXT) = 0;
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index dfc0507cd..8e4be4ae8 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -110,7 +110,6 @@ static LokHookFunction2* initFunction = nullptr;
 static bool AnonymizeFilenames = false;
 static bool AnonymizeUsernames = false;
 static std::string ObfuscatedFileId;
-static std::string ObfuscatedUserId;
 #endif
 
 #if ENABLE_DEBUG
@@ -712,6 +711,7 @@ public:
         _docKey(docKey),
         _docId(docId),
         _url(url),
+        _obfuscatedFileId(Util::getFilenameFromURL(docKey)),
         _tileQueue(std::move(tileQueue)),
         _ws(ws),
         _docPassword(""),
@@ -1899,6 +1899,11 @@ private:
         return _documentMutex;
     }
 
+    std::string getObfuscatedFileId() override
+    {
+        return _obfuscatedFileId;
+    }
+
 private:
     std::shared_ptr<lok::Office> _loKit;
     const std::string _jailId;
@@ -1907,6 +1912,7 @@ private:
     /// Short numerical ID. Unique during the lifetime of WSD.
     const std::string _docId;
     const std::string _url;
+    const std::string _obfuscatedFileId;
     std::string _jailedUrl;
     std::string _renderOpts;
 
@@ -2251,7 +2257,7 @@ void lokit_main(const std::string& childRoot,
                         const std::string& sessionId = tokens[1];
                         const std::string& docKey = tokens[2];
                         const std::string& docId = tokens[3];
-                        const std::string fileId = Util::getFilenameFromPath(docKey);
+                        const std::string fileId = Util::getFilenameFromURL(docKey);
                         Util::mapAnonymized(fileId, fileId); // Identity mapping, since fileId is already obfuscated
 
                         std::string url;
diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp
index 8cd237cab..c291f9fc3 100644
--- a/test/WhiteBoxTests.cpp
+++ b/test/WhiteBoxTests.cpp
@@ -27,6 +27,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST_SUITE(WhiteBoxTests);
 
     CPPUNIT_TEST(testLOOLProtocolFunctions);
+    CPPUNIT_TEST(testSplitting);
     CPPUNIT_TEST(testMessageAbbreviation);
     CPPUNIT_TEST(testTokenizer);
     CPPUNIT_TEST(testReplace);
@@ -36,10 +37,12 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testRectanglesIntersect);
     CPPUNIT_TEST(testAuthorization);
     CPPUNIT_TEST(testJson);
+    CPPUNIT_TEST(testAnonymization);
 
     CPPUNIT_TEST_SUITE_END();
 
     void testLOOLProtocolFunctions();
+    void testSplitting();
     void testMessageAbbreviation();
     void testTokenizer();
     void testReplace();
@@ -49,6 +52,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     void testRectanglesIntersect();
     void testAuthorization();
     void testJson();
+    void testAnonymization();
 };
 
 void WhiteBoxTests::testLOOLProtocolFunctions()
@@ -142,6 +146,115 @@ void WhiteBoxTests::testLOOLProtocolFunctions()
     CPPUNIT_ASSERT_EQUAL(std::string(""), Util::trim(s));
 }
 
+void WhiteBoxTests::testSplitting()
+{
+    CPPUNIT_ASSERT_EQUAL(std::string(), Util::getDelimitedInitialSubstring(nullptr, 5, '\n'));
+    CPPUNIT_ASSERT_EQUAL(std::string(), Util::getDelimitedInitialSubstring(nullptr, -1, '\n'));
+    CPPUNIT_ASSERT_EQUAL(std::string(), Util::getDelimitedInitialSubstring("abc", 0, '\n'));
+    CPPUNIT_ASSERT_EQUAL(std::string(), Util::getDelimitedInitialSubstring("abc", -1, '\n'));
+    CPPUNIT_ASSERT_EQUAL(std::string("ab"), Util::getDelimitedInitialSubstring("abc", 2, '\n'));
+
+    std::string first;
+    std::string second;
+
+    std::tie(first, second) = Util::split(std::string(""), '.', true);
+    std::tie(first, second) = Util::split(std::string(""), '.', false);
+
+    std::tie(first, second) = Util::splitLast(std::string(""), '.', true);
+    std::tie(first, second) = Util::splitLast(std::string(""), '.', false);
+
+    // Split first, remove delim.
+    std::tie(first, second) = Util::split(std::string("a"), '.', true);
+    CPPUNIT_ASSERT_EQUAL(std::string("a"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), second);
+
+    // Split first, keep delim.
+    std::tie(first, second) = Util::split(std::string("a"), '.', false);
+    CPPUNIT_ASSERT_EQUAL(std::string("a"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), second);
+
+    // Split first, remove delim.
+    std::tie(first, second) = Util::splitLast(std::string("a"), '.', true);
+    CPPUNIT_ASSERT_EQUAL(std::string("a"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), second);
+
+    // Split first, keep delim.
+    std::tie(first, second) = Util::splitLast(std::string("a"), '.', false);
+    CPPUNIT_ASSERT_EQUAL(std::string("a"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), second);
+
+    // Split first, remove delim.
+    std::tie(first, second) = Util::split(std::string("aa.bb"), '.', true);
+    CPPUNIT_ASSERT_EQUAL(std::string("aa"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("bb"), second);
+
+    // Split first, keep delim.
+    std::tie(first, second) = Util::split(std::string("aa.bb"), '.', false);
+    CPPUNIT_ASSERT_EQUAL(std::string("aa"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string(".bb"), second);
+
+    CPPUNIT_ASSERT_EQUAL(5UL, Util::getLastDelimiterPosition("aa.bb.cc", 8, '.'));
+
+    // Split last, remove delim.
+    std::tie(first, second) = Util::splitLast(std::string("aa.bb.cc"), '.', true);
+    CPPUNIT_ASSERT_EQUAL(std::string("aa.bb"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("cc"), second);
+
+    // Split last, keep delim.
+    std::tie(first, second) = Util::splitLast(std::string("aa.bb.cc"), '.', false);
+    CPPUNIT_ASSERT_EQUAL(std::string("aa.bb"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string(".cc"), second);
+
+    // Split last, remove delim.
+    std::tie(first, second) = Util::splitLast(std::string("/owncloud/index.php/apps/richdocuments/wopi/files/13_ocgdpzbkm39u"), '/', true);
+    CPPUNIT_ASSERT_EQUAL(std::string("/owncloud/index.php/apps/richdocuments/wopi/files"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("13_ocgdpzbkm39u"), second);
+
+    // Split last, keep delim.
+    std::tie(first, second) = Util::splitLast(std::string("/owncloud/index.php/apps/richdocuments/wopi/files/13_ocgdpzbkm39u"), '/', false);
+    CPPUNIT_ASSERT_EQUAL(std::string("/owncloud/index.php/apps/richdocuments/wopi/files"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("/13_ocgdpzbkm39u"), second);
+
+    std::string third;
+    std::string fourth;
+
+    std::tie(first, second, third, fourth) = Util::splitUrl("filename");
+    CPPUNIT_ASSERT_EQUAL(std::string(""), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("filename"), second);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), third);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), fourth);
+
+    std::tie(first, second, third, fourth) = Util::splitUrl("filename.ext");
+    CPPUNIT_ASSERT_EQUAL(std::string(""), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("filename"), second);
+    CPPUNIT_ASSERT_EQUAL(std::string(".ext"), third);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), fourth);
+
+    std::tie(first, second, third, fourth) = Util::splitUrl("/path/to/filename");
+    CPPUNIT_ASSERT_EQUAL(std::string("/path/to/"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("filename"), second);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), third);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), fourth);
+
+    std::tie(first, second, third, fourth) = Util::splitUrl("http://domain.com/path/filename");
+    CPPUNIT_ASSERT_EQUAL(std::string("http://domain.com/path/"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("filename"), second);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), third);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), fourth);
+
+    std::tie(first, second, third, fourth) = Util::splitUrl("http://domain.com/path/filename.ext");
+    CPPUNIT_ASSERT_EQUAL(std::string("http://domain.com/path/"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("filename"), second);
+    CPPUNIT_ASSERT_EQUAL(std::string(".ext"), third);
+    CPPUNIT_ASSERT_EQUAL(std::string(""), fourth);
+
+    std::tie(first, second, third, fourth) = Util::splitUrl("http://domain.com/path/filename.ext?params=3&command=5");
+    CPPUNIT_ASSERT_EQUAL(std::string("http://domain.com/path/"), first);
+    CPPUNIT_ASSERT_EQUAL(std::string("filename"), second);
+    CPPUNIT_ASSERT_EQUAL(std::string(".ext"), third);
+    CPPUNIT_ASSERT_EQUAL(std::string("?params=3&command=5"), fourth);
+}
+
 void WhiteBoxTests::testMessageAbbreviation()
 {
     CPPUNIT_ASSERT_EQUAL(std::string(), Util::getDelimitedInitialSubstring(nullptr, 5, '\n'));
@@ -387,6 +500,11 @@ public:
         return _mutex;
     }
 
+    std::string getObfuscatedFileId() override
+    {
+        return std::string();
+    }
+
     std::shared_ptr<TileQueue>& getTileQueue() override
     {
         return _tileQueue;
@@ -505,6 +623,40 @@ void WhiteBoxTests::testJson()
     CPPUNIT_ASSERT_EQUAL(std::string("user at user.com"), sValue);
 }
 
+void WhiteBoxTests::testAnonymization()
+{
+    static const std::string name = "some name with space";
+    CPPUNIT_ASSERT_EQUAL(std::string("#0#77d#"), Util::anonymizeUrl(name));
+    Util::mapAnonymized(name, name);
+    CPPUNIT_ASSERT_EQUAL(name, Util::anonymizeUrl(name));
+
+    static const std::string filename = "filename.ext";
+    CPPUNIT_ASSERT_EQUAL(std::string("#1#341#.ext"), Util::anonymizeUrl(filename));
+    Util::mapAnonymized("filename", "filename");
+    CPPUNIT_ASSERT_EQUAL(name, Util::anonymizeUrl(name));
+
+    static const std::string filenameTestx = "testx (6).odt";
+    CPPUNIT_ASSERT_EQUAL(std::string("#2#2df#.odt"), Util::anonymizeUrl(filenameTestx));
+    Util::mapAnonymized("testx (6)", "testx (6)");
+    CPPUNIT_ASSERT_EQUAL(filenameTestx, Util::anonymizeUrl(filenameTestx));
+
+    static const std::string path = "/path/to/filename.ext";
+    CPPUNIT_ASSERT_EQUAL(path, Util::anonymizeUrl(path));
+
+    static const std::string plainUrl = "http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/736_ocgdpzbkm39u?access_token=Hn0zttjbwkvGWb5BHbDa5ArgTykJAyBl&access_token_ttl=0&permission=edit";
+    const std::string urlAnonymized = Util::replace(plainUrl, "736_ocgdpzbkm39u", "#3#5a1#");
+    CPPUNIT_ASSERT_EQUAL(urlAnonymized, Util::anonymizeUrl(plainUrl));
+    Util::mapAnonymized("736_ocgdpzbkm39u", "736_ocgdpzbkm39u");
+    CPPUNIT_ASSERT_EQUAL(plainUrl, Util::anonymizeUrl(plainUrl));
+
+    static const std::string fileUrl = "http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/736_ocgdpzbkm39u/secret.odt?access_token=Hn0zttjbwkvGWb5BHbDa5ArgTykJAyBl&access_token_ttl=0&permission=edit";
+    const std::string urlAnonymized2 = Util::replace(fileUrl, "secret", "#4#286#");
+    CPPUNIT_ASSERT_EQUAL(urlAnonymized2, Util::anonymizeUrl(fileUrl));
+    Util::mapAnonymized("secret", "736_ocgdpzbkm39u");
+    const std::string urlAnonymized3 = Util::replace(fileUrl, "secret", "736_ocgdpzbkm39u");
+    CPPUNIT_ASSERT_EQUAL(urlAnonymized3, Util::anonymizeUrl(fileUrl));
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(WhiteBoxTests);
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a0ec6a64c..258e53d45 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -510,10 +510,13 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
 
         std::ostringstream ossWopiInfo;
         wopiInfo->stringify(ossWopiInfo);
+        const std::string wopiInfoString = ossWopiInfo.str();
+        LOG_TRC("Sending wopi info to client: " << wopiInfoString);
+
         // Contains PostMessageOrigin property which is necessary to post messages to parent
         // frame. Important to send this message immediately and not enqueue it so that in case
         // document load fails, loleaflet is able to tell its parent frame via PostMessage API.
-        session->sendMessage("wopi: " + ossWopiInfo.str());
+        session->sendMessage("wopi: " + wopiInfoString);
 
         // Mark the session as 'Document owner' if WOPI hosts supports it
         if (userId == _storage->getFileInfo()._ownerId)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 7c1b365d5..f4d9c2235 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1666,6 +1666,7 @@ private:
         Poco::Net::HTTPRequest request;
         try
         {
+            // This is the prisoner/child connection; don't anonymize.
             request.read(message);
 
             auto logger = Log::info();
@@ -1673,7 +1674,7 @@ private:
             {
                 logger << "#" << socket->getFD() << ": Prisoner HTTP Request: "
                        << request.getMethod() << ' '
-                       << LOOLWSD::anonymizeUrl(request.getURI()) << ' '
+                       << request.getURI() << ' '
                        << request.getVersion();
 
                 for (const auto& it : request)
@@ -1684,7 +1685,7 @@ private:
                 LOG_END(logger);
             }
 
-            LOG_TRC("Child connection with URI [" << LOOLWSD::anonymizeUrl(request.getURI()) << "].");
+            LOG_TRC("Child connection with URI [" << request.getURI() << "].");
             if (request.getURI().find(NEW_CHILD_URI) != 0)
             {
                 LOG_ERR("Invalid incoming URI.");
@@ -1713,13 +1714,13 @@ private:
 
             if (pid <= 0)
             {
-                LOG_ERR("Invalid PID in child URI [" << LOOLWSD::anonymizeUrl(request.getURI()) << "].");
+                LOG_ERR("Invalid PID in child URI [" << request.getURI() << "].");
                 return;
             }
 
             if (jailId.empty())
             {
-                LOG_ERR("Invalid JailId in child URI [" << LOOLWSD::anonymizeUrl(request.getURI()) << "].");
+                LOG_ERR("Invalid JailId in child URI [" << request.getURI() << "].");
                 return;
             }
 
@@ -1922,7 +1923,9 @@ private:
                 else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws" &&
                          request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0)
                 {
-                    handleClientWsUpgrade(request, reqPathTokens[1], disposition);
+                    std::string decodedUri;
+                    Poco::URI::decode(reqPathTokens[1], decodedUri);
+                    handleClientWsUpgrade(request, decodedUri, disposition);
                 }
                 else
                 {
@@ -2343,6 +2346,14 @@ private:
 #endif
             }
 
+            LOG_INF("URL [" << url << "].");
+            const auto uriPublic = DocumentBroker::sanitizeURI(url);
+            LOG_INF("URI [" << uriPublic.getPath() << "].");
+            const auto docKey = DocumentBroker::getDocKey(uriPublic);
+            LOG_INF("DocKey [" << docKey << "].");
+            const std::string fileId = Util::getFilenameFromURL(docKey);
+            Util::mapAnonymized(fileId, fileId); // Identity mapping, since fileId is already obfuscated
+
             LOG_INF("Starting GET request handler for session [" << _id << "] on url [" << LOOLWSD::anonymizeUrl(url) << "].");
 
             // Indicate to the client that document broker is searching.
@@ -2350,10 +2361,6 @@ private:
             LOG_TRC("Sending to Client [" << status << "].");
             ws.sendMessage(status);
 
-            const auto uriPublic = DocumentBroker::sanitizeURI(url);
-            const auto docKey = DocumentBroker::getDocKey(uriPublic);
-            const std::string fileId = Util::getFilenameFromPath(docKey);
-            Util::mapAnonymized(fileId, fileId); // Identity mapping, since fileId is already obfuscated
             LOG_INF("Sanitized URI [" << LOOLWSD::anonymizeUrl(url) << "] to [" << LOOLWSD::anonymizeUrl(uriPublic.toString()) <<
                     "] and mapped to docKey [" << LOOLWSD::anonymizeUrl(docKey) << "] for session [" << _id << "].");
 
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index f71e33455..17b207f84 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -33,6 +33,7 @@
 #include <Poco/Net/SSLManager.h>
 #include <Poco/StreamCopier.h>
 #include <Poco/Timestamp.h>
+#include <Poco/URI.h>
 
 // For residual Poco SSL usage.
 #include <Poco/Net/AcceptCertificateHandler.h>
@@ -470,7 +471,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Au
         // Anonymize key values.
         if (LOOLWSD::AnonymizeFilenames || LOOLWSD::AnonymizeUsernames)
         {
-            Util::mapAnonymized(filename, Util::getFilenameFromPath(_uri.toString()));
+            Util::mapAnonymized(Util::getFilenameFromURL(filename), Util::getFilenameFromURL(_uri.toString()));
 
             JsonUtil::findJSONValue(object, "ObfuscatedUserId", obfuscatedUserId, false);
             if (!obfuscatedUserId.empty())
@@ -721,9 +722,17 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
                 if (JsonUtil::parseJSON(responseString, object))
                 {
                     // Anonymize the filename
+                    std::string url;
+                    JsonUtil::findJSONValue(object, "Url", url);
+                    std::string decodedUrl;
+                    Poco::URI::decode(url, decodedUrl);
+                    const std::string obfuscatedFileId = Util::getFilenameFromURL(decodedUrl);
+
                     std::string filename;
                     JsonUtil::findJSONValue(object, "Name", filename);
-                    object->set("Name", LOOLWSD::anonymizeUsername(filename));
+                    const std::string filenameOnly = Util::getFilenameFromURL(filename);
+                    Util::mapAnonymized(filenameOnly, obfuscatedFileId);
+                    object->set("Name", LOOLWSD::anonymizeUrl(filename));
                     // Stringify to log.
                     std::ostringstream ossResponse;
                     object->stringify(ossResponse);


More information about the Libreoffice-commits mailing list