[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - loleaflet/dist loleaflet/main.js loleaflet/src wsd/DocumentBroker.cpp wsd/Exceptions.hpp wsd/LOOLWSD.cpp wsd/Storage.cpp
Pranav Kant
pranavk at collabora.co.uk
Tue May 23 09:26:38 UTC 2017
loleaflet/dist/errormessages.js | 5 ++--
loleaflet/main.js | 2 -
loleaflet/src/core/Socket.js | 21 ++++++++++++++++--
loleaflet/src/map/handler/Map.WOPI.js | 6 +++--
wsd/DocumentBroker.cpp | 5 +++-
wsd/Exceptions.hpp | 16 +++++++-------
wsd/LOOLWSD.cpp | 10 ++++++--
wsd/Storage.cpp | 38 +++++++++++++++++++++++-----------
8 files changed, 71 insertions(+), 32 deletions(-)
New commits:
commit 0c609596fbab743de7a8f023c0071d870404c193
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Fri May 19 11:43:43 2017 +0530
wsd: Fail gracefully when storage misbehaves
When WOPI's CheckFileInfo or GetFile responds with status code other
than HTTP 200, show a message to the user indicating some problem in the
storage.
Currently, we open an empty document if storage doesn't return a
document which surely is not correct.
Mention the storage server address when asking user to contact the
server administrator to be more friendly.
Combination of following cherry-picks from master:
(cherry picked from commit 2e9e6a753d9721dc6776362d34350380f797dab0)
(cherry picked from commit bcf958c50056637d4899fc9cd38b87a15dfa57b0)
(cherry picked from commit aed840ea04ce2e7ae120698f50e78268b542db12)
(cherry picked from commit 1ea87b627e077590c6eefa455ce3c8f7d0564068)
Change-Id: I7e2455d3c4da8be2c476460b94812174c34fe529
Reviewed-on: https://gerrit.libreoffice.org/37903
Reviewed-by: Jan Holesovsky <kendy at collabora.com>
Tested-by: Jan Holesovsky <kendy at collabora.com>
diff --git a/loleaflet/dist/errormessages.js b/loleaflet/dist/errormessages.js
index d1eccac1..89f31fc7 100644
--- a/loleaflet/dist/errormessages.js
+++ b/loleaflet/dist/errormessages.js
@@ -10,6 +10,7 @@ exports.sessionexpired = _('Your session has been expired. Further changes to do
exports.faileddocloading = _('Failed to load the document. Please ensure the file type is supported and not corrupted, and try again.');
exports.storage = {
- savediskfull: _('Save failed due to no disk space left on storage server. Document will now be read-only. Please contact the server administrator to continue editing.'),
- savefailed: _('Document cannot be saved to storage. Check your permissions or contact the storage server administrator.')
+ 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.'),
+ savefailed: _('Document cannot be saved to storage. Check your permissions or contact the storage server (%storageserver) administrator.')
};
diff --git a/loleaflet/main.js b/loleaflet/main.js
index fed59570..48200cd9 100644
--- a/loleaflet/main.js
+++ b/loleaflet/main.js
@@ -113,7 +113,7 @@ var map = L.map('map', {
wopi: isWopi,
alwaysActive: alwaysActive,
idleTimeoutSecs: idleTimeoutSecs, // Dim when user is idle.
- outOfFocusTimeoutSecs: outOfFocusTimeoutSecs, // Dim after switching tabs.
+ outOfFocusTimeoutSecs: outOfFocusTimeoutSecs // Dim after switching tabs.
});
// toolbar.js (loaded in <script> tag accesses map as global variable,
// so expose it
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 0e8374d4..25f5f83f 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -313,13 +313,28 @@ L.Socket = L.Class.extend({
return;
}
else if (textMsg.startsWith('error:') && command.errorCmd === 'storage') {
+ var storageError;
if (command.errorKind === 'savediskfull') {
- this._map.fire('error', {msg: errorMessages.storage.savediskfull});
+ storageError = errorMessages.storage.savediskfull;
}
else if (command.errorKind === 'savefailed') {
- // Just warn the user
- this._map.fire('warn', {msg: errorMessages.storage.savefailed});
+ storageError = errorMessages.storage.savefailed;
}
+ else if (command.errorKind === 'loadfailed') {
+ storageError = errorMessages.storage.loadfailed;
+ // Since this is a document load failure, wsd will disconnect the socket anyway,
+ // better we do it first so that another error message doesn't override this one
+ // upon socket close.
+ this._map.hideBusy();
+ this.close();
+ }
+
+ // Parse the storage url as link
+ var tmpLink = document.createElement('a');
+ tmpLink.href = this._map.options.doc;
+ // Insert the storage server address to be more friendly
+ storageError = storageError.replace('%storageserver', tmpLink.host);
+ this._map.fire('warn', {msg: storageError});
return;
}
diff --git a/loleaflet/src/map/handler/Map.WOPI.js b/loleaflet/src/map/handler/Map.WOPI.js
index 110c0adb..20d11a6f 100644
--- a/loleaflet/src/map/handler/Map.WOPI.js
+++ b/loleaflet/src/map/handler/Map.WOPI.js
@@ -4,8 +4,10 @@
/* global title */
L.Map.WOPI = L.Handler.extend({
-
- PostMessageOrigin: false,
+ // If the CheckFileInfo call fails on server side, we won't have any PostMessageOrigin.
+ // So use '*' because we still needs to send 'close' message to the parent frame which
+ // wouldn't be possible otherwise.
+ PostMessageOrigin: '*',
DocumentLoadedTime: false,
HidePrintOption: false,
HideSaveOption: false,
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index b13c75f7..5b15fff1 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -431,7 +431,10 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
std::ostringstream ossWopiInfo;
wopiInfo->stringify(ossWopiInfo);
- session->sendTextFrame("wopi: " + ossWopiInfo.str());
+ // Contains PostMessageOrigin property which is necessary to post messages to parent
+ // frame. Important to send this message immediately and not enqueue it so that in case
+ // document load fails, loleaflet is able to tell its parent frame via PostMessage API.
+ session->sendMessage("wopi: " + ossWopiInfo.str());
// Mark the session as 'Document owner' if WOPI hosts supports it
if (userid == _storage->getFileInfo()._ownerId)
diff --git a/wsd/Exceptions.hpp b/wsd/Exceptions.hpp
index 11c9de59..f0bf82b7 100644
--- a/wsd/Exceptions.hpp
+++ b/wsd/Exceptions.hpp
@@ -34,6 +34,14 @@ public:
using LoolException::LoolException;
};
+/// General exception thrown when we are not able to
+/// connect to storage.
+class StorageConnectionException : public LoolException
+{
+public:
+ using LoolException::LoolException;
+};
+
/// A bad-request exception that is meant to signify,
/// and translate into, an HTTP bad request.
class BadRequestException : public LoolException
@@ -58,14 +66,6 @@ public:
using LoolException::LoolException;
};
-/// An access-denied exception that is meant to signify
-/// a storage authentication error.
-class AccessDeniedException : public LoolException
-{
-public:
- using LoolException::LoolException;
-};
-
/// A service-unavailable exception that is meant to signify
/// an internal error.
class ServiceUnavailableException : public LoolException
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 5f9fd929..8ec8423b 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2116,12 +2116,16 @@ private:
const std::string msg = "error: cmd=internal kind=unauthorized";
clientSession->sendMessage(msg);
}
- catch (const std::exception& exc)
+ catch (const StorageConnectionException& exc)
{
- LOG_ERR("Error while handling loading : " << exc.what());
- const std::string msg = "error: cmd=internal kind=unauthorized";
+ // Alert user about failed load
+ const std::string msg = "error: cmd=storage kind=loadfailed";
clientSession->sendMessage(msg);
}
+ catch (const std::exception& exc)
+ {
+ LOG_ERR("Error while loading : " << exc.what());
+ }
});
});
}
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 3c800535..5b3107c9 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -441,6 +441,12 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const st
LOG_END(logger);
}
+ if (response.getStatus() != Poco::Net::HTTPResponse::HTTP_OK)
+ {
+ LOG_ERR("WOPI::CheckFileInfo failed with " << response.getStatus() << ' ' << response.getReason());
+ throw StorageConnectionException("WOPI::CheckFileInfo failed");
+ }
+
Poco::StreamCopier::copyToString(rs, resMsg);
}
catch(const Poco::Exception& pexc)
@@ -560,15 +566,25 @@ std::string WopiStorage::loadStorageFileToLocal(const std::string& accessToken)
LOG_END(logger);
}
- _jailedFilePath = Poco::Path(getLocalRootPath(), _fileInfo._filename).toString();
- std::ofstream ofs(_jailedFilePath);
- std::copy(std::istreambuf_iterator<char>(rs),
- std::istreambuf_iterator<char>(),
- std::ostreambuf_iterator<char>(ofs));
-
- LOG_INF("WOPI::GetFile downloaded " << getFileSize(_jailedFilePath) << " bytes from [" << uriObject.toString() <<
- "] -> " << _jailedFilePath << " in " << diff.count() << "s : " <<
- response.getStatus() << " " << response.getReason());
+ if (response.getStatus() != Poco::Net::HTTPResponse::HTTP_OK)
+ {
+ LOG_ERR("WOPI::GetFile failed with " << response.getStatus() << ' ' << response.getReason());
+ throw StorageConnectionException("WOPI::GetFile failed");
+ }
+ else // Successful
+ {
+ _jailedFilePath = Poco::Path(getLocalRootPath(), _fileInfo._filename).toString();
+ std::ofstream ofs(_jailedFilePath);
+ std::copy(std::istreambuf_iterator<char>(rs),
+ std::istreambuf_iterator<char>(),
+ std::ostreambuf_iterator<char>(ofs));
+ LOG_INF("WOPI::GetFile downloaded " << getFileSize(_jailedFilePath) << " bytes from [" << uriObject.toString() <<
+ "] -> " << _jailedFilePath << " in " << diff.count() << "s");
+
+ _isLoaded = true;
+ // Now return the jailed path.
+ return Poco::Path(_jailPath, _fileInfo._filename).toString();
+ }
}
catch(const Poco::Exception& pexc)
{
@@ -577,9 +593,7 @@ std::string WopiStorage::loadStorageFileToLocal(const std::string& accessToken)
throw;
}
- _isLoaded = true;
- // Now return the jailed path.
- return Poco::Path(_jailPath, _fileInfo._filename).toString();
+ return "";
}
StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& accessToken)
More information about the Libreoffice-commits
mailing list