[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - common/Util.cpp kit/ForKit.cpp kit/Kit.cpp kit/Kit.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Tue May 16 14:28:57 UTC 2017
common/Util.cpp | 10 ++++++++--
kit/ForKit.cpp | 12 ++++++++----
kit/Kit.cpp | 16 +++++++---------
kit/Kit.hpp | 1 +
wsd/DocumentBroker.cpp | 2 +-
wsd/DocumentBroker.hpp | 8 +++++++-
wsd/LOOLWSD.cpp | 15 +++++++++++++--
7 files changed, 45 insertions(+), 19 deletions(-)
New commits:
commit a7340a54450f571c2b1a1b3e637a08c1b00f452e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun May 7 11:05:34 2017 -0400
wsd: random jail paths instead of pid
Jail paths are now generate from a PRNG
instead of using the PID of the kit process.
The PRN is converted to base-64 and used
as the directory name where a given
kit is jailed.
Change-Id: I8e4bc35d9ccdfdae0e542ab707c417cd29ad52f3
Reviewed-on: https://gerrit.libreoffice.org/37372
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 9798eafb8c71ef9eafedb58478d260573eda2e71)
Reviewed-on: https://gerrit.libreoffice.org/37412
Reviewed-by: Jan Holesovsky <kendy at collabora.com>
Tested-by: Jan Holesovsky <kendy at collabora.com>
diff --git a/common/Util.cpp b/common/Util.cpp
index 3260d130..612f971e 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -99,8 +99,14 @@ namespace Util
std::string getFilename(const size_t length)
{
- std::string s = getB64String(length);
- std::replace(s.begin(), s.end(), '/', '_');
+ std::string s = getB64String(length * 2);
+ s.erase(std::remove_if(s.begin(), s.end(),
+ [](const std::string::value_type& c)
+ {
+ // Remove undesirable characters in a filename.
+ return c == '/' || c == ' ' || c == '+';
+ }),
+ s.end());
return s.substr(0, length);
}
}
diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp
index 035f69a9..cc6759de 100644
--- a/kit/ForKit.cpp
+++ b/kit/ForKit.cpp
@@ -193,6 +193,7 @@ static void cleanupChildren()
std::vector<std::string> jails;
Process::PID exitedChildPid;
int status;
+ // Reap quickly without doing slow cleanup so WSD can spawn more rapidly.
while ((exitedChildPid = waitpid(-1, &status, WUNTRACED | WNOHANG)) > 0)
{
const auto it = childJails.find(exitedChildPid);
@@ -221,7 +222,10 @@ static int createLibreOfficeKit(const std::string& childRoot,
const std::string& loSubPath,
bool queryVersion = false)
{
- LOG_DBG("Forking a loolkit process.");
+ // Generate a jail ID to be used for in the jail path.
+ const std::string jailId = Util::rng::getFilename(16);
+
+ LOG_DBG("Forking a loolkit process with jailId: " << jailId << ".");
const Process::PID pid = fork();
if (!pid)
@@ -248,9 +252,9 @@ static int createLibreOfficeKit(const std::string& childRoot,
}
#ifndef KIT_IN_PROCESS
- lokit_main(childRoot, sysTemplate, loTemplate, loSubPath, NoCapsForKit, queryVersion, DisplayVersion);
+ lokit_main(childRoot, jailId, sysTemplate, loTemplate, loSubPath, NoCapsForKit, queryVersion, DisplayVersion);
#else
- lokit_main(childRoot, sysTemplate, loTemplate, loSubPath, true, queryVersion, DisplayVersion);
+ lokit_main(childRoot, jailId, sysTemplate, loTemplate, loSubPath, true, queryVersion, DisplayVersion);
#endif
}
else
@@ -263,7 +267,7 @@ static int createLibreOfficeKit(const std::string& childRoot,
else
{
LOG_INF("Forked kit [" << pid << "].");
- childJails[pid] = childRoot + std::to_string(pid);
+ childJails[pid] = childRoot + jailId;
}
#ifndef KIT_IN_PROCESS
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 144c3b2c..628e9295 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1577,6 +1577,7 @@ void documentViewCallback(const int type, const char* payload, void* data)
#ifndef BUILDING_TESTS
void lokit_main(const std::string& childRoot,
+ const std::string& jailId,
const std::string& sysTemplate,
const std::string& loTemplate,
const std::string& loSubPath,
@@ -1610,12 +1611,6 @@ void lokit_main(const std::string& childRoot,
assert(!loTemplate.empty());
assert(!loSubPath.empty());
- // Ideally this will be a random ID, but forkit will cleanup
- // our jail directory when we die, and it's simpler to know
- // the jailId (i.e. the path) implicitly by knowing our pid.
- static const std::string pid = std::to_string(Process::id());
- static const std::string jailId = pid;
-
LOG_DBG("Process started.");
std::string userdir_url;
@@ -1772,7 +1767,10 @@ void lokit_main(const std::string& childRoot,
assert(loKit);
LOG_INF("Process is ready.");
- std::string requestUrl = std::string(NEW_CHILD_URI) + "pid=" + pid;
+ static const std::string pid = std::to_string(Process::id());
+
+ std::string requestUrl = NEW_CHILD_URI;
+ requestUrl += "pid=" + pid + "&jailid=" + jailId;
if (queryVersion)
{
char* versionInfo = loKit->getVersionInfo();
@@ -1796,9 +1794,9 @@ void lokit_main(const std::string& childRoot,
auto queue = std::make_shared<TileQueue>();
- const std::string socketName = "child_ws_" + std::to_string(getpid());
+ const std::string socketName = "child_ws_" + pid;
IoUtil::SocketProcessor(ws, socketName,
- [&socketName, &ws, &loKit, &queue](const std::vector<char>& data)
+ [&socketName, &ws, &loKit, &jailId, &queue](const std::vector<char>& data)
{
std::string message(data.data(), data.size());
diff --git a/kit/Kit.hpp b/kit/Kit.hpp
index 6895bbdb..f003e26a 100644
--- a/kit/Kit.hpp
+++ b/kit/Kit.hpp
@@ -10,6 +10,7 @@
#define INCLUDED_LOOLKIT_HPP
void lokit_main(const std::string& childRoot,
+ const std::string& jailId,
const std::string& sysTemplate,
const std::string& loTemplate,
const std::string& loSubPath,
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 5c09854a..bc5db5ed 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -804,7 +804,7 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
try
{
// First load the document, since this can fail.
- if (!load(session, std::to_string(_childProcess->getPid())))
+ if (!load(session, _childProcess->getJailId()))
{
const auto msg = "Failed to load document with URI [" + session->getPublicUri().toString() + "].";
LOG_ERR(msg);
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 5b64d7ed..3f7ebc67 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -59,9 +59,13 @@ class ChildProcess
public:
/// @param pid is the process ID of the child.
/// @param socket is the underlying Sockeet to the child.
- ChildProcess(const Poco::Process::PID pid, const std::shared_ptr<StreamSocket>& socket, const Poco::Net::HTTPRequest &request) :
+ ChildProcess(const Poco::Process::PID pid,
+ const std::string& jailId,
+ const std::shared_ptr<StreamSocket>& socket,
+ const Poco::Net::HTTPRequest &request) :
_pid(pid),
+ _jailId(jailId),
_ws(std::make_shared<WebSocketHandler>(socket, request)),
_socket(socket)
{
@@ -143,6 +147,7 @@ public:
}
Poco::Process::PID getPid() const { return _pid; }
+ const std::string& getJailId() const { return _jailId; }
/// Send a text payload to the child-process WS.
bool sendTextFrame(const std::string& data)
@@ -185,6 +190,7 @@ public:
private:
Poco::Process::PID _pid;
+ const std::string _jailId;
std::shared_ptr<WebSocketHandler> _ws;
std::shared_ptr<Socket> _socket;
std::weak_ptr<DocumentBroker> _docBroker;
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 74d46528..57d3f0d4 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1431,12 +1431,17 @@ private:
// New Child is spawned.
const auto params = Poco::URI(request.getURI()).getQueryParameters();
Poco::Process::PID pid = -1;
+ std::string jailId;
for (const auto& param : params)
{
if (param.first == "pid")
{
pid = std::stoi(param.second);
}
+ else if (param.first == "jailid")
+ {
+ jailId = param.second;
+ }
else if (param.first == "version")
{
LOOLWSD::LOKitVersion = param.second;
@@ -1449,13 +1454,19 @@ private:
return;
}
+ if (jailId.empty())
+ {
+ LOG_ERR("Invalid JailId in child URI [" << request.getURI() << "].");
+ return SocketHandlerInterface::SocketOwnership::UNCHANGED;
+ }
+
in.clear();
- LOG_INF("New child [" << pid << "].");
+ LOG_INF("New child [" << pid << "], jailId: " << jailId << ".");
UnitWSD::get().newChild(*this);
- auto child = std::make_shared<ChildProcess>(pid, socket, request);
+ auto child = std::make_shared<ChildProcess>(pid, jailId, socket, request);
_childProcess = child; // weak
More information about the Libreoffice-commits
mailing list