[Libreoffice-commits] online.git: wsd/ClientSession.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/Storage.cpp wsd/Storage.hpp
Jan Holesovsky
kendy at collabora.com
Fri May 19 08:37:11 UTC 2017
wsd/ClientSession.cpp | 1
wsd/DocumentBroker.cpp | 19 ++++++++---------
wsd/DocumentBroker.hpp | 1
wsd/Storage.cpp | 46 ++++++-------------------------------------
wsd/Storage.hpp | 52 +++----------------------------------------------
5 files changed, 21 insertions(+), 98 deletions(-)
New commits:
commit 9db41725f423f00f95db4b68d70827d24f80bb6c
Author: Jan Holesovsky <kendy at collabora.com>
Date: Fri May 19 10:32:24 2017 +0200
Revert "wsd: use WOPI host instance ID to support hostname aliases"
Turns out this introduces two calls to the CheckFileInfo which is not really
what we should be doing; instead, let's do a kind of cannonicalization in the
WOPI host directly.
This reverts commit ec2fd0844f997f9a7347229e282daeb8dc4471bc.
Change-Id: I311bf8a45b706ed9a4d8cd00db0a990ac6d461b4
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 479bc2b9..105b30d3 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -737,6 +737,7 @@ bool ClientSession::forwardToClient(const std::shared_ptr<Message>& payload)
std::string ClientSession::getAccessToken() const
{
+ std::string accessToken;
Poco::URI::QueryParameters queryParams = _uriPublic.getQueryParameters();
for (auto& param: queryParams)
{
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 66a5de80..e5924f87 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -102,7 +102,15 @@ Poco::URI DocumentBroker::sanitizeURI(const std::string& uri)
std::string DocumentBroker::getDocKey(const Poco::URI& uri)
{
- return StorageBase::getUniqueDocId(uri);
+ // If multiple host-names are used to access us, then
+ // we force same document (when opened from
+ // alias hosts) to load as separate documents and sharing doesn't
+ // work. Worse, saving overwrites one another.
+ // But we also do not want different WOPI hosts using the same path
+ // for some file getting shared across WOPI hosts
+ std::string docKey;
+ Poco::URI::encode(uri.getHost() + ":" + std::to_string(uri.getPort()) + uri.getPath(), "", docKey);
+ return docKey;
}
/// The Document Broker Poll - one of these in a thread per document
@@ -401,7 +409,6 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
LOG_ERR("Failed to create Storage instance for [" << _docKey << "] in " << jailPath.toString());
return false;
}
-
firstInstance = true;
}
@@ -416,14 +423,6 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
std::unique_ptr<WopiStorage::WOPIFileInfo> wopifileinfo = wopiStorage->getWOPIFileInfo(session->getAccessToken());
userid = wopifileinfo->_userid;
username = wopifileinfo->_username;
- if (firstInstance)
- {
- _hostInstanceId = wopifileinfo->_hostInstanceId;
- }
- else if (!_hostInstanceId.empty() && _hostInstanceId != wopifileinfo->_hostInstanceId)
- {
- throw UnauthorizedRequestException("Unauthorized WOPI host.");
- }
if (!wopifileinfo->_userCanWrite)
{
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 8ac67ee2..46481cba 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -369,7 +369,6 @@ private:
Poco::URI _uriJailed;
std::string _jailId;
std::string _filename;
- std::string _hostInstanceId;
/// The last time we tried saving, regardless of whether the
/// document was modified and saved or not.
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 0c31e53c..12f0a49e 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -125,7 +125,7 @@ void StorageBase::initialize()
#endif
}
-bool StorageBase::isLocalhost(const std::string& targetHost)
+bool isLocalhost(const std::string& targetHost)
{
std::string targetAddress;
try
@@ -202,7 +202,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std
{
LOG_INF("Public URI [" << uri.toString() << "] considered WOPI.");
const auto& targetHost = uri.getHost();
- if (isWopiHostAuthorized(targetHost))
+ if (WopiHosts.match(targetHost) || isLocalhost(targetHost))
{
return std::unique_ptr<StorageBase>(new WopiStorage(uri, jailRoot, jailPath));
}
@@ -213,39 +213,6 @@ std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std
throw BadRequestException("No Storage configured or invalid URI.");
}
-std::string StorageBase::getUniqueDocId(const Poco::URI& uri)
-{
- std::string docId;
- if (uri.isRelative() || uri.getScheme() == "file")
- {
- Poco::URI::encode(uri.getPath(), "", docId);
- }
- else if (WopiEnabled)
- {
- const auto& targetHost = uri.getHost();
- if (isWopiHostAuthorized(targetHost))
- {
- std::string accessToken;
- Poco::URI::QueryParameters queryParams = uri.getQueryParameters();
- for (auto& param: queryParams)
- {
- if (param.first == "access_token")
- accessToken = param.second;
- }
-
- const std::unique_ptr<WopiStorage::WOPIFileInfo> info = WopiStorage::getWOPIFileInfo(uri, accessToken);
- const std::string prefix = !info->_hostInstanceId.empty()
- ? info->_hostInstanceId
- : (uri.getHost() + ':' + std::to_string(uri.getPort()));
- Poco::URI::encode(prefix + uri.getPath(), "", docId);
- }
- else
- throw UnauthorizedRequestException("No acceptable WOPI hosts found matching the target host [" + targetHost + "] in config.");
- }
-
- return docId;
-}
-
std::atomic<unsigned> LocalStorage::LastLocalStorageId;
std::unique_ptr<LocalStorage::LocalFileInfo> LocalStorage::getLocalFileInfo()
@@ -459,9 +426,10 @@ void addStorageDebugCookie(Poco::Net::HTTPRequest& request)
} // anonymous namespace
-std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(Poco::URI uriObject, const std::string& accessToken)
+std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const std::string& accessToken)
{
// update the access_token to the one matching to the session
+ Poco::URI uriObject(_uri);
setQueryParameter(uriObject, "access_token", accessToken);
LOG_DBG("Getting info for wopi uri [" << uriObject.toString() << "].");
@@ -509,7 +477,6 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(Poco::UR
std::string ownerId;
std::string userId;
std::string userName;
- std::string hostInstanceId;
bool canWrite = false;
bool enableOwnerTermination = false;
std::string postMessageOrigin;
@@ -544,7 +511,6 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(Poco::UR
getWOPIValue(object, "DisableExport", disableExport);
getWOPIValue(object, "DisableCopy", disableCopy);
getWOPIValue(object, "LastModifiedTime", lastModifiedTime);
- getWOPIValue(object, "HostInstanceId", hostInstanceId);
}
else
{
@@ -574,7 +540,9 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(Poco::UR
}
}
- return std::unique_ptr<WopiStorage::WOPIFileInfo>(new WOPIFileInfo(filename, ownerId, modifiedTime, size, userId, userName, hostInstanceId, canWrite, postMessageOrigin, hidePrintOption, hideSaveOption, hideExportOption, enableOwnerTermination, disablePrint, disableExport, disableCopy, callDuration));
+ _fileInfo = FileInfo({filename, ownerId, modifiedTime, size});
+
+ return std::unique_ptr<WopiStorage::WOPIFileInfo>(new WOPIFileInfo({userId, userName, canWrite, postMessageOrigin, hidePrintOption, hideSaveOption, hideExportOption, enableOwnerTermination, disablePrint, disableExport, disableCopy, callDuration}));
}
/// uri format: http://server/<...>/wopi*/files/<id>/content
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index 7cb77432..6613fbbc 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -100,29 +100,11 @@ public:
static std::unique_ptr<StorageBase> create(const Poco::URI& uri,
const std::string& jailRoot,
const std::string& jailPath);
-
- /// Given the URI of a doc, return a unique doc ID.
- /// Wopi host aliases are resolved to unique host ID.
- static std::string getUniqueDocId(const Poco::URI& uri);
-
protected:
/// Returns the root path of the jail directory of docs.
std::string getLocalRootPath() const;
- /// Returns true iff WOPI is enabled, and the host is whitelisted (or local).
- static bool isWopiHostAuthorized(const std::string& host)
- {
- if (WopiEnabled)
- {
- return (WopiHosts.match(host) || isLocalhost(host));
- }
-
- return false;
- }
-
- static bool isLocalhost(const std::string& host);
-
protected:
const Poco::URI _uri;
std::string _localStorePath;
@@ -194,16 +176,11 @@ public:
"], jailPath: [" << jailPath << "], uri: [" << uri.toString() << "].");
}
- class WOPIFileInfo : public FileInfo
+ class WOPIFileInfo
{
public:
- WOPIFileInfo(const std::string& filename,
- const std::string& ownerId,
- const Poco::Timestamp& modifiedTime,
- size_t size,
- const std::string& userid,
+ WOPIFileInfo(const std::string& userid,
const std::string& username,
- const std::string& hostInstanceId,
const bool userCanWrite,
const std::string& postMessageOrigin,
const bool hidePrintOption,
@@ -214,10 +191,8 @@ public:
const bool disableExport,
const bool disableCopy,
const std::chrono::duration<double> callDuration)
- : FileInfo(filename, ownerId, modifiedTime, size),
- _userid(userid),
+ : _userid(userid),
_username(username),
- _hostInstanceId(hostInstanceId),
_userCanWrite(userCanWrite),
_postMessageOrigin(postMessageOrigin),
_hidePrintOption(hidePrintOption),
@@ -235,8 +210,6 @@ public:
std::string _userid;
/// Display Name of user accessing the file
std::string _username;
- /// Host instance ID (unique to the given host).
- std::string _hostInstanceId;
/// If user accessing the file has write permission
bool _userCanWrite;
/// WOPI Post message property
@@ -263,12 +236,7 @@ public:
/// provided during the initial creation of the WOPI storage.
/// Also extracts the basic file information from the response
/// which can then be obtained using getFileInfo()
- std::unique_ptr<WOPIFileInfo> getWOPIFileInfo(const std::string& accessToken)
- {
- std::unique_ptr<WOPIFileInfo> info = getWOPIFileInfo(_uri, accessToken);
- _fileInfo = FileInfo(info->_filename, info->_ownerId, info->_modifiedTime, info->_size);
- return info;
- }
+ std::unique_ptr<WOPIFileInfo> getWOPIFileInfo(const std::string& accessToken);
/// uri format: http://server/<...>/wopi*/files/<id>/content
std::string loadStorageFileToLocal(const std::string& accessToken) override;
@@ -278,11 +246,6 @@ public:
/// Total time taken for making WOPI calls during load
std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; }
- /// Given the URI of a doc, return a unique doc ID.
- static std::string getUniqueDocId(const Poco::URI& uri);
-
- static std::unique_ptr<WOPIFileInfo> getWOPIFileInfo(Poco::URI uriObject, const std::string& accessToken);
-
private:
// Time spend in loading the file from storage
std::chrono::duration<double> _wopiLoadDuration;
@@ -310,13 +273,6 @@ public:
SaveResult saveLocalFileToStorage(const std::string& accessToken) override;
- /// Given the URI of a doc, return a unique doc ID.
- static std::string getUniqueDocId(const std::string& uri)
- {
- // TODO: Implement.
- return uri;
- }
-
private:
std::unique_ptr<AuthBase> _authAgent;
};
More information about the Libreoffice-commits
mailing list