[Libreoffice-commits] online.git: loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

[Ashod Nakashian]

 loolwsd/DocumentBroker.hpp |   46 +++++++++++++++++++++++++--------------------
 loolwsd/LOOLWSD.cpp        |   12 ++++++-----
 2 files changed, 33 insertions(+), 25 deletions(-)

New commits:
commit 66c8c0a3007737d3e07f8307bf732893de16dfad
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sat Mar 19 18:49:36 2016 -0400

    loolwsd: refactored DocumentBroker
    
    Change-Id: Ie7d9f46e49db8978541b4775fbf6d2578879a111
    Reviewed-on: https://gerrit.libreoffice.org/23449
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 141fb2d..2686af3 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -27,10 +27,8 @@ class DocumentBroker
 public:
 
     static
-    std::shared_ptr<DocumentBroker> create(std::string uri, const std::string& childRoot)
+    Poco::URI sanitizeURI(std::string uri)
     {
-        Log::info("Creating DocumentBroker for uri: " + uri + ".");
-
         // The URI of the document should be url-encoded.
         std::string decodedUri;
         Poco::URI::decode(uri, decodedUri);
@@ -47,10 +45,31 @@ public:
             throw std::runtime_error("Invalid URI.");
         }
 
+        return uriPublic;
+    }
+
+    /// Returns a document-specific key based
+    /// on the URI of the document.
+    static
+    std::string getDocKey(const Poco::URI& uri)
+    {
+        // Keep the host as part of the key to close a potential security hole.
         std::string docKey;
-        Poco::URI::encode(uriPublic.getPath(), "", docKey);
+        Poco::URI::encode(uri.getHost() + uri.getPath(), "", docKey);
+        return docKey;
+    }
 
-        return std::shared_ptr<DocumentBroker>(new DocumentBroker(uriPublic, docKey, childRoot));
+    DocumentBroker(const Poco::URI& uriPublic,
+                   const std::string& docKey,
+                   const std::string& childRoot) :
+       _uriPublic(uriPublic),
+       _docKey(docKey),
+       _childRoot(childRoot),
+       _sessionsCount(0)
+    {
+        assert(!_docKey.empty());
+        assert(!_childRoot.empty());
+        Log::info("DocumentBroker [" + _uriPublic.toString() + "] created. DocKey: [" + _docKey + "]");
     }
 
     ~DocumentBroker()
@@ -67,7 +86,8 @@ public:
 
         if (_storage)
         {
-            // Already loaded. Just return.
+            // Already loaded. Only validate.
+
             return true;
         }
 
@@ -123,20 +143,6 @@ public:
     }
 
 private:
-    DocumentBroker(const Poco::URI& uriPublic,
-                   const std::string& docKey,
-                   const std::string& childRoot) :
-       _uriPublic(uriPublic),
-       _docKey(docKey),
-       _childRoot(childRoot),
-       _sessionsCount(0)
-    {
-        assert(!_docKey.empty());
-        assert(!_childRoot.empty());
-        Log::info("DocumentBroker [" + _uriPublic.toString() + "] created. DocKey: [" + _docKey + "]");
-    }
-
-private:
     const Poco::URI _uriPublic;
     const std::string _docKey;
     const std::string _childRoot;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 1f7789d..38b57cc 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -372,8 +372,9 @@ private:
                 if (!format.empty())
                 {
                     Log::info("Conversion request for URI [" + fromPath + "].");
-                    auto docBroker = DocumentBroker::create(fromPath, LOOLWSD::ChildRoot);
-                    const auto docKey = docBroker->getDocKey();
+                    auto uriPublic = DocumentBroker::sanitizeURI(fromPath);
+                    const auto docKey = DocumentBroker::getDocKey(uriPublic);
+                    auto docBroker = std::make_shared<DocumentBroker>(uriPublic, docKey, LOOLWSD::ChildRoot);
 
                     // This lock could become a bottleneck.
                     // In that case, we can use a pool and index by publicPath.
@@ -552,9 +553,9 @@ private:
             uri.erase(0, 1);
         }
 
-        auto docBroker = DocumentBroker::create(uri, LOOLWSD::ChildRoot);
-        const auto docKey = docBroker->getDocKey();
-
+        const auto uriPublic = DocumentBroker::sanitizeURI(uri);
+        const auto docKey = DocumentBroker::getDocKey(uriPublic);
+        std::shared_ptr<DocumentBroker> docBroker;
         // This lock could become a bottleneck.
         // In that case, we can use a pool and index by publicPath.
         std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
@@ -572,6 +573,7 @@ private:
         {
             // Set one we just created.
             Log::debug("New DocumentBroker for docKey [" + docKey + "].");
+            docBroker = std::make_shared<DocumentBroker>(uriPublic, docKey, LOOLWSD::ChildRoot);
             docBrokers.emplace(docKey, docBroker);
         }