[Libreoffice-commits] online.git: common/Session.hpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/Storage.cpp wsd/Storage.hpp wsd/TestStubs.cpp

[Mike Kaganski (via logerrit)]

 common/Session.hpp     |    2 +-
 wsd/ClientSession.cpp  |   14 +++++++++++---
 wsd/ClientSession.hpp  |    7 ++++++-
 wsd/DocumentBroker.cpp |    6 ++++++
 wsd/Storage.cpp        |    4 ++++
 wsd/Storage.hpp        |    2 ++
 wsd/TestStubs.cpp      |    2 +-
 7 files changed, 31 insertions(+), 6 deletions(-)

New commits:
commit e9c4c0286af8e139dd2c3d4bddfcf9d3736e6e1e
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Wed Jul 1 11:34:08 2020 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Mon Jul 20 15:47:33 2020 +0200

    Handle failed locking as (temporarily) read-only session
    
    E.g., opening a checked-out document in SharePoint
    
    Change-Id: Ifd5225d8450d7f2f5ba9661f158551c5c16f9b09
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97596
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>

diff --git a/common/Session.hpp b/common/Session.hpp
index 64e5e1502..b42cc38e3 100644
--- a/common/Session.hpp
+++ b/common/Session.hpp
@@ -76,7 +76,7 @@ public:
     const std::string& getName() const { return _name; }
     bool isDisconnected() const { return _disconnected; }
 
-    virtual void setReadOnly() { _isReadOnly = true; }
+    virtual void setReadOnly(bool bVal = true) { _isReadOnly = bVal; }
     bool isReadOnly() const { return _isReadOnly; }
 
     /// overridden to prepend client ids on messages by the Kit
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 0f5a09b2c..6536b08fd 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -977,11 +977,19 @@ bool ClientSession::filterMessage(const std::string& message) const
     return allowed;
 }
 
-void ClientSession::setReadOnly()
+void ClientSession::setReadOnly(bool bVal)
 {
-    Session::setReadOnly();
+    Session::setReadOnly(bVal);
     // Also inform the client
-    sendTextFrame("perm: readonly");
+    const std::string sPerm = bVal ? "readonly" : "edit";
+    sendTextFrame("perm: " + sPerm);
+}
+
+void ClientSession::setLockFailed(const std::string& sReason)
+{
+    _isLockFailed = true;
+    setReadOnly();
+    sendTextFrame("lockfailed:" + sReason);
 }
 
 bool ClientSession::hasQueuedMessages() const
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 9802e889c..ab3402935 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -38,7 +38,8 @@ public:
     void construct();
     virtual ~ClientSession();
 
-    void setReadOnly() override;
+    void setReadOnly(bool bVal = true) override;
+    void setLockFailed(const std::string& sReason);
 
     enum SessionState {
         DETACHED,        // initial
@@ -251,6 +252,10 @@ private:
     /// Whether this session is the owner of currently opened document
     bool _isDocumentOwner;
 
+    /// If it is allowed to try to switch from read-only to edit mode,
+    /// because it's read-only just because of transient lock failure.
+    bool _isLockFailed = false;
+
     /// The socket to which the converted (saveas) doc is sent.
     std::shared_ptr<StreamSocket> _saveAsSocket;
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index c4f80aa61..0de4f15f9 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -782,9 +782,15 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
             session->getAuthorization(), session->getCookies(), *_lockCtx, templateSource);
 
         // Only lock the document on storage for editing sessions
+        // FIXME: why not lock before loadStorageFileToLocal? Would also prevent race conditions
         if (!session->isReadOnly() &&
             !_storage->updateLockState(session->getAuthorization(), session->getCookies(), *_lockCtx, true))
+        {
             LOG_ERR("Failed to lock!");
+            session->setLockFailed(_lockCtx->_lockFailureReason);
+            // TODO: make this "read-only" a special one with a notification (infobar? balloon tip?)
+            //       and a button to unlock
+        }
 
 #if !MOBILEAPP
         // Check if we have a prefilter "plugin" for this document format
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index e67c568ac..08a974d0d 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -770,6 +770,7 @@ WopiStorage::WOPIFileInfo::WOPIFileInfo(const FileInfo &fileInfo,
 bool WopiStorage::updateLockState(const Authorization& auth, const std::string& cookies,
                                   LockContext& lockCtx, bool lock)
 {
+    lockCtx._lockFailureReason.clear();
     if (!lockCtx._supportsLocks)
         return true;
 
@@ -821,7 +822,10 @@ bool WopiStorage::updateLockState(const Authorization& auth, const std::string&
         {
             std::string sMoreInfo = response.get("X-WOPI-LockFailureReason", "");
             if (!sMoreInfo.empty())
+            {
+                lockCtx._lockFailureReason = sMoreInfo;
                 sMoreInfo = ", failure reason: \"" + sMoreInfo + "\"";
+            }
             LOG_WRN("Un-successful " << wopiLog << " with status " << response.getStatus() <<
                     sMoreInfo << " and response: " << responseString);
         }
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index d087fb174..df1b59e5e 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -38,6 +38,8 @@ struct LockContext
     std::string _lockToken;
     /// Time of last successful lock (re-)acquisition
     std::chrono::steady_clock::time_point _lastLockTime;
+    /// Reason for unsuccessful locking request
+    std::string _lockFailureReason;
 
     LockContext() : _supportsLocks(false), _isLocked(false) { }
 
diff --git a/wsd/TestStubs.cpp b/wsd/TestStubs.cpp
index ad0859a8e..1efd3251f 100644
--- a/wsd/TestStubs.cpp
+++ b/wsd/TestStubs.cpp
@@ -33,7 +33,7 @@ void ClientSession::writeQueuedMessages() {}
 
 void ClientSession::dumpState(std::ostream& /*os*/) {}
 
-void ClientSession::setReadOnly() {}
+void ClientSession::setReadOnly(bool) {}
 
 bool ClientSession::_handleInput(const char* /*buffer*/, int /*length*/) { return false; }