[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

[Ashod Nakashian]

 wsd/DocumentBroker.cpp |   29 ++++++++++++++++++++++++-----
 wsd/DocumentBroker.hpp |    3 +++
 wsd/LOOLWSD.cpp        |    6 +++---
 3 files changed, 30 insertions(+), 8 deletions(-)

New commits:
commit af0011b2b65c7237b69c1a47a31b77f6f872ffbf
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Mon May 22 15:04:35 2017 -0400

    wsd: don't stop doc on unauthorized loading
    
    When a client connects with expired/invalid
    access_token, the document should remain
    active for other/existing clients, if any.
    
    However, if no clients exists (i.e. the
    first client has invalid access_token),
    then the document should be unloaded and
    cleaned up.
    
    Reviewed-on: https://gerrit.libreoffice.org/37916
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>
    (cherry picked from commit e877adc84bc6f90bcfa52f9f63666358e6614885)
    
    Change-Id: Iaad95a4286325cc6ee130b37e3ad635993a71c72
    Reviewed-on: https://gerrit.libreoffice.org/37918
    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 5b6fb3c7..4ddab06b 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -801,6 +801,25 @@ std::string DocumentBroker::getJailRoot() const
 
 size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
 {
+    try
+    {
+        return addSessionInternal(session);
+    }
+    catch (const std::exception& exc)
+    {
+        LOG_ERR("Failed to add session to [" << _docKey << "] with URI [" << session->getPublicUri().toString() << "]: " << exc.what());
+        if (_sessions.empty())
+        {
+            LOG_INF("Doc [" << _docKey << "] has no more sessions. Marking to destroy.");
+            _markToDestroy = true;
+        }
+
+        throw;
+    }
+}
+
+size_t DocumentBroker::addSessionInternal(const std::shared_ptr<ClientSession>& session)
+{
     assertCorrectThread();
 
     try
@@ -831,12 +850,7 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
     _markToDestroy = false;
     _stop = false;
 
-    // Add and attach the session.
-    _sessions.emplace(session->getId(), session);
-    session->setAttached();
-
     const auto id = session->getId();
-    const auto count = _sessions.size();
 
     // Request a new session from the child kit.
     const std::string aMessage = "session " + id + ' ' + _docKey + ' ' + _docId;
@@ -845,6 +859,11 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
     // Tell the admin console about this new doc
     Admin::instance().addDoc(_docKey, getPid(), getFilename(), id);
 
+    // Add and attach the session.
+    _sessions.emplace(session->getId(), session);
+    session->setAttached();
+
+    const auto count = _sessions.size();
     LOG_TRC("Added " << (session->isReadOnly() ? "readonly" : "non-readonly") <<
             " session [" << id << "] to docKey [" <<
             _docKey << "] to have " << count << " sessions.");
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 46481cba..4fe17501 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -346,6 +346,9 @@ private:
     /// Saves the doc to the storage.
     bool saveToStorageInternal(const std::string& sesionId, bool success, const std::string& result = "");
 
+    /// Loads a new session and adds to the sessions container.
+    size_t addSessionInternal(const std::shared_ptr<ClientSession>& session);
+
     /// Removes a session by ID. Returns the new number of sessions.
     size_t removeSessionInternal(const std::string& id);
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 03ed228f..5f9fd929 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1543,6 +1543,7 @@ private:
     {
         auto socket = _socket.lock();
         std::vector<char>& in = socket->_inBuffer;
+        LOG_TRC("#" << socket->getFD() << " handling incoming " << in.size() << " bytes.");
 
         // Find the end of the header, if any.
         static const std::string marker("\r\n\r\n");
@@ -1619,6 +1620,7 @@ private:
                             Admin::instance().insertNewSocket(moveSocket);
                         });
                 }
+
             }
             // Client post and websocket connections
             else if ((request.getMethod() == HTTPRequest::HTTP_GET ||
@@ -1869,7 +1871,7 @@ private:
 
                         docBroker->addCallback([docBroker, moveSocket, clientSession, format]()
                         {
-			    auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
+                            auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
                             clientSession->setSaveAsSocket(streamSocket);
 
                             // Move the socket into DocBroker.
@@ -2113,14 +2115,12 @@ private:
                                 LOG_ERR("Unauthorized Request while loading session for " << docBroker->getDocKey() << ": " << exc.what());
                                 const std::string msg = "error: cmd=internal kind=unauthorized";
                                 clientSession->sendMessage(msg);
-                                docBroker->stop();
                             }
                             catch (const std::exception& exc)
                             {
                                 LOG_ERR("Error while handling loading : " << exc.what());
                                 const std::string msg = "error: cmd=internal kind=unauthorized";
                                 clientSession->sendMessage(msg);
-                                docBroker->stop();
                             }
                         });
                     });