[Libreoffice-commits] online.git: common/Unit.hpp kit/Kit.cpp loolwsd.xml.in test/Makefile.am test/UnitConvert.cpp test/UnitFuzz.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Thu Nov 8 01:28:10 UTC 2018
common/Unit.hpp | 3 -
kit/Kit.cpp | 7 --
loolwsd.xml.in | 1
test/Makefile.am | 4 +
test/UnitConvert.cpp | 123 +++++++++++++++++++++++++++++++++++++++++++++++++
test/UnitFuzz.cpp | 3 -
wsd/DocumentBroker.cpp | 24 +++++++--
wsd/DocumentBroker.hpp | 3 -
wsd/LOOLWSD.cpp | 7 +-
9 files changed, 158 insertions(+), 17 deletions(-)
New commits:
commit fa74404019044f240686226f1c6dfefb27356c98
Author: Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Wed Nov 7 23:00:32 2018 +0000
Commit: Michael Meeks <michael.meeks at collabora.com>
CommitDate: Thu Nov 8 01:27:40 2018 +0000
Add a time limit for badly behaved / huge document load / conversions.
Also improve debug printing of load times in dumpstate.
Change-Id: Ib3fd70dffb57588cd90bd928c4be9890cee8bc65
diff --git a/common/Unit.hpp b/common/Unit.hpp
index 5844cc3bd..4d018b80b 100644
--- a/common/Unit.hpp
+++ b/common/Unit.hpp
@@ -275,8 +275,7 @@ public:
virtual void postFork() {}
/// Kit got a message
- virtual bool filterKitMessage(const std::shared_ptr<LOOLWebSocket>& /* ws */,
- std::string& /* message */)
+ virtual bool filterKitMessage(WebSocketHandler *, std::string &/* message */ )
{
return false;
}
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 8eda6ce10..2c21155d7 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -2109,12 +2109,8 @@ protected:
{
std::string message(data.data(), data.size());
-#if 0 // FIXME might be needed for unit tests #ifndef KIT_IN_PROCESS
- if (UnitKit::get().filterKitMessage(ws, message))
- {
+ if (UnitKit::get().filterKitMessage(this, message))
return;
- }
-#endif
std::vector<std::string> tokens = LOOLProtocol::tokenize(message);
Log::StreamLogger logger = Log::debug();
@@ -2295,6 +2291,7 @@ void lokit_main(
jailPath = Path::forDirectory(childRoot + "/" + jailId);
LOG_INF("Jail path: " << jailPath.toString());
File(jailPath).createDirectories();
+ chmod(jailPath.toString().c_str(), S_IXUSR | S_IWUSR | S_IRUSR);
if (bRunInsideJail)
{
diff --git a/loolwsd.xml.in b/loolwsd.xml.in
index 29ab2ef8f..54cda7e63 100644
--- a/loolwsd.xml.in
+++ b/loolwsd.xml.in
@@ -26,6 +26,7 @@
<limit_stack_mem_kb desc="The maximum stack size allowed to each document process. 0 for unlimited." type="uint">8000</limit_stack_mem_kb>
<limit_file_size_mb desc="The maximum file size allowed to each document process to write. 0 for unlimited." type="uint">0</limit_file_size_mb>
<limit_num_open_files desc="The maximum number of files allowed to each document process to open. 0 for unlimited." type="uint">0</limit_num_open_files>
+ <limit_load_secs desc="Maximum number of seconds to wait for a document load to succeed. 0 for unlimited." type="uint" default="100">100</limit_load_secs>
</per_document>
<per_view desc="View-specific settings.">
diff --git a/test/Makefile.am b/test/Makefile.am
index 9b7d8def3..62b859bcc 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -13,6 +13,7 @@ AM_CXXFLAGS = $(CPPUNIT_CFLAGS) -DTDOC=\"$(top_srcdir)/test/data\" \
-I${top_srcdir}/common -I${top_srcdir}/net -I${top_srcdir}/wsd -I${top_srcdir}/kit
noinst_LTLIBRARIES = \
+ unit-convert.la \
unit-timeout.la unit-prefork.la \
unit-storage.la unit-client.la \
unit-admin.la unit-tilecache.la \
@@ -87,6 +88,7 @@ unit_admin_la_SOURCES = UnitAdmin.cpp
unit_admin_la_LIBADD = $(CPPUNIT_LIBS)
unit_client_la_SOURCES = UnitClient.cpp ${test_all_source}
unit_client_la_LIBADD = $(CPPUNIT_LIBS)
+unit_convert_la_SOURCES = UnitConvert.cpp
unit_timeout_la_SOURCES = UnitTimeout.cpp
unit_prefork_la_SOURCES = UnitPrefork.cpp
unit_storage_la_SOURCES = UnitStorage.cpp
@@ -117,7 +119,7 @@ check-local:
./run_unit.sh --log-file test.log --trs-file test.trs
# FIXME 2: unit-oob.la fails with symbol undefined:
# UnitWSD::testHandleRequest(UnitWSD::TestRequest, UnitHTTPServerRequest&, UnitHTTPServerResponse&) ,
-TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la \
+TESTS = unit-convert.la unit-prefork.la unit-tilecache.la unit-timeout.la \
unit-oauth.la unit-wopi.la unit-wopi-saveas.la \
unit-wopi-ownertermination.la unit-wopi-versionrestore.la \
unit-wopi-documentconflict.la
diff --git a/test/UnitConvert.cpp b/test/UnitConvert.cpp
new file mode 100644
index 000000000..2b30b00a8
--- /dev/null
+++ b/test/UnitConvert.cpp
@@ -0,0 +1,123 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <config.h>
+
+#include <cassert>
+#include <iostream>
+#include <random>
+
+#include <Common.hpp>
+#include <IoUtil.hpp>
+#include <Protocol.hpp>
+#include <LOOLWebSocket.hpp>
+#include <Unit.hpp>
+#include <Util.hpp>
+#include <FileUtil.hpp>
+#include <helpers.hpp>
+
+#include <Poco/Timestamp.h>
+#include <Poco/StringTokenizer.h>
+#include <Poco/Net/HTTPServerRequest.h>
+#include <Poco/Net/HTMLForm.h>
+#include <Poco/Net/StringPartSource.h>
+#include <Poco/Util/LayeredConfiguration.h>
+
+// Inside the WSD process
+class UnitConvert : public UnitWSD
+{
+ bool _workerStarted;
+ std::thread _worker;
+
+public:
+ UnitConvert() :
+ _workerStarted(false)
+ {
+ setHasKitHooks();
+ setTimeout(3600 * 1000); /* one hour */
+ }
+ ~UnitConvert()
+ {
+ LOG_INF("Joining test worker thread\n");
+ _worker.join();
+ }
+
+ void configure(Poco::Util::LayeredConfiguration& config) override
+ {
+ UnitWSD::configure(config);
+
+ config.setBool("ssl.enable", true);
+ config.setInt("per_document.limit_load_secs", 1);
+ }
+
+ void invokeTest() override
+ {
+ if (_workerStarted)
+ return;
+ _workerStarted = true;
+ std::cerr << "Starting thread ...\n";
+ _worker = std::thread([this]{
+ std::cerr << "Now started thread ...\n";
+ std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(Poco::URI(helpers::getTestServerURI())));
+ session->setTimeout(Poco::Timespan(10, 0)); // 10 seconds.
+
+ Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to/pdf");
+ Poco::Net::HTMLForm form;
+ form.setEncoding(Poco::Net::HTMLForm::ENCODING_MULTIPART);
+ form.set("format", "txt");
+ form.addPart("data", new Poco::Net::StringPartSource("Hello World Content", "text/plain", "foo.txt"));
+ form.prepareSubmit(request);
+ form.write(session->sendRequest(request));
+
+ Poco::Net::HTTPResponse response;
+ std::stringstream actualStream;
+ try {
+ session->receiveResponse(response);
+ } catch (Poco::Net::NoMessageException &) {
+ std::cerr << "No response as expected.\n";
+ exitTest(TestResult::Ok); // child should have timed out and been killed.
+ return;
+ } // else
+ std::cerr << "Failed to terminate the sleeping kit\n";
+ exitTest(TestResult::Failed);
+ });
+ }
+};
+
+// Inside the forkit & kit processes
+class UnitKitConvert : public UnitKit
+{
+public:
+ UnitKitConvert()
+ {
+ setTimeout(3600 * 1000); /* one hour */
+ }
+ bool filterKitMessage(WebSocketHandler *, std::string &message) override
+ {
+ std::cerr << "kit message " << message << "\n";
+ if (message.find("load") != std::string::npos)
+ {
+ std::cerr << "Load message received - starting to sleep\n";
+ sleep(60);
+ }
+ return false;
+ }
+};
+
+UnitBase *unit_create_wsd(void)
+{
+ return new UnitConvert();
+}
+
+UnitBase *unit_create_kit(void)
+{
+ return new UnitKitConvert();
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/UnitFuzz.cpp b/test/UnitFuzz.cpp
index e36f78dd6..e9b1d9d06 100644
--- a/test/UnitFuzz.cpp
+++ b/test/UnitFuzz.cpp
@@ -137,8 +137,7 @@ public:
std::cerr << "\n\nYour KIT process has fuzzing hooks\n\n\n";
setTimeout(3600 * 1000); /* one hour */
}
- virtual bool filterKitMessage(const std::shared_ptr<LOOLWebSocket> & /* ws */,
- std::string & /* message */) override
+ virtual bool filterKitMessage(WebSocketHandler *, std::string & /* message */) override
{
return false;
}
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 7b56fc651..fd8fdd4ca 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -253,6 +253,9 @@ void DocumentBroker::pollThread()
#endif
auto last30SecCheckTime = std::chrono::steady_clock::now();
+ int limit_load_secs = LOOLWSD::getConfigValue<int>("per_document.limit_load_secs", 100);
+ auto loadDeadline = std::chrono::steady_clock::now() + std::chrono::seconds(limit_load_secs);
+
// Main polling loop goodness.
while (!_stop && _poll->continuePolling() && !TerminationFlag)
{
@@ -261,6 +264,15 @@ void DocumentBroker::pollThread()
const auto now = std::chrono::steady_clock::now();
#ifndef MOBILEAPP
+ if (!_isLoaded && (limit_load_secs > 0) && (now > loadDeadline))
+ {
+ // Brutal but effective.
+ if (_childProcess)
+ _childProcess->terminate();
+ stop("Load timed out");
+ continue;
+ }
+
if (std::chrono::duration_cast<std::chrono::milliseconds>
(now - lastBWUpdateTime).count() >= 5 * 1000)
{
@@ -1852,6 +1864,8 @@ void DocumentBroker::dumpState(std::ostream& os)
uint64_t sent, recv;
getIOStats(sent, recv);
+ auto now = std::chrono::steady_clock::now();
+
os << " Broker: " << LOOLWSD::anonymizeUrl(_filename) << " pid: " << getPid();
if (_markToDestroy)
os << " *** Marked to destroy ***";
@@ -1860,7 +1874,9 @@ void DocumentBroker::dumpState(std::ostream& os)
if (_isLoaded)
os << "\n loaded in: " << _loadDuration.count() << "ms";
else
- os << "\n still loading...";
+ os << "\n still loading... " <<
+ std::chrono::duration_cast<std::chrono::seconds>(
+ now - _threadStart).count() << "s";
os << "\n sent: " << sent;
os << "\n recv: " << recv;
os << "\n modified?: " << _isModified;
@@ -1871,9 +1887,9 @@ void DocumentBroker::dumpState(std::ostream& os)
os << "\n doc key: " << _docKey;
os << "\n doc id: " << _docId;
os << "\n num sessions: " << _sessions.size();
- const std::time_t t = std::chrono::system_clock::to_time_t(std::chrono::time_point_cast<std::chrono::seconds>(
- std::chrono::system_clock::now()
- + (_lastSaveTime - std::chrono::steady_clock::now())));
+ const std::time_t t = std::chrono::system_clock::to_time_t(
+ std::chrono::time_point_cast<std::chrono::seconds>(
+ std::chrono::system_clock::now() + (_lastSaveTime - now)));
os << "\n last saved: " << std::ctime(&t);
os << "\n cursor " << _cursorPosX << ", " << _cursorPosY
<< "( " << _cursorWidth << "," << _cursorHeight << ")\n";
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index c0ab0d57c..6f66bfb58 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -79,10 +79,11 @@ public:
~ChildProcess()
{
+ LOG_DBG("~ChildProcess dtor [" << _pid << "].");
+
if (_pid <= 0)
return;
- LOG_DBG("~ChildProcess dtor [" << _pid << "].");
terminate();
// No need for the socket anymore.
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 85040f13e..15a9c9c0a 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -590,6 +590,8 @@ public:
Path tempPath = _convertTo? Path::forDirectory(Poco::TemporaryFile::tempName("/tmp/convert-to") + "/") :
Path::forDirectory(Poco::TemporaryFile::tempName() + "/");
File(tempPath).createDirectories();
+ chmod(tempPath.toString().c_str(), S_IXUSR | S_IWUSR | S_IRUSR);
+
// Prevent user inputting anything funny here.
// A "filename" should always be a filename, not a path
const Path filenameParam(params.get("filename"));
@@ -783,6 +785,7 @@ void LOOLWSD::initialize(Application& self)
{ "per_document.idlesave_duration_secs", "30" },
{ "per_document.limit_file_size_mb", "0" },
{ "per_document.limit_num_open_files", "0" },
+ { "per_document.limit_load_secs", "100" },
{ "per_document.limit_stack_mem_kb", "8000" },
{ "per_document.limit_virt_mem_mb", "0" },
{ "per_document.max_concurrency", "4" },
@@ -2321,6 +2324,7 @@ private:
if (!allowConvertTo(socket->clientAddress(), request, true))
{
+ LOG_TRC("Conversion not allowed from this address");
std::ostringstream oss;
oss << "HTTP/1.1 403\r\n"
<< "Date: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n"
@@ -2337,10 +2341,9 @@ private:
format = tokens[3];
bool sent = false;
+ LOG_INF("Conversion request for URI [" << fromPath << "] format [" << format << "].");
if (!fromPath.empty() && !format.empty())
{
- LOG_INF("Conversion request for URI [" << fromPath << "].");
-
Poco::URI uriPublic = DocumentBroker::sanitizeURI(fromPath);
const std::string docKey = DocumentBroker::getDocKey(uriPublic);
More information about the Libreoffice-commits
mailing list