[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4' - 3 commits - wsd/ClientSession.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Mon Mar 11 07:58:46 UTC 2019


 wsd/ClientSession.cpp  |    5 -----
 wsd/DocumentBroker.cpp |   23 +++++++++++++++++------
 wsd/DocumentBroker.hpp |   21 +++++++++++++++++----
 wsd/LOOLWSD.cpp        |   30 ++++++++++++++++++------------
 4 files changed, 52 insertions(+), 27 deletions(-)

New commits:
commit 0f2e3641fcd27e7f6bbbdc13eb24462568eacec6
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Fri Mar 1 22:25:44 2019 +0100
Commit:     Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Mar 11 08:58:39 2019 +0100

    tdf#123482 - cleanup convert-to folder more reliably.
    
    Change-Id: I029bb4136984e05485e462c92da80b92b00fdebc
    Reviewed-on: https://gerrit.libreoffice.org/68601
    Reviewed-by: Aron Budea <aron.budea at collabora.com>
    Tested-by: Aron Budea <aron.budea at collabora.com>
    Reviewed-by: Andras Timar <andras.timar at collabora.com>

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 3c53fdb53..f3afc4efc 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -867,11 +867,6 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
 
             // Now terminate.
             docBroker->stop("Finished saveas handler.");
-
-            // Remove file and directory
-            Poco::Path path = docBroker->getDocKey();
-            Poco::File(path).remove();
-            Poco::File(path.makeParent()).remove();
         }
 
         return true;
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index aba041044..d9b205801 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -37,6 +37,7 @@
 #include <common/Message.hpp>
 #include <common/Protocol.hpp>
 #include <common/Unit.hpp>
+#include <common/FileUtil.hpp>
 
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -1887,6 +1888,18 @@ void DocumentBroker::getIOStats(uint64_t &sent, uint64_t &recv)
     }
 }
 
+ConvertToBroker::~ConvertToBroker()
+{
+    if (!_uriOrig.empty())
+    {
+        // Remove source file and directory
+        Poco::Path path = _uriOrig;
+        Poco::File(path).remove();
+        Poco::File(path.makeParent()).remove();
+        FileUtil::removeFile(_uriOrig);
+    }
+}
+
 void DocumentBroker::dumpState(std::ostream& os)
 {
     std::unique_lock<std::mutex> lock(_mutex);
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index a27442014..6d5a3e63b 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -223,7 +223,7 @@ public:
                    const Poco::URI& uriPublic,
                    const std::string& docKey);
 
-    ~DocumentBroker();
+    virtual ~DocumentBroker();
 
     /// Start processing events
     void startThread();
@@ -402,8 +402,9 @@ private:
     /// Sum the I/O stats from all connected sessions
     void getIOStats(uint64_t &sent, uint64_t &recv);
 
-private:
+protected:
     const std::string _uriOrig;
+private:
     const Poco::URI _uriPublic;
     /// URL-based key. May be repeated during the lifetime of WSD.
     const std::string _docKey;
@@ -472,6 +473,19 @@ private:
     static std::atomic<unsigned> DocBrokerId;
 };
 
+class ConvertToBroker : public DocumentBroker
+{
+public:
+    /// Construct DocumentBroker with URI and docKey
+    ConvertToBroker(const std::string& uri,
+                    const Poco::URI& uriPublic,
+                    const std::string& docKey)
+        : DocumentBroker(uri, uriPublic, docKey)
+    {
+    }
+    virtual ~ConvertToBroker();
+};
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index a0c9e31c2..9227f6f3f 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -593,6 +593,7 @@ public:
         if (!params.has("filename"))
             return;
 
+        // FIXME: needs wrapping - until then - keep in sync with ~ConvertToBroker
         Path tempPath = _convertTo? Path::forDirectory(Poco::TemporaryFile::tempName("/tmp/convert-to") + "/") :
                                     Path::forDirectory(Poco::TemporaryFile::tempName() + "/");
         File(tempPath).createDirectories();
@@ -2369,7 +2370,7 @@ private:
                     std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
 
                     LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
-                    auto docBroker = std::make_shared<DocumentBroker>(fromPath, uriPublic, docKey);
+                    auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey);
 
                     cleanupDocBrokers();
 
commit b79002b3ce57d9016cd6704fd70d5487e51297c0
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Fri Mar 1 22:03:35 2019 +0100
Commit:     Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Mar 11 08:58:32 2019 +0100

    Simpify DocumentBroker constructor.
    
    Change-Id: I0bf29df9316b129d34862c7464bb6636d42a850d
    Reviewed-on: https://gerrit.libreoffice.org/68600
    Reviewed-by: Aron Budea <aron.budea at collabora.com>
    Tested-by: Aron Budea <aron.budea at collabora.com>
    Reviewed-by: Andras Timar <andras.timar at collabora.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index b0052142e..aba041044 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -160,13 +160,11 @@ std::atomic<unsigned> DocumentBroker::DocBrokerId(1);
 
 DocumentBroker::DocumentBroker(const std::string& uri,
                                const Poco::URI& uriPublic,
-                               const std::string& docKey,
-                               const std::string& childRoot) :
+                               const std::string& docKey) :
     _uriOrig(uri),
     _uriPublic(uriPublic),
     _docKey(docKey),
     _docId(Util::encodeId(DocBrokerId++, 3)),
