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 | 18 ++++++- wsd/DocumentBroker.hpp | 3 - wsd/LOOLWSD.cpp | 5 + 9 files changed, 154 insertions(+), 13 deletions(-)
New commits: commit de9a768416d9fa41d8a5b00939cf342ab718658b Author: Michael Meeks <[email protected]> AuthorDate: Wed Nov 7 23:00:32 2018 +0000 Commit: Tor Lillqvist <[email protected]> CommitDate: Thu Nov 8 15:04:37 2018 +0100 Add a time limit for badly behaved / huge document load / conversions. Also improve debug printing of load times in dumpstate. Change-Id: Ib3fd70dffb57588cd90bd928c4be9890cee8bc65 Reviewed-on: https://gerrit.libreoffice.org/63054 Reviewed-by: Tor Lillqvist <[email protected]> Tested-by: Tor Lillqvist <[email protected]> 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 50ed15435..ce1632318 100644 --- a/kit/Kit.cpp +++ b/kit/Kit.cpp @@ -1989,12 +1989,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 LOG_TRC(_socketName << ": recv [" << LOOLProtocol::getAbbreviatedMessage(message) << "]."); std::vector<std::string> tokens = LOOLProtocol::tokenize(message); @@ -2135,6 +2131,7 @@ void lokit_main(const std::string& childRoot, 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 26e5db7a8..b951846e4 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 575a78ca5..227ec56d8 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 \ @@ -83,6 +84,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 @@ -112,7 +114,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 683678847..90d0ecf97 100644 --- a/test/UnitFuzz.cpp +++ b/test/UnitFuzz.cpp @@ -144,8 +144,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 32e20e8f5..5b976caf3 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -248,6 +248,9 @@ void DocumentBroker::pollThread() auto lastBWUpdateTime = std::chrono::steady_clock::now(); 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) { @@ -255,6 +258,15 @@ void DocumentBroker::pollThread() const auto now = std::chrono::steady_clock::now(); + 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) { @@ -1821,6 +1833,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 ***"; @@ -1829,7 +1843,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; diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index cef6bb886..d0fc1a747 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -78,10 +78,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 c16f33f22..68472329d 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -520,6 +520,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")); @@ -709,6 +711,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" }, @@ -2139,7 +2142,6 @@ private: if (!allowPostFrom(socket->clientAddress())) { LOG_ERR("client address DENY: " << socket->clientAddress().c_str()); - std::ostringstream oss; oss << "HTTP/1.1 403\r\n" << "Date: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" @@ -2156,6 +2158,7 @@ private: format = tokens[3]; bool sent = false; + LOG_INF("Conversion request for URI [" << fromPath << "] format [" << format << "]."); if (!fromPath.empty()) { if (!format.empty()) _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
