[Libreoffice-commits] online.git: 2 commits - 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
Fri May 19 15:24:29 UTC 2017
loleaflet/dist/errormessages.js | 5 ++--
loleaflet/main.js | 2 -
loleaflet/src/core/Socket.js | 23 +++++++++++++++++---
loleaflet/src/map/handler/Map.WOPI.js | 6 +++--
wsd/DocumentBroker.cpp | 11 ++++++++-
wsd/Exceptions.hpp | 16 +++++++-------
wsd/LOOLWSD.cpp | 14 +++++++++---
wsd/Storage.cpp | 38 +++++++++++++++++++++++-----------
8 files changed, 82 insertions(+), 33 deletions(-)
New commits:
commit bcf958c50056637d4899fc9cd38b87a15dfa57b0
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Thu May 18 23:35:45 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.
Change-Id: I15f0489f36db8689b43d42f6b691fdd21815e4fa
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 03643901..1047c4b7 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -312,13 +312,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 e5924f87..7f8b9bce 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -452,7 +452,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)
@@ -813,6 +816,12 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
throw std::runtime_error(msg);
}
}
+ catch (const StorageConnectionException& exc)
+ {
+ // Alert user about failed load
+ session->sendMessage("error: cmd=storage kind=loadfailed");
+ throw;
+ }
catch (const StorageSpaceLowException&)
{
LOG_ERR("Out of storage while loading document with URI [" << session->getPublicUri().toString() << "].");
diff --git a/wsd/Exceptions.hpp b/wsd/Exceptions.hpp
index a6fbe848..3a3d242f 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
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 1c6c91d6..84d332a7 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1883,7 +1883,7 @@ private:
docBroker->addCallback([docBroker, moveSocket, clientSession, format]()
{
- auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
+ auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
clientSession->setSaveAsSocket(streamSocket);
// Move the socket into DocBroker.
@@ -2125,10 +2125,18 @@ private:
catch (const std::exception& exc)
{
LOG_ERR("Error while handling loading : " << exc.what());
- const std::string msg = "error: cmd=internal kind=unauthorized";
- clientSession->sendMessage(msg);
+ // only send our default error message if we haven't handled the
+ // exception already up the stack
+ if (std::uncaught_exception())
+ {
+ // FIXME: Are we sure we want to say that all other failures due
+ // to an 'unauthorized' WOPI host ?
+ const std::string msg = "error: cmd=internal kind=unauthorized";
+ clientSession->sendMessage(msg);
+ }
docBroker->stop();
}
+
});
});
}
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 12f0a49e..e259836b 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -462,6 +462,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)
@@ -582,15 +588,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)
{
@@ -599,9 +615,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)
commit 2e9e6a753d9721dc6776362d34350380f797dab0
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Fri May 19 11:43:43 2017 +0530
Bin unused Exception class
Change-Id: I961af47f6c112b6d9bb2e9d4ad184e162377506f
diff --git a/wsd/Exceptions.hpp b/wsd/Exceptions.hpp
index 2c4905a6..a6fbe848 100644
--- a/wsd/Exceptions.hpp
+++ b/wsd/Exceptions.hpp
@@ -58,14 +58,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
More information about the Libreoffice-commits
mailing list