-    _childRoot(childRoot),
     _cacheRoot(getCachePath(uriPublic.toString())),
     _documentChangedInStorage(false),
     _lastSaveTime(std::chrono::steady_clock::now()),
@@ -186,10 +184,10 @@ DocumentBroker::DocumentBroker(const std::string& uri,
     _debugRenderedTileCount(0)
 {
     assert(!_docKey.empty());
-    assert(!_childRoot.empty());
+    assert(!LOOLWSD::ChildRoot.empty());
 
     LOG_INF("DocumentBroker [" << LOOLWSD::anonymizeUrl(_uriPublic.toString()) <<
-            "] created with docKey [" << _docKey << "] and root [" << _childRoot << "]");
+            "] created with docKey [" << _docKey << "]");
 }
 
 void DocumentBroker::startThread()
@@ -1077,7 +1075,7 @@ bool DocumentBroker::sendUnoSave(const std::string& sessionId, bool dontTerminat
 std::string DocumentBroker::getJailRoot() const
 {
     assert(!_jailId.empty());
-    return Poco::Path(_childRoot, _jailId).toString();
+    return Poco::Path(LOOLWSD::ChildRoot, _jailId).toString();
 }
 
 size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 3d47c722d..a27442014 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -221,8 +221,7 @@ public:
     /// Construct DocumentBroker with URI, docKey, and root path.
     DocumentBroker(const std::string& uri,
                    const Poco::URI& uriPublic,
-                   const std::string& docKey,
-                   const std::string& childRoot);
+                   const std::string& docKey);
 
     ~DocumentBroker();
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index ff81f6719..a0c9e31c2 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1707,7 +1707,7 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w
 
         // Set the one we just created.
         LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
-        docBroker = std::make_shared<DocumentBroker>(uri, uriPublic, docKey, LOOLWSD::ChildRoot);
+        docBroker = std::make_shared<DocumentBroker>(uri, uriPublic, docKey);
         DocBrokers.emplace(docKey, docBroker);
         LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << "].");
     }
@@ -2369,7 +2369,7 @@ private:
                     std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
 
                     LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
-                    auto docBroker = std::make_shared<DocumentBroker>(fromPath, uriPublic, docKey, LOOLWSD::ChildRoot);
+                    auto docBroker = std::make_shared<DocumentBroker>(fromPath, uriPublic, docKey);
 
                     cleanupDocBrokers();
 
commit 8878989794b4aa064810f48c565f050896c9ae9c
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Fri Mar 1 18:59:40 2019 +0100
Commit:     Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Mar 11 08:58:25 2019 +0100

    Avoid using un-necessary reference.
    
    Change-Id: Id5a9fed8fb790f2af8facac119e9e0da476b1e47
    Reviewed-on: https://gerrit.libreoffice.org/68599
    Reviewed-by: Aron Budea <aron.budea at collabora.com>
    Tested-by: Aron Budea <aron.budea at collabora.com>
    Reviewed-by: Andras Timar <andras.timar at collabora.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index f43aca9c5..ff81f6719 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -558,18 +558,24 @@ std::shared_ptr<ChildProcess> getNewChild_Blocks(
 
 #ifndef MOBILEAPP
 
-/// Handles the filename part of the convert-to POST request payload.
+/// Handles the filename part of the convert-to POST request payload,
+/// Also owns the file - cleaning it up when destroyed.
 class ConvertToPartHandler : public PartHandler
 {
-    std::string& _filename;
+    std::string _filename;
 
     /// Is it really a convert-to, ie. use an especially formed path?
     bool _convertTo;
 
 public:
-    ConvertToPartHandler(std::string& filename, bool convertTo = false)
-        : _filename(filename)
-        , _convertTo(convertTo)
+    std::string getFilename() const { return _filename; }
+
+    ConvertToPartHandler(bool convertTo = false)
+        : _convertTo(convertTo)
+    {
+    }
+
+    virtual ~ConvertToPartHandler()
     {
     }
 
@@ -2327,8 +2333,7 @@ private:
         StringTokenizer tokens(request.getURI(), "/?");
         if (tokens.count() > 2 && tokens[2] == "convert-to")
         {
-            std::string fromPath;
-            ConvertToPartHandler handler(fromPath, /*convertTo =*/ true);
+            ConvertToPartHandler handler(/*convertTo =*/ true);
             HTMLForm form(request, message, handler);
 
             std::string format = (form.has("format") ? form.get("format") : "");
@@ -2352,6 +2357,7 @@ private:
                 format = tokens[3];
 
             bool sent = false;
+            std::string fromPath = handler.getFilename();
             LOG_INF("Conversion request for URI [" << fromPath << "] format [" << format << "].");
             if (!fromPath.empty() && !format.empty())
             {
@@ -2437,8 +2443,7 @@ private:
         {
             LOG_INF("Insert file request.");
 
-            std::string tmpPath;
-            ConvertToPartHandler handler(tmpPath);
+            ConvertToPartHandler handler;
             HTMLForm form(request, message, handler);
 
             if (form.has("childid") && form.has("name"))
@@ -2468,7 +2473,7 @@ private:
                                               + JAILED_DOCUMENT_ROOT + "insertfile";
                     File(dirPath).createDirectories();
                     std::string fileName = dirPath + "/" + form.get("name");
-                    File(tmpPath).moveTo(fileName);
+                    File(handler.getFilename()).moveTo(fileName);
                     response.setContentLength(0);
                     socket->send(response);
                     return;


More information about the Libreoffice-commits mailing list