[Libreoffice-commits] online.git: 3 commits - loleaflet/dist loleaflet/src loolwsd/ClientSession.cpp loolwsd/ClientSession.hpp loolwsd/DocumentBroker.cpp loolwsd/LOOLWSD.cpp loolwsd/protocol.txt loolwsd/Storage.cpp loolwsd/Storage.hpp

Pranav Kant pranavk at collabora.co.uk
Wed Oct 19 15:50:00 UTC 2016


 loleaflet/dist/toolbar/toolbar.js |    3 ++-
 loleaflet/src/control/Parts.js    |    3 +++
 loleaflet/src/core/Socket.js      |   13 +++++++++++++
 loolwsd/ClientSession.cpp         |    7 +++++++
 loolwsd/ClientSession.hpp         |    1 +
 loolwsd/DocumentBroker.cpp        |   12 +++++++++++-
 loolwsd/LOOLWSD.cpp               |    3 +++
 loolwsd/Storage.cpp               |   10 +++++++---
 loolwsd/Storage.hpp               |    9 ++++++---
 loolwsd/protocol.txt              |    5 +++++
 10 files changed, 58 insertions(+), 8 deletions(-)

New commits:
commit 9f69359b33c40a2c196f43bee4c91cfc0d5b0a31
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Wed Oct 19 21:08:33 2016 +0530

    loleaflet: Disable these toolbar buttons too, in readonly mode
    
    Change-Id: I8bc661dd3cfd689c7c7ded367deacdba853153e4

diff --git a/loleaflet/dist/toolbar/toolbar.js b/loleaflet/dist/toolbar/toolbar.js
index 4f2b649..dc2b8b5 100644
--- a/loleaflet/dist/toolbar/toolbar.js
+++ b/loleaflet/dist/toolbar/toolbar.js
@@ -488,7 +488,8 @@ var formatButtons = {
 	'annotation': true, 'inserttable': true,
 	'fontcolor': true, 'backcolor': true, 'bullet': true, 'numbering': true,
 	'alignleft': true, 'alignhorizontal': true, 'alignright': true, 'alignblock': true,
-	'incrementindent': true, 'decrementindent': true, 'insertgraphic': true
+	'incrementindent': true, 'decrementindent': true, 'insertgraphic': true,
+	'insertfootnote': true, 'repair': true
 };
 
 var userJoinedPopupMessage = '<div>' + _('%user has joined') + '</div>';
commit 1be2a78564afd84172aba44edc4bab42cd30c34d
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Wed Oct 19 20:22:53 2016 +0530

    Handle WOPI's UserCanWrite to determine readonliness
    
    permission= parameter in URL is still supported, but overridden
    by UserCanWrite parameter.
    
    Also, introduce a new protocol message, perm: which dictates
    loleaflet about the permission rather than the other way around
    (only in case of WOPI)
    
    It is to be noted that by default loolwsd assumes very
    restrictive permissions, so not providing UserCanWrite in WOPI
    implementation by a WOPI host would lead to opening of only
    readonly session.
    
    Change-Id: I2013c1661fd491c79bb367a41e1a7036fa03f984

diff --git a/loleaflet/src/control/Parts.js b/loleaflet/src/control/Parts.js
index cdde4d1..1139594 100644
--- a/loleaflet/src/control/Parts.js
+++ b/loleaflet/src/control/Parts.js
@@ -295,6 +295,9 @@ L.Map.include({
 	},
 
 	getDocType: function () {
+		if (!this._docLayer)
+			return null;
+
 		return this._docLayer._docType;
 	}
 });
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index c0c9cdb..9258858 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -144,6 +144,19 @@ L.Socket = L.Class.extend({
 			                         lokitVersionObj.ProductVersion + lokitVersionObj.ProductExtension.replace('.10.','-') +
 			                         ' (git hash: ' + lokitVersionObj.BuildId.substring(0, 7) + ')');
 		}
+		else if (textMsg.startsWith('perm:')) {
+			var perm = textMsg.substring('perm:'.length);
+
+			// This message is often received very early before doclayer is initialized
+			// Change options.permission so that when docLayer is initialized, it
+			// picks up the new value of permission rather than something else
+			this._map.options.permission = 'readonly';
+			// Lets also try to set the permission ourself since this can well be received
+			// after doclayer is initialized. There's no harm to call this in any case.
+			this._map.setPermission(perm);
+
+			return;
+		}
 		else if (textMsg.startsWith('error:') && command.errorCmd === 'internal') {
 			this._map._fatal = true;
 			if (command.errorKind === 'diskfull') {
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index 2aad600..ee5510b 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -368,4 +368,11 @@ bool ClientSession::forwardToChild(const std::string& message,
     return docBroker->forwardToChild(getId(), message);
 }
 
+void ClientSession::setReadOnly()
+{
+    _isReadOnly = true;
+    // Also inform the client
+    sendTextFrame("perm: readonly");
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index 201e7c1..6ba901b 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -30,6 +30,7 @@ public:
 
     virtual ~ClientSession();
 
+    void setReadOnly();
     bool isReadOnly() const { return _isReadOnly; }
 
     void setPeer(const std::shared_ptr<PrisonerSession>& peer) { _peer = peer; }
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 9ce4488..3960ef2 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -215,6 +215,11 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI
         }
         Log::debug("Setting username of the session to: " + fileInfo._userName);
         it->second->setUserName(fileInfo._userName);
+        if (!fileInfo._canWrite)
+        {
+            Log::debug("Setting the session as readonly");
+            it->second->setReadOnly();
+        }
 
         // Lets load the document now
         if (_storage->isLoaded())
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 312d7b6..df9cf2e 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -796,6 +796,9 @@ private:
                 isReadOnly = param.second == "readonly";
         }
 
+        // In case of WOPI and if this session is not set as readonly, it might be set so
+        // later after making a call to WOPI host which tells us the permission on files
+        // (UserCanWrite param)
         auto session = std::make_shared<ClientSession>(id, ws, docBroker, uriPublic, isReadOnly);
 
         // Above this point exceptions are safe and will auto-cleanup.
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index d500aae..f12c056 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -185,7 +185,7 @@ StorageBase::FileInfo LocalStorage::getFileInfo(const Poco::URI& uriPublic)
     const auto file = Poco::File(path);
     const auto lastModified = file.getLastModified();
     const auto size = file.getSize();
-    return FileInfo({filename, lastModified, size, "localhost", std::string("Local Host #") + std::to_string(_localStorageId)});
+    return FileInfo({filename, lastModified, size, "localhost", std::string("Local Host #") + std::to_string(_localStorageId), true});
 }
 
 std::string LocalStorage::loadStorageFileToLocal()
@@ -293,6 +293,7 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
     size_t size = 0;
     std::string userId;
     std::string userName;
+    bool canWrite = false;
     std::string resMsg;
     Poco::StreamCopier::copyToString(rs, resMsg);
 
@@ -313,10 +314,12 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
         userId = (userIdVar.isString() ? userIdVar.toString() : "");
         const auto userNameVar = object->get("UserFriendlyName");
         userName = (userNameVar.isString() ? userNameVar.toString() : "anonymous");
+        const auto canWriteVar = object->get("UserCanWrite");
+        canWrite = canWriteVar.isString() ? (canWriteVar.toString() == "true") : false;
     }
 
     // WOPI doesn't support file last modified time.
-    _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName});
+    _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName, canWrite});
     return _fileInfo;
 }
 
@@ -371,6 +374,7 @@ std::string WopiStorage::loadStorageFileToLocal()
 bool WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
 {
     Log::info("Uploading URI [" + uriPublic.toString() + "] from [" + _jailedFilePath + "].");
+    // TODO: Check if this URI has write permission (canWrite = true)
     const auto size = getFileSize(_jailedFilePath);
 
     Poco::URI uriObject(uriPublic);
@@ -406,7 +410,7 @@ StorageBase::FileInfo WebDAVStorage::getFileInfo(const Poco::URI& uriPublic)
 {
     Log::debug("Getting info for webdav uri [" + uriPublic.toString() + "].");
     assert(false && "Not Implemented!");
-    return FileInfo({"bazinga", Poco::Timestamp(), 0, "admin", "admin"});
+    return FileInfo({"bazinga", Poco::Timestamp(), 0, "admin", "admin", false});
 }
 
 std::string WebDAVStorage::loadStorageFileToLocal()
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 2e53b6a..163443c 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -35,12 +35,14 @@ public:
                  const Poco::Timestamp& modifiedTime,
                  size_t size,
                  const std::string& userId,
-                 const std::string& userName)
+                 const std::string& userName,
+                 const bool canWrite)
             : _filename(filename),
               _modifiedTime(modifiedTime),
               _size(size),
               _userId(userId),
-              _userName(userName)
+              _userName(userName),
+              _canWrite(canWrite)
         {
         }
 
@@ -54,6 +56,7 @@ public:
         size_t _size;
         std::string _userId;
         std::string _userName;
+        bool _canWrite;
     };
 
     /// localStorePath the absolute root path of the chroot.
@@ -64,7 +67,7 @@ public:
         _uri(uri),
         _localStorePath(localStorePath),
         _jailPath(jailPath),
-        _fileInfo("", Poco::Timestamp(), 0, "", ""),
+        _fileInfo("", Poco::Timestamp(), 0, "", "", false),
         _isLoaded(false)
     {
         Log::debug("Storage ctor: " + uri.toString());
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index 51d608a..eb40f7a 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -337,6 +337,11 @@ stats: <key> <value>
     Contains statistical data. Eg: 'stats: wopiduration 5' means that
     wopi calls made in loading of document took 5 seconds.
 
+perm: <permission>
+
+    <permission> can be one of 'edit', 'view', 'readonly'. Client must
+    change the UI accordingly (disabling buttons etc.)
+
 
 child -> parent
 ===============
commit ef32f300171d7e9824d86cf32f0e0954448b0014
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Wed Oct 19 15:51:29 2016 +0530

    loolwsd: Fix a crash due to invalid fileinfo
    
    Throw if document load fails. Ignoring such a scenario would lead
    to crash later in the document load process.
    
    Change-Id: Ie80fc4f26e08e4920d4c04726f947edd2837a0cf

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 3adc1ad..9ce4488 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -434,7 +434,12 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
         _lastEditableSession = false;
         _markToDestroy = false;
 
-        load(id, std::to_string(_childProcess->getPid()));
+        bool isLoaded = load(id, std::to_string(_childProcess->getPid()));
+        if (!isLoaded)
+        {
+            Log::error("Error loading document with URI [" + session->getPublicUri().toString() + "].");
+            throw;
+        }
     }
     catch (const StorageSpaceLowException&)
     {


More information about the Libreoffice-commits mailing list