[Libreoffice-commits] online.git: 3 commits - loleaflet/dist loleaflet/src wsd/DocumentBroker.cpp wsd/Storage.cpp wsd/Storage.hpp
Pranav Kant
pranavk at collabora.co.uk
Thu Jun 1 10:47:37 UTC 2017
loleaflet/dist/errormessages.js | 3
loleaflet/src/core/Socket.js | 4 +
wsd/DocumentBroker.cpp | 31 ++++----
wsd/Storage.cpp | 138 ++++++++++++++++++++++++++++------------
wsd/Storage.hpp | 7 ++
5 files changed, 128 insertions(+), 55 deletions(-)
New commits:
commit ba4e75cfae5eb174f8b56254f3c9513bab2afed5
Author: Pranav Kant <pranavk at 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.
Change-Id: I61539dfae672bc104b8008f030f96e90f9ff48a5
diff --git a/loleaflet/dist/errormessages.js b/loleaflet/dist/errormessages.js
index 8ea1185c..08cd395b 100644
--- a/loleaflet/dist/errormessages.js
+++ b/loleaflet/dist/errormessages.js
@@ -13,5 +13,6 @@ exports.storage = {
loadfailed: _('Failed to read document from storage. Please contact your storage server (%storageserver) administrator.'),
savediskfull: _('Save failed due to no disk space left on storage server. Document will now be read-only. Please contact the server (%storageserver) administrator to continue editing.'),
saveunauthorized: _('Document cannot be saved to storage server (%storageserver) due to expired or invalid access token. Refresh your session to not to lose your work.'),
- savefailed: _('Document cannot be saved to storage. Check your permissions or contact the storage server (%storageserver) administrator.')
+ savefailed: _('Document cannot be saved to storage. Check your permissions or contact the storage server (%storageserver) administrator.'),
+ documentconflict: _('Document has been changed in storage outside WOPI.')
};
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index fdce2a54..37b67fd4 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -331,6 +331,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 211814e5..a97e34da 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -494,8 +494,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");
+ }
}
}
@@ -645,6 +649,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 e52bcd41..394b9136 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 34b56de3..89cc4de9 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,
commit 15a4474572a231f6a3f3ef713d2e69cea0af7297
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed May 31 23:04:32 2017 +0530
Remove superfluous WOPI calls to getFileInfo to check timestamp
One can add the timetamp information in the PutFile call itself. This
way we can avoid making an extra CheckFileInfo call here.
Change-Id: Iae180262e648c36b9cfeb6d5fabdf5d243b93afb
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index e5e8a19c..211814e5 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -614,19 +614,6 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
_lastSaveTime = std::chrono::steady_clock::now();
_poll->wakeup();
- // Calling getWOPIFileInfo() or getLocalFileInfo() has the side-effect of updating
- // StorageBase::_fileInfo. Get the timestamp of the document as persisted in its storage
- // from there.
- // FIXME: Yes, of course we should turn this stuff into a virtual function and avoid this
- // dynamic_cast dance.
- if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr)
- {
- auto wopiFileInfo = static_cast<WopiStorage*>(_storage.get())->getWOPIFileInfo(accessToken);
- }
- else if (dynamic_cast<LocalStorage*>(_storage.get()) != nullptr)
- {
- auto localFileInfo = static_cast<LocalStorage*>(_storage.get())->getLocalFileInfo();
- }
// So set _documentLastModifiedTime then
_documentLastModifiedTime = _storage->getFileInfo()._modifiedTime;
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 91d95eca..e52bcd41 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -291,6 +291,10 @@ StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const std::string&
{
LOG_INF("Copying " << _jailedFilePath << " to " << _uri.getPath());
Poco::File(_jailedFilePath).copyTo(_uri.getPath());
+
+ // update its fileinfo object. This is used later to check if someone else changed the
+ // document while we are/were editing it
+ _fileInfo._modifiedTime = Poco::File(_uri.getPath()).getLastModified();
}
}
catch (const Poco::Exception& exc)
commit cf968e6768696872b30d51f027959489e9ca0d16
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed May 31 22:52:54 2017 +0530
Factor out iso8601 to Poco::Timestamp parsing
Change-Id: I627a7b6b72899371e880e461685f99a86a858232
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index a698b04f..91d95eca 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -424,6 +424,25 @@ void addStorageDebugCookie(Poco::Net::HTTPRequest& request)
#endif
}
+Poco::Timestamp iso8601ToTimestamp(const std::string& iso8601Time)
+{
+ Poco::Timestamp timestamp = Poco::Timestamp::fromEpochTime(0);
+ try
+ {
+ int timeZoneDifferential;
+ Poco::DateTime dateTime;
+ Poco::DateTimeParser::parse(Poco::DateTimeFormat::ISO8601_FRAC_FORMAT, iso8601Time, dateTime, timeZoneDifferential);
+ timestamp = dateTime.timestamp();
+ }
+ catch (const Poco::SyntaxException& exc)
+ {
+ LOG_WRN("Time [" << iso8601Time << "] is in invalid format: " << exc.displayText() <<
+ (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
+ }
+
+ return timestamp;
+}
+
} // anonymous namespace
std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const std::string& accessToken)
@@ -526,28 +545,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const st
throw UnauthorizedRequestException("Access denied. WOPI::CheckFileInfo failed on: " + uriObject.toString());
}
- Poco::Timestamp modifiedTime = Poco::Timestamp::fromEpochTime(0);
- if (lastModifiedTime != "")
- {
- Poco::DateTime dateTime;
- int timeZoneDifferential;
- bool valid = false;
- try
- {
- Poco::DateTimeParser::parse(Poco::DateTimeFormat::ISO8601_FRAC_FORMAT, lastModifiedTime, dateTime, timeZoneDifferential);
- valid = true;
- }
- catch (const Poco::SyntaxException& exc)
- {
- LOG_WRN("LastModifiedTime property [" << lastModifiedTime << "] was invalid format: " << exc.displayText() <<
- (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
- }
- if (valid)
- {
- modifiedTime = dateTime.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}));
@@ -657,6 +655,18 @@ 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)
+ {
+ 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);
+ _fileInfo._modifiedTime = iso8601ToTimestamp(lastModifiedTime);
+ }
}
else if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_REQUESTENTITYTOOLARGE)
{
More information about the Libreoffice-commits
mailing list