diff options
author | Pranav Kant <pranavk@collabora.co.uk> | 2016-11-23 17:39:54 +0530 |
---|---|---|
committer | Pranav Kant <pranavk@collabora.co.uk> | 2016-11-23 18:01:21 +0530 |
commit | f8e0b8c11e94810fdde73f8b4d27b80a989670cb (patch) | |
tree | 8511cf563a165dce2ad3b4b6eb043512e6cb6f0c | |
parent | b7065d603de8e8277d485a5d6d8e323de81a5855 (diff) |
tdf#103679: Handle storage diskfull & other PutFile errors
The new behavior is to warn the user when we try to save to
storage and set all the sessions of
the opened document to readonly, if storage server has no disk
space left. In case of WOPI, this is intimated by HTTP response code 413 -
request entity too large.
If save operation to storage failed due to reasons other than
413, just warn the user and let it continue editing the document.
We can add more reasons of failure and act accordingly in future.
Change-Id: I4b046fc38bbc0d752c89d90acb5991a958b76670
-rw-r--r-- | loleaflet/dist/errormessages.js | 5 | ||||
-rw-r--r-- | loleaflet/src/core/Socket.js | 11 | ||||
-rw-r--r-- | loolwsd/DocumentBroker.cpp | 17 | ||||
-rw-r--r-- | loolwsd/PrisonerSession.cpp | 2 | ||||
-rw-r--r-- | loolwsd/Storage.cpp | 23 | ||||
-rw-r--r-- | loolwsd/Storage.hpp | 15 | ||||
-rw-r--r-- | loolwsd/protocol.txt | 11 |
7 files changed, 68 insertions, 16 deletions
diff --git a/loleaflet/dist/errormessages.js b/loleaflet/dist/errormessages.js index 348c6c1a1..036f96559 100644 --- a/loleaflet/dist/errormessages.js +++ b/loleaflet/dist/errormessages.js @@ -4,3 +4,8 @@ exports.limitreached = _('This development build is limited to %0 documents, and exports.serviceunavailable = _('Service is unavailable. Please try again later and report to your administrator if the issue persists.'); exports.unauthorized = _('Unauthorized WOPI host. Please try again later and report to your administrator if the issue persists.'); exports.wrongwopisrc = _('Wrong WOPISrc, usage: WOPISrc=valid encoded URI, or file_path, usage: file_path=/path/to/doc/'); + +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.') +}; diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js index d88961b12..5f720689b 100644 --- a/loleaflet/src/core/Socket.js +++ b/loleaflet/src/core/Socket.js @@ -239,6 +239,17 @@ L.Socket = L.Class.extend({ return; } + else if (textMsg.startsWith('error:') && command.errorCmd === 'storage') { + if (command.errorKind === 'savediskfull') { + // Just warn the user + this._map.fire('error', {msg: errorMessages.storage.savediskfull}); + } + else if (command.errorKind === 'savefailed') { + this._map.fire('error', {msg: errorMessages.storage.savefailed}); + } + + return; + } else if (textMsg.startsWith('error:') && command.errorCmd === 'internal') { this._map._fatal = true; if (command.errorKind === 'diskfull') { diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp index 14bbd4bfe..fcd4d7a15 100644 --- a/loolwsd/DocumentBroker.cpp +++ b/loolwsd/DocumentBroker.cpp @@ -366,7 +366,8 @@ bool DocumentBroker::save(const std::string& sessionId, bool success, const std: LOG_DBG("Saving to URI [" << uri << "]."); assert(_storage && _tileCache); - if (_storage->saveLocalFileToStorage(uriPublic)) + StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(uriPublic); + if (storageSaveResult == StorageBase::SaveResult::OK) { _isModified = false; _tileCache->setUnsavedChanges(false); @@ -377,6 +378,20 @@ bool DocumentBroker::save(const std::string& sessionId, bool success, const std: _saveCV.notify_all(); return true; } + else if (storageSaveResult == StorageBase::SaveResult::DISKFULL) + { + // Make everyone readonly and tell everyone that storage is low on diskspace + for (auto& sessionIt : _sessions) + { + sessionIt.second->setReadOnly(); + sessionIt.second->sendTextFrame("perm: readonly"); + sessionIt.second->sendTextFrame("error: cmd=storage kind=savediskfull"); + } + } + else if (storageSaveResult == StorageBase::SaveResult::FAILED) + { + it->second->sendTextFrame("error: cmd=storage kind=savefailed"); + } LOG_ERR("Failed to save to URI [" << uri << "]."); return false; diff --git a/loolwsd/PrisonerSession.cpp b/loolwsd/PrisonerSession.cpp index bb352cc23..28e4ac673 100644 --- a/loolwsd/PrisonerSession.cpp +++ b/loolwsd/PrisonerSession.cpp @@ -87,7 +87,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length) } if (!_docBroker->save(getId(), success, result)) - peer->sendTextFrame("error: cmd=internal kind=diskfull"); + LOG_ERR("Saving document to storage failed."); return true; } } diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp index 51abe6935..21800aecb 100644 --- a/loolwsd/Storage.cpp +++ b/loolwsd/Storage.cpp @@ -251,7 +251,7 @@ std::string LocalStorage::loadStorageFileToLocal() return Poco::Path(_jailPath, filename).toString(); } -bool LocalStorage::saveLocalFileToStorage(const Poco::URI& uriPublic) +StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Poco::URI& uriPublic) { try { @@ -269,7 +269,7 @@ bool LocalStorage::saveLocalFileToStorage(const Poco::URI& uriPublic) throw; } - return true; + return StorageBase::SaveResult::OK; } namespace { @@ -472,7 +472,7 @@ std::string WopiStorage::loadStorageFileToLocal() return Poco::Path(_jailPath, _fileInfo._filename).toString(); } -bool WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic) +StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic) { LOG_INF("Uploading URI [" << uriPublic.toString() << "] from [" << _jailedFilePath + "]."); // TODO: Check if this URI has write permission (canWrite = true) @@ -499,12 +499,21 @@ bool WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic) Poco::StreamCopier::copyStream(rs, oss); LOG_INF("WOPI::PutFile response: " << oss.str()); - const auto success = (response.getStatus() == Poco::Net::HTTPResponse::HTTP_OK); LOG_INF("WOPI::PutFile uploaded " << size << " bytes from [" << _jailedFilePath << "] -> [" << uriObject.toString() << "]: " << response.getStatus() << " " << response.getReason()); - return success; + StorageBase::SaveResult saveResult = StorageBase::SaveResult::FAILED; + if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_OK) + { + saveResult = StorageBase::SaveResult::OK; + } + else if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_REQUESTENTITYTOOLARGE) + { + saveResult = StorageBase::SaveResult::DISKFULL; + } + + return saveResult; } std::string WebDAVStorage::loadStorageFileToLocal() @@ -514,10 +523,10 @@ std::string WebDAVStorage::loadStorageFileToLocal() return _uri.toString(); } -bool WebDAVStorage::saveLocalFileToStorage(const Poco::URI& /*uriPublic*/) +StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const Poco::URI& /*uriPublic*/) { // TODO: implement webdav PUT. - return false; + return StorageBase::SaveResult::OK; } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp index d116eec9b..684488f4a 100644 --- a/loolwsd/Storage.hpp +++ b/loolwsd/Storage.hpp @@ -52,6 +52,13 @@ public: size_t _size; }; + enum SaveResult + { + OK, + DISKFULL, + FAILED + }; + /// localStorePath the absolute root path of the chroot. /// jailPath the path within the jail that the child uses. StorageBase(const Poco::URI& uri, @@ -80,7 +87,7 @@ public: virtual std::string loadStorageFileToLocal() = 0; /// Writes the contents of the file back to the source. - virtual bool saveLocalFileToStorage(const Poco::URI& uriPublic) = 0; + virtual SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) = 0; static size_t getFileSize(const std::string& filename); @@ -140,7 +147,7 @@ public: std::string loadStorageFileToLocal() override; - bool saveLocalFileToStorage(const Poco::URI& uriPublic) override; + SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override; private: /// True if the jailed file is not linked but copied. @@ -214,7 +221,7 @@ public: /// uri format: http://server/<...>/wopi*/files/<id>/content std::string loadStorageFileToLocal() override; - bool saveLocalFileToStorage(const Poco::URI& uriPublic) override; + SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override; /// Total time taken for making WOPI calls during load std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; } @@ -244,7 +251,7 @@ public: std::string loadStorageFileToLocal() override; - bool saveLocalFileToStorage(const Poco::URI& uriPublic) override; + SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override; private: std::unique_ptr<AuthBase> _authAgent; diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt index c316b32d1..5ac2e0ed9 100644 --- a/loolwsd/protocol.txt +++ b/loolwsd/protocol.txt @@ -220,9 +220,14 @@ error: cmd=<command> kind=<kind> [code=<error_code>] [params=1,2,3,...,N] <freeErrorText> <command> is the command part of the corresponding client->server - message that caused the error. <command> can also be 'internal' - for errors generated without directly corresponding to a client - message. <kind> is some single-word classification + message that caused the error. + + <command> can also take following values: + 'internal': for errors generated without directly corresponding to a client + message. + 'storage': for errors pertaining to storage (filesysytem, wopi etc.) + + <kind> is some single-word classification <code> (when provided) further specifies the error as forwarded from LibreOffice |