loleaflet/src/core/Socket.js | 4 +
wsd/DocumentBroker.cpp | 18 ++++++-
wsd/Storage.cpp | 100 +++++++++++++++++++++++++++++++------------
wsd/Storage.hpp | 7 +++
4 files changed, 100 insertions(+), 29 deletions(-)
New commits:
commit 4f60d5d359ace0c2671ebc68524ba5db7db08e34
Author: Pranav Kant <pran...@collabora.co.uk>
Date: Wed May 31 23:18:33 2017 +0530
Inform all clients when document changed behind our back
Introduce a new header X-LOOL-WOPI-Timestamp
This is a WOPI header extension to detect any external document change. For
example, when the file that is already opened by LOOL is changed
in storage.
The WOPI host sends LastModifiedTime field (in WOPI specs) as part
of the CheckFileInfo response. It also expects wsd to send the
same timestamp in X-LOOL-WOPI-Timestamp header during WOPI::PutFile. If
this header is present, then WOPI host checks, before saving the
document, if the timestamp in the header is equal to the timestamp of
the file in its storage. Only upon meeting this condition, it saves the
file back to storage, otherwise it informs us about some change
to the document.
We are supposed to inform the user accordingly. If user is okay
with over-writing the document, then we can omit sending
X-LOOL-WOPI-Timestamp header, in which case, no check as mentioned above
would be performed while saving the file and document will be
overwritten.
Also, use a separate list of LOOL status codes to denote such a change.
It would be wrong to use HTTP_CONFLICT status code for denoting doc
changed in storage scenario. WOPI specs reserves that for WOPI locks
which are not yet implemented. Better to use a separate LOOL specific
status codes synced across WOPI hosts and us to denote scenario that we
expect and are not covered in WOPI specs.
(cherry picked from commit ba4e75cfae5eb174f8b56254f3c9513bab2afed5)
Change-Id: I61539dfae672bc104b8008f030f96e90f9ff48a5
Reviewed-on: https://gerrit.libreoffice.org/38527
Reviewed-by: Jan Holesovsky <ke...@collabora.com>
Tested-by: Jan Holesovsky <ke...@collabora.com>
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index e137acca..894725aa 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -343,6 +343,10 @@ L.Socket = L.Class.extend({
this._map.hideBusy();
this.close();
}
+ else if (command.errorKind === 'documentconflict')
+ {
+ storageError =
errorMessages.storage.documentconflict;
+ }
// Parse the storage url as link
var tmpLink = document.createElement('a');
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 138b9bb7..2157e191 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -492,8 +492,12 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
fileInfo._modifiedTime != Zero &&
_documentLastModifiedTime != fileInfo._modifiedTime)
{
- LOG_ERR("Document has been modified behind our back, URI [" <<
session->getPublicUri().toString() << "].");
- // What do do?
+ LOG_WRN("Document [" << _docKey << "] has been modified behind our
back. Informing all clients.");
+ // Inform all clients
+ for (const auto& sessionIt : _sessions)
+ {
+ sessionIt.second->sendTextFrame("error: cmd=storage
kind=documentconflict");
+ }
}
}
@@ -644,6 +648,16 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId,
LOG_ERR("Failed to save docKey [" << _docKey << "] to URI [" << uri <<
"]. Notifying client.");
it->second->sendTextFrame("error: cmd=storage kind=savefailed");
}
+ else if (storageSaveResult == StorageBase::SaveResult::DOC_CHANGED)
+ {
+ LOG_ERR("PutFile says that Document changed in storage");
+
+ // Inform all clients
+ for (const auto& sessionIt : _sessions)
+ {
+ sessionIt.second->sendTextFrame("error: cmd=storage
kind=documentconflict");
+ }
+ }
return false;
}
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 5725c58e..aee09ae9 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -351,6 +351,27 @@ int getLevenshteinDist(const std::string& string1, const
std::string& string2) {
return matrix[string1.size()][string2.size()];
}
+// Gets value for `key` directly from the given JSON in `object`
+template <typename T>
+T getJSONValue(const Poco::JSON::Object::Ptr &object, const std::string& key)
+{
+ T value = T();
+ try
+ {
+ const Poco::Dynamic::Var valueVar = object->get(key);
+ value = valueVar.convert<T>();
+ }
+ catch (const Poco::Exception& exc)
+ {
+ LOG_ERR("getJSONValue: " << exc.displayText() <<
+ (exc.nested() ? " (" + exc.nested()->displayText() + ")" :
""));
+ }
+
+ return value;
+}
+
+// Function that searches `object` for `key` and warns if there are minor
mis-spellings involved
+// Upon successfull search, fills `value` with value found in object.
template <typename T>
void getWOPIValue(const Poco::JSON::Object::Ptr &object, const std::string&
key, T& value)
{
@@ -375,23 +396,31 @@ void getWOPIValue(const Poco::JSON::Object::Ptr &object,
const std::string& key,
return;
}
- try
- {
- const Poco::Dynamic::Var valueVar = object->get(userInput);
- value = valueVar.convert<T>();
- }
- catch (const Poco::Exception& exc)
- {
- LOG_ERR("getWOPIValue: " << exc.displayText() <<
- (exc.nested() ? " (" + exc.nested()->displayText() + ")" :
""));
- }
-
+ value = getJSONValue<T>(object, userInput);
return;
}
LOG_WRN("Missing JSON property [" << key << "]");
}
+// Parse the json string and fill the Poco::JSON object
+// Returns true if parsing successful otherwise false
+bool parseJSON(const std::string& json, Poco::JSON::Object::Ptr& object)
+{
+ bool success = false;
+ const auto index = json.find_first_of("{");
+ if (index != std::string::npos)
+ {
+ const std::string stringJSON = json.substr(index);
+ Poco::JSON::Parser parser;
+ const auto result = parser.parse(stringJSON);
+ object = result.extract<Poco::JSON::Object::Ptr>();
+ success = true;
+ }
+
+ return success;
+}
+
void setQueryParameter(Poco::URI& uriObject, const std::string& key, const
std::string& value)
{
Poco::URI::QueryParameters queryParams = uriObject.getQueryParameters();
@@ -447,6 +476,8 @@ Poco::Timestamp iso8601ToTimestamp(const std::string&
iso8601Time)
return timestamp;
}
+
+
} // anonymous namespace
std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const
std::string& accessToken)
@@ -519,13 +550,9 @@ std::unique_ptr<WopiStorage::WOPIFileInfo>
WopiStorage::getWOPIFileInfo(const st
std::string lastModifiedTime;
LOG_DBG("WOPI::CheckFileInfo returned: " << resMsg << ". Call duration: "
<< callDuration.count() << "s");
- const auto index = resMsg.find_first_of('{');
- if (index != std::string::npos)
+ Poco::JSON::Object::Ptr object;
+ if (parseJSON(resMsg, object))
{
- const std::string stringJSON = resMsg.substr(index);
- Poco::JSON::Parser parser;
- const auto result = parser.parse(stringJSON);
- const auto& object = result.extract<Poco::JSON::Object::Ptr>();
getWOPIValue(object, "BaseFileName", filename);
getWOPIValue(object, "Size", size);
getWOPIValue(object, "OwnerId", ownerId);
@@ -549,7 +576,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo>
WopiStorage::getWOPIFileInfo(const st
throw UnauthorizedRequestException("Access denied. WOPI::CheckFileInfo
failed on: " + uriObject.toString());
}
- modifiedTime = iso8601ToTimestamp(lastModifiedTime);
+ const Poco::Timestamp modifiedTime = iso8601ToTimestamp(lastModifiedTime);
_fileInfo = FileInfo({filename, ownerId, modifiedTime, size});
return std::unique_ptr<WopiStorage::WOPIFileInfo>(new
WOPIFileInfo({userId, userName, userExtraInfo, canWrite, postMessageOrigin,
hidePrintOption, hideSaveOption, hideExportOption, enableOwnerTermination,
disablePrint, disableExport, disableCopy, callDuration}));
@@ -641,6 +668,9 @@ StorageBase::SaveResult
WopiStorage::saveLocalFileToStorage(const std::string& a
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST,
uriObject.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
request.set("X-WOPI-Override", "PUT");
+ request.set("X-LOOL-WOPI-Timestamp",
+
Poco::DateTimeFormatter::format(Poco::DateTime(_fileInfo._modifiedTime),
+
Poco::DateTimeFormat::ISO8601_FRAC_FORMAT));
request.setContentType("application/octet-stream");
request.setContentLength(size);
addStorageDebugCookie(request);
@@ -659,18 +689,17 @@ StorageBase::SaveResult
WopiStorage::saveLocalFileToStorage(const std::string& a
if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_OK)
{
saveResult = StorageBase::SaveResult::OK;
- const auto index = oss.str().find_first_of('{');
- if (index != std::string::npos)
+ Poco::JSON::Object::Ptr object;
+ if (parseJSON(oss.str(), object))
{
- const std::string stringJSON = oss.str().substr(index);
- Poco::JSON::Parser parser;
- const auto result = parser.parse(stringJSON);
- const auto& object = result.extract<Poco::JSON::Object::Ptr>();
-
- std::string lastModifiedTime;
- getWOPIValue(object, "LastFileModifiedTime", lastModifiedTime);
+ const std::string lastModifiedTime =
getJSONValue<std::string>(object, "LastModifiedTime");
+ LOG_TRC("WOPI::PutFile returns LastModifiedTime [" <<
lastModifiedTime << "].");
_fileInfo._modifiedTime = iso8601ToTimestamp(lastModifiedTime);
}
+ else
+ {
+ LOG_WRN("Invalid/Missing JSON found in WOPI::PutFile
response");
+ }
}
else if (response.getStatus() ==
Poco::Net::HTTPResponse::HTTP_REQUESTENTITYTOOLARGE)
{
@@ -680,6 +709,23 @@ StorageBase::SaveResult
WopiStorage::saveLocalFileToStorage(const std::string& a
{
saveResult = StorageBase::SaveResult::UNAUTHORIZED;
}
+ else if (response.getStatus() ==
Poco::Net::HTTPResponse::HTTP_CONFLICT)
+ {
+ saveResult = StorageBase::SaveResult::CONFLICT;
+ Poco::JSON::Object::Ptr object;
+ if (parseJSON(oss.str(), object))
+ {
+ const unsigned loolStatusCode = getJSONValue<unsigned>(object,
"LOOLStatusCode");
+ if (loolStatusCode ==
static_cast<unsigned>(LOOLStatusCode::DOC_CHANGED))
+ {
+ saveResult = StorageBase::SaveResult::DOC_CHANGED;
+ }
+ }
+ else
+ {
+ LOG_WRN("Invalid/missing JSON in WOPI::PutFile response");
+ }
+ }
}
catch(const Poco::Exception& pexc)
{
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index ed0331c0..dc29685b 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -58,9 +58,16 @@ public:
OK,
DISKFULL,
UNAUTHORIZED,
+ DOC_CHANGED, /* Document changed in storage */
+ CONFLICT,
FAILED
};
+ enum class LOOLStatusCode
+ {
+ DOC_CHANGED = 1010 // Document changed externally in storage
+ };
+
/// localStorePath the absolute root path of the chroot.
/// jailPath the path within the jail that the child uses.
StorageBase(const Poco::URI& uri,
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
