[Libreoffice-commits] online.git: Branch 'libreoffice-7-0' - kit/ChildSession.cpp loleaflet/src test/UnitSession.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp
Szymon KÅ‚os (via logerrit)
logerrit at kemper.freedesktop.org
Tue Sep 8 10:33:08 UTC 2020
kit/ChildSession.cpp | 10 +++++++-
loleaflet/src/core/Socket.js | 3 ++
loleaflet/src/layer/tile/TileLayer.js | 2 -
test/UnitSession.cpp | 16 +++++---------
wsd/DocumentBroker.cpp | 27 ++++++++++++++++++++++++
wsd/DocumentBroker.hpp | 9 ++++++++
wsd/LOOLWSD.cpp | 38 ++++++++++++++++------------------
7 files changed, 72 insertions(+), 33 deletions(-)
New commits:
commit da0be0fdece89af60aad6869bc8f74365abec3cb
Author: Szymon Kłos <szymon.klos at collabora.com>
AuthorDate: Thu Sep 3 11:34:13 2020 +0200
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Tue Sep 8 12:32:48 2020 +0200
Simplify download process
Use hash to identify download and pass that to the client.
This allows us to reduce parameters for download requests.
DocBroker maps download ids to URL in the file system.
Change-Id: I254d4f0ccaf3cff9f038a817c8162510ae228bc5
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101992
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Tested-by: Michael Meeks <michael.meeks at collabora.com>
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/102184
Tested-by: Andras Timar <andras.timar at collabora.com>
Reviewed-by: Andras Timar <andras.timar at collabora.com>
diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp
index 6ab9b6797..b3b7b43e7 100644
--- a/kit/ChildSession.cpp
+++ b/kit/ChildSession.cpp
@@ -952,7 +952,8 @@ bool ChildSession::downloadAs(const char* /*buffer*/, int /*length*/, const Stri
// The file is removed upon downloading.
const std::string tmpDir = FileUtil::createRandomDir(jailDoc);
- const std::string url = jailDoc + tmpDir + "/" + filenameParam.getFileName();
+ const std::string urlToSend = tmpDir + "/" + filenameParam.getFileName();
+ const std::string url = jailDoc + urlToSend;
const std::string urlAnonym = jailDoc + tmpDir + "/" + Poco::Path(nameAnonym).getFileName();
LOG_DBG("Calling LOK's saveAs with: url='" << urlAnonym << "', format='" <<
@@ -963,7 +964,12 @@ bool ChildSession::downloadAs(const char* /*buffer*/, int /*length*/, const Stri
format.empty() ? nullptr : format.c_str(),
filterOptions.empty() ? nullptr : filterOptions.c_str());
- sendTextFrame("downloadas: jail=" + _jailId + " dir=" + tmpDir + " name=" + name +
+ // Register download id -> URL mapping in the DocumentBroker
+ std::string docBrokerMessage = "registerdownload: downloadid=" + tmpDir + " url=" + urlToSend;
+ _docManager->sendFrame(docBrokerMessage.c_str(), docBrokerMessage.length());
+
+ // Send download id to the client
+ sendTextFrame("downloadas: downloadid=" + tmpDir +
" port=" + std::to_string(ClientPortNumber) + " id=" + id);
#endif
return true;
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 80decbee2..b5757c717 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -1106,6 +1106,9 @@ L.Socket = L.Class.extend({
else if (tokens[i].substring(0, 4) === 'dir=') {
command.dir = tokens[i].substring(4);
}
+ else if (tokens[i].substring(0, 11) === 'downloadid=') {
+ command.downloadid = tokens[i].substring(11);
+ }
else if (tokens[i].substring(0, 5) === 'name=') {
command.name = tokens[i].substring(5);
}
diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index ded1b572c..3e2223e88 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -909,7 +909,7 @@ L.TileLayer = L.GridLayer.extend({
wopiSrc = '?WOPISrc=' + this._map.options.wopiSrc;
}
var url = this._map.options.webserver + this._map.options.serviceRoot + '/' + this._map.options.urlPrefix + '/' +
- encodeURIComponent(this._map.options.doc) + '/' + command.jail + '/' + command.dir + '/' + command.name + wopiSrc;
+ encodeURIComponent(this._map.options.doc) + '/download/' + command.downloadid + wopiSrc;
this._map.hideBusy();
if (this._map['wopi'].DownloadAsPostMessage) {
diff --git a/test/UnitSession.cpp b/test/UnitSession.cpp
index f8b859fa1..4f872f982 100644
--- a/test/UnitSession.cpp
+++ b/test/UnitSession.cpp
@@ -185,22 +185,18 @@ UnitBase::TestResult UnitSession::testSlideShow()
!response.empty());
StringVector tokens(LOOLProtocol::tokenize(response.substr(11), ' '));
- // "downloadas: jail= dir= name=slideshow.svg port= id=slideshow"
- const std::string jail = tokens[0].substr(std::string("jail=").size());
- const std::string dir = tokens[1].substr(std::string("dir=").size());
- const std::string name = tokens[2].substr(std::string("name=").size());
- const int port = std::stoi(tokens[3].substr(std::string("port=").size()));
- const std::string id = tokens[4].substr(std::string("id=").size());
- LOK_ASSERT(!jail.empty());
- LOK_ASSERT(!dir.empty());
- LOK_ASSERT_EQUAL(std::string("slideshow.svg"), name);
+ // "downloadas: downloadId= port= id=slideshow"
+ const std::string downloadId = tokens[0].substr(std::string("downloadId=").size());
+ const int port = std::stoi(tokens[1].substr(std::string("port=").size()));
+ const std::string id = tokens[2].substr(std::string("id=").size());
+ LOK_ASSERT(!downloadId.empty());
LOK_ASSERT_EQUAL(static_cast<int>(Poco::URI(helpers::getTestServerURI()).getPort()),
port);
LOK_ASSERT_EQUAL(std::string("slideshow"), id);
std::string encodedDoc;
Poco::URI::encode(documentPath, ":/?", encodedDoc);
- const std::string path = "/lool/" + encodedDoc + "/" + jail + "/" + dir + "/" + name;
+ const std::string path = "/lool/" + encodedDoc + "/download/" + downloadId;
std::unique_ptr<Poco::Net::HTTPClientSession> session(
helpers::createSession(Poco::URI(helpers::getTestServerURI())));
Poco::Net::HTTPRequest requestSVG(Poco::Net::HTTPRequest::HTTP_GET, path);
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 717adec15..98f18c30a 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1581,6 +1581,22 @@ void DocumentBroker::alertAllUsers(const std::string& msg)
}
}
+std::string DocumentBroker::getDownloadURL(const std::string& downloadId)
+{
+ auto aFound = _registeredDownloadLinks.find(downloadId);
+ if (aFound != _registeredDownloadLinks.end())
+ return aFound->second;
+
+ return "";
+}
+
+void DocumentBroker::unregisterDownloadId(const std::string& downloadId)
+{
+ auto aFound = _registeredDownloadLinks.find(downloadId);
+ if (aFound != _registeredDownloadLinks.end())
+ _registeredDownloadLinks.erase(aFound);
+}
+
/// Handles input from the prisoner / child kit process
bool DocumentBroker::handleInput(const std::vector<char>& payload)
{
@@ -1617,6 +1633,17 @@ bool DocumentBroker::handleInput(const std::vector<char>& payload)
LOG_CHECK_RET(kind != "", false);
Util::alertAllUsers(cmd, kind);
}
+ else if (command == "registerdownload:")
+ {
+ LOG_CHECK_RET(message->tokens().size() == 3, false);
+ std::string downloadid, url;
+ LOOLProtocol::getTokenString((*message)[1], "downloadid", downloadid);
+ LOG_CHECK_RET(downloadid != "", false);
+ LOOLProtocol::getTokenString((*message)[2], "url", url);
+ LOG_CHECK_RET(url != "", false);
+
+ _registeredDownloadLinks[downloadid] = url;
+ }
else
{
LOG_ERR("Unexpected message: [" << msg << "].");
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 17300d0cb..8f79c52dc 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -293,6 +293,12 @@ public:
/// Estimate memory usage / bytes
size_t getMemorySize() const;
+ /// Get URL for corresponding download id if registered, or empty string otherwise
+ std::string getDownloadURL(const std::string& downloadId);
+
+ /// Remove download id mapping
+ void unregisterDownloadId(const std::string& downloadId);
+
private:
/// get the session id of a session that can write the document for save / locking.
std::string getWriteableSessionId() const;
@@ -439,6 +445,9 @@ private:
/// Unique DocBroker ID for tracing and debugging.
static std::atomic<unsigned> DocBrokerId;
+
+ // Maps download id -> URL
+ std::map<std::string, std::string> _registeredDownloadLinks;
};
#if !MOBILEAPP
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 17c2484fd..b1a7da739 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2855,7 +2855,7 @@ private:
}
}
}
- else if (requestDetails.size() >= 5)
+ else if (requestDetails.equals(2, "download"))
{
LOG_INF("File download request.");
// TODO: Check that the user in question has access to this file!
@@ -2870,29 +2870,23 @@ private:
throw BadRequestException("DocKey [" + docKey + "] is invalid.");
}
- // 2. Cross-check if received child id is correct
- if (docBrokerIt->second->getJailId() != requestDetails[2])
- {
- throw BadRequestException("ChildId does not correspond to docKey");
- }
+ std::string downloadId = requestDetails[3];
+ std::string url = docBrokerIt->second->getDownloadURL(downloadId);
+ docBrokerIt->second->unregisterDownloadId(downloadId);
+ std::string jailId = docBrokerIt->second->getJailId();
- // 3. Don't let user download the file in main doc directory containing
- // the document being edited otherwise we will end up deleting main directory
- // after download finishes
- if (docBrokerIt->second->getJailId() == requestDetails[3])
- {
- throw BadRequestException("RandomDir cannot be equal to ChildId");
- }
docBrokersLock.unlock();
- std::string fileName;
- URI::decode(requestDetails[4], fileName);
- const Path filePath(LOOLWSD::ChildRoot + requestDetails[2]
- + JAILED_DOCUMENT_ROOT + requestDetails[3] + "/" + fileName);
+ bool foundDownloadId = !url.empty();
+
+ const Path filePath(LOOLWSD::ChildRoot + jailId + JAILED_DOCUMENT_ROOT + url);
const std::string filePathAnonym = LOOLWSD::anonymizeUrl(filePath.toString());
- LOG_INF("HTTP request for: " << filePathAnonym);
- if (filePath.isAbsolute() && File(filePath).exists())
+
+ if (foundDownloadId && filePath.isAbsolute() && File(filePath).exists())
{
+ LOG_INF("HTTP request for: " << filePathAnonym);
+
+ std::string fileName = filePath.getFileName();
const Poco::URI postRequestUri(request.getURI());
const Poco::URI::QueryParameters postRequestQueryParams = postRequestUri.getQueryParameters();
@@ -2926,7 +2920,11 @@ private:
}
else
{
- LOG_ERR("Download file [" << filePathAnonym << "] not found.");
+ if (foundDownloadId)
+ LOG_ERR("Download file [" << filePathAnonym << "] not found.");
+ else
+ LOG_ERR("Download with id [" << downloadId << "] not found.");
+
std::ostringstream oss;
oss << "HTTP/1.1 404 Not Found\r\n"
<< "Date: " << Util::getHttpTimeNow() << "\r\n"
More information about the Libreoffice-commits
mailing list