From e4466b418b62a435948ac6c422dd37fc7427a88d Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Fri, 14 Oct 2016 14:58:19 +0530 Subject: [PATCH] loolwsd: Storage class: std::string -> Poco::URI This saves us from encoding/decoding mess of URIs. Poco::URI is flexible enough and can give encoded or decoded version whenever required, so lets avoid storing the URI in std::string where we have no information about whether its encoded or not. Change-Id: I4a6979e13b20d9f56fc5a0baa630cb1b35ca33b0 --- loolwsd/Storage.cpp | 26 +++++++++++++------------- loolwsd/Storage.hpp | 20 ++++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp index 7cf5693ab..23a924e6f 100644 --- a/loolwsd/Storage.cpp +++ b/loolwsd/Storage.cpp @@ -143,7 +143,7 @@ std::unique_ptr StorageBase::create(const std::string& jailRoot, co Log::info("Public URI [" + uri.toString() + "] is a file."); if (FilesystemEnabled) { - return std::unique_ptr(new LocalStorage(jailRoot, jailPath, uri.getPath())); + return std::unique_ptr(new LocalStorage(jailRoot, jailPath, uri)); } Log::error("Local Storage is disabled by default. Enable in the config file or on the command-line to enable."); @@ -154,7 +154,7 @@ std::unique_ptr StorageBase::create(const std::string& jailRoot, co const auto& targetHost = uri.getHost(); if (WopiHosts.match(targetHost) || isLocalhost(targetHost)) { - return std::unique_ptr(new WopiStorage(jailRoot, jailPath, uri.toString())); + return std::unique_ptr(new WopiStorage(jailRoot, jailPath, uri)); } Log::error("No acceptable WOPI hosts found matching the target host [" + targetHost + "] in config."); @@ -181,15 +181,15 @@ std::string LocalStorage::loadStorageFileToLocal() const auto rootPath = getLocalRootPath(); // /chroot/jailId/user/doc/childId/file.ext - const auto filename = Poco::Path(_uri).getFileName(); + const auto filename = Poco::Path(_uri.getPath()).getFileName(); _jailedFilePath = Poco::Path(rootPath, filename).toString(); - Log::info("Public URI [" + _uri + + Log::info("Public URI [" + _uri.getPath() + "] jailed to [" + _jailedFilePath + "]."); // Despite the talk about URIs it seems that _uri is actually just a pathname here - const auto publicFilePath = _uri; + const auto publicFilePath = _uri.getPath(); if (!Util::checkDiskSpace(publicFilePath)) throw StorageSpaceLowException("Low disk space for " + publicFilePath); @@ -228,13 +228,13 @@ bool LocalStorage::saveLocalFileToStorage() // Copy the file back. if (_isCopy && Poco::File(_jailedFilePath).exists()) { - Log::info("Copying " + _jailedFilePath + " to " + _uri); - Poco::File(_jailedFilePath).copyTo(_uri); + Log::info("Copying " + _jailedFilePath + " to " + _uri.getPath()); + Poco::File(_jailedFilePath).copyTo(_uri.getPath()); } } catch (const Poco::Exception& exc) { - Log::error("copyTo(\"" + _jailedFilePath + "\", \"" + _uri + "\") failed: " + exc.displayText()); + Log::error("copyTo(\"" + _jailedFilePath + "\", \"" + _uri.getPath() + "\") failed: " + exc.displayText()); throw; } @@ -304,9 +304,9 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uri) /// uri format: http://server/<...>/wopi*/files//content std::string WopiStorage::loadStorageFileToLocal() { - Log::info("Downloading URI [" + _uri + "]."); + Log::info("Downloading URI [" + _uri.toString() + "]."); - _fileInfo = getFileInfo(Poco::URI(_uri)); + _fileInfo = getFileInfo(_uri); if (_fileInfo._size == 0 && _fileInfo._filename.empty()) { //TODO: Should throw a more appropriate exception. @@ -329,7 +329,7 @@ std::string WopiStorage::loadStorageFileToLocal() std::istream& rs = psession->receiveResponse(response); auto logger = Log::trace(); - logger << "WOPI::GetFile header for URI [" << _uri << "]:\n"; + logger << "WOPI::GetFile header for URI [" << _uri.toString() << "]:\n"; for (auto& pair : response) { logger << '\t' + pair.first + ": " + pair.second << " / "; @@ -354,7 +354,7 @@ std::string WopiStorage::loadStorageFileToLocal() bool WopiStorage::saveLocalFileToStorage() { - Log::info("Uploading URI [" + _uri + "] from [" + _jailedFilePath + "]."); + Log::info("Uploading URI [" + _uri.toString() + "] from [" + _jailedFilePath + "]."); const auto size = getFileSize(_jailedFilePath); Poco::URI uriObject(_uri); @@ -397,7 +397,7 @@ StorageBase::FileInfo WebDAVStorage::getFileInfo(const Poco::URI& uri) std::string WebDAVStorage::loadStorageFileToLocal() { // TODO: implement webdav GET. - return _uri; + return _uri.toString(); } bool WebDAVStorage::saveLocalFileToStorage() diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp index 23bc7cde0..285b9a940 100644 --- a/loolwsd/Storage.hpp +++ b/loolwsd/Storage.hpp @@ -60,18 +60,18 @@ public: /// jailPath the path within the jail that the child uses. StorageBase(const std::string& localStorePath, const std::string& jailPath, - const std::string& uri) : + const Poco::URI& uri) : _localStorePath(localStorePath), _jailPath(jailPath), _uri(uri), _fileInfo("", Poco::Timestamp(), 0, "", "") { - Log::debug("Storage ctor: " + uri); + Log::debug("Storage ctor: " + uri.toString()); } std::string getLocalRootPath() const; - const std::string& getUri() const { return _uri; } + const std::string getUri() const { return _uri.toString(); } /// Returns information about the file. virtual FileInfo getFileInfo(const Poco::URI& uri) = 0; @@ -100,7 +100,7 @@ public: protected: const std::string _localStorePath; const std::string _jailPath; - const std::string _uri; + const Poco::URI _uri; std::string _jailedFilePath; FileInfo _fileInfo; @@ -116,13 +116,13 @@ class LocalStorage : public StorageBase public: LocalStorage(const std::string& localStorePath, const std::string& jailPath, - const std::string& uri) : + const Poco::URI& uri) : StorageBase(localStorePath, jailPath, uri), _isCopy(false), _localStorageId(LocalStorage::LastLocalStorageId++) { Log::info("LocalStorage ctor with localStorePath: [" + localStorePath + - "], jailPath: [" + jailPath + "], uri: [" + uri + "]."); + "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "]."); } FileInfo getFileInfo(const Poco::URI& uri) override; @@ -145,11 +145,11 @@ class WopiStorage : public StorageBase public: WopiStorage(const std::string& localStorePath, const std::string& jailPath, - const std::string& uri) : + const Poco::URI& uri) : StorageBase(localStorePath, jailPath, uri) { Log::info("WopiStorage ctor with localStorePath: [" + localStorePath + - "], jailPath: [" + jailPath + "], uri: [" + uri + "]."); + "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "]."); } FileInfo getFileInfo(const Poco::URI& uri) override; @@ -166,13 +166,13 @@ class WebDAVStorage : public StorageBase public: WebDAVStorage(const std::string& localStorePath, const std::string& jailPath, - const std::string& uri, + const Poco::URI& uri, std::unique_ptr authAgent) : StorageBase(localStorePath, jailPath, uri), _authAgent(std::move(authAgent)) { Log::info("WebDAVStorage ctor with localStorePath: [" + localStorePath + - "], jailPath: [" + jailPath + "], uri: [" + uri + "]."); + "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "]."); } FileInfo getFileInfo(const Poco::URI& uri) override;