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

Michael Meeks (via logerrit) logerrit at kemper.freedesktop.org
Thu Dec 12 06:43:00 UTC 2019


 wsd/DocumentBroker.cpp |   74 +++++++++++++++++++++++++++++++++++++++++++++++--
 wsd/DocumentBroker.hpp |   17 +++++++++--
 wsd/LOOLWSD.cpp        |   63 +----------------------------------------
 3 files changed, 89 insertions(+), 65 deletions(-)

New commits:
commit 948b424abbb3ee493d148896492d18186e3507f2
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Thu Dec 12 05:09:35 2019 +0000
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Thu Dec 12 07:42:42 2019 +0100

    convert-to: wait for load to complete before attempting the save.
    
    Change-Id: Iee3a8a6720bbc29fc4e113bf705f405b840e1e45
    Reviewed-on: https://gerrit.libreoffice.org/85009
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index af756c676..700a56fbf 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -2195,14 +2195,64 @@ size_t ConvertToBroker::getInstanceCount()
 
 ConvertToBroker::ConvertToBroker(const std::string& uri,
                                  const Poco::URI& uriPublic,
-                                 const std::string& docKey)
-    : DocumentBroker(uri, uriPublic, docKey)
+                                 const std::string& docKey,
+                                 const std::string& format,
+                                 const std::string& sOptions) :
+    DocumentBroker(uri, uriPublic, docKey),
+    _format(format),
+    _sOptions(sOptions)
 {
     static const int limit_convert_secs = LOOLWSD::getConfigValue<int>("per_document.limit_convert_secs", 100);
     NumConverters++;
     _limitLifeSeconds = limit_convert_secs;
 }
 
+bool ConvertToBroker::startConversion(SocketDisposition &disposition, const std::string &id)
+{
+    std::shared_ptr<ConvertToBroker> docBroker = std::static_pointer_cast<ConvertToBroker>(shared_from_this());
+
+    // Create a session to load the document.
+    const bool isReadOnly = true;
+    _clientSession = std::make_shared<ClientSession>(id, docBroker, getPublicUri(), isReadOnly, "nocliphost");
+    _clientSession->construct();
+
+    if (!_clientSession)
+        return false;
+
+    disposition.setMove([docBroker] (const std::shared_ptr<Socket> &moveSocket)
+        {
+            // Perform all of this after removing the socket
+
+            // Make sure the thread is running before adding callback.
+            docBroker->startThread();
+
+            // We no longer own this socket.
+            moveSocket->setThreadOwner(std::thread::id(0));
+
+            docBroker->addCallback([docBroker, moveSocket]()
+                 {
+                     auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
+                     docBroker->_clientSession->setSaveAsSocket(streamSocket);
+
+                     // Move the socket into DocBroker.
+                     docBroker->addSocketToPoll(moveSocket);
+
+                     // First add and load the session.
+                     docBroker->addSession(docBroker->_clientSession);
+
+                     // Load the document manually and request saving in the target format.
+                     std::string encodedFrom;
+                     Poco::URI::encode(docBroker->getPublicUri().getPath(), "", encodedFrom);
+                     const std::string load = "load url=" + encodedFrom;
+                     std::vector<char> loadRequest(load.begin(), load.end());
+                     docBroker->_clientSession->handleMessage(true, WSOpCode::Text, loadRequest);
+
+                     // Save is done in the setLoaded
+                 });
+        });
+    return true;
+}
+
 void ConvertToBroker::dispose()
 {
     if (!_uriOrig.empty())
@@ -2233,6 +2283,26 @@ void ConvertToBroker::removeFile(const std::string &uriOrig)
     }
 }
 
+void ConvertToBroker::setLoaded()
+{
+    DocumentBroker::setLoaded();
+
+    // FIXME: Check for security violations.
+    Poco::Path toPath(getPublicUri().getPath());
+    toPath.setExtension(_format);
+    const std::string toJailURL = "file://" + std::string(JAILED_DOCUMENT_ROOT) + toPath.getFileName();
+    std::string encodedTo;
+    Poco::URI::encode(toJailURL, "", encodedTo);
+
+    // Convert it to the requested format.
+    const std::string saveAsCmd = "saveas url=" + encodedTo + " format=" + _format + " options=" + _sOptions;
+
+    // Send the save request ...
+    std::vector<char> saveasRequest(saveAsCmd.begin(), saveAsCmd.end());
+
+    _clientSession->handleMessage(true, WSOpCode::Text, saveasRequest);
+}
+
 std::vector<std::shared_ptr<ClientSession>> DocumentBroker::getSessionsTestOnlyUnsafe()
 {
     std::vector<std::shared_ptr<ClientSession>> result;
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 3b8f6e844..5632c56b6 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -245,7 +245,8 @@ public:
     /// Thread safe termination of this broker if it has a lingering thread
     void joinThread();
 
-    void setLoaded();
+    /// Notify that the load has completed
+    virtual void setLoaded();
 
     bool isDocumentChangedInStorage() { return _documentChangedInStorage; }
 
@@ -518,16 +519,28 @@ private:
 
 class ConvertToBroker : public DocumentBroker
 {
+    const std::string _format;
+    const std::string _sOptions;
+    std::shared_ptr<ClientSession> _clientSession;
+
 public:
     /// Construct DocumentBroker with URI and docKey
     ConvertToBroker(const std::string& uri,
                     const Poco::URI& uriPublic,
-                    const std::string& docKey);
+                    const std::string& docKey,
+                    const std::string& format,
+                    const std::string& sOptions);
     virtual ~ConvertToBroker();
 
+    /// Move socket to this broker for response & do conversion
+    bool startConversion(SocketDisposition &disposition, const std::string &id);
+
     /// Called when removed from the DocBrokers list
     void dispose() override;
 
+    /// When the load completes - lets start saving
+    void setLoaded() override;
+
     /// How many live conversions are running.
     static size_t getInstanceCount();
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 524415b6f..3a597c4a1 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2605,7 +2605,6 @@ private:
             if (tokens.count() > 3)
                 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())
@@ -2627,7 +2626,7 @@ private:
                 std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
 
                 LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
-                auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey);
+                auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey, format, sOptions);
                 handler.takeFile();
 
                 cleanupDocBrokers();
@@ -2636,70 +2635,12 @@ private:
                 DocBrokers.emplace(docKey, docBroker);
                 LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << "].");
 
-                // Load the document.
-                // TODO: Move to DocumentBroker.
-                const bool isReadOnly = true;
-                std::shared_ptr<ClientSession> clientSession = createNewClientSession(
-                    nullptr, _id, uriPublic, docBroker, isReadOnly, "nocliphost");
-                if (clientSession)
-                {
-                    disposition.setMove([docBroker, clientSession, format, sOptions]
-                                        (const std::shared_ptr<Socket> &moveSocket)
-                    {
-                        // Perform all of this after removing the socket
-
-                        // Make sure the thread is running before adding callback.
-                        docBroker->startThread();
-
-                        // We no longer own this socket.
-                        moveSocket->setThreadOwner(std::thread::id(0));
-
-                        docBroker->addCallback([docBroker, moveSocket, clientSession, format, sOptions]()
-                        {
-                            auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
-                            clientSession->setSaveAsSocket(streamSocket);
-
-                            // Move the socket into DocBroker.
-                            docBroker->addSocketToPoll(moveSocket);
-
-                            // First add and load the session.
-                            docBroker->addSession(clientSession);
-
-                            // Load the document manually and request saving in the target format.
-                            std::string encodedFrom;
-                            URI::encode(docBroker->getPublicUri().getPath(), "", encodedFrom);
-                            const std::string load = "load url=" + encodedFrom;
-                            std::vector<char> loadRequest(load.begin(), load.end());
-                            clientSession->handleMessage(true, WSOpCode::Text, loadRequest);
-
-                            // FIXME: Check for security violations.
-                            Path toPath(docBroker->getPublicUri().getPath());
-                            toPath.setExtension(format);
-                            const std::string toJailURL = "file://" + std::string(JAILED_DOCUMENT_ROOT) + toPath.getFileName();
-                            std::string encodedTo;
-                            URI::encode(toJailURL, "", encodedTo);
-
-                            // Convert it to the requested format.
-                            const auto saveas = "saveas url=" + encodedTo + " format=" + format + " options=" + sOptions;
-                            std::vector<char> saveasRequest(saveas.begin(), saveas.end());
-                            clientSession->handleMessage(true, WSOpCode::Text, saveasRequest);
-                        });
-                    });
-
-                    sent = true;
-                }
-                else
+                if (!docBroker->startConversion(disposition, _id))
                 {
                     LOG_WRN("Failed to create Client Session with id [" << _id << "] on docKey [" << docKey << "].");
                     cleanupDocBrokers();
                 }
             }
-
-            if (!sent)
-            {
-                // TODO: We should differentiate between bad request and failed conversion.
-                throw BadRequestException("Failed to convert and send file.");
-            }
             return;
         }
         else if (tokens.count() >= 4 && tokens[3] == "insertfile")


More information about the Libreoffice-commits mailing list