Rebased ref, commits from common ancestor: commit e9c3e0752b8c00cab931dcf109ed81418f801242 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Fri Jan 17 21:18:42 2020 +0000 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Sat Jan 18 16:40:05 2020 +0000
test: dung out redundant LOOL_TEST_CLIENT_PORT. And cleanup other related oddities. Change-Id: I2d179a2ece6a8553e10e406ad4e5da08a2ff58e5 diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp index 164489589..ac0191e33 100644 --- a/kit/ForKit.cpp +++ b/kit/ForKit.cpp @@ -396,15 +396,6 @@ int main(int argc, char** argv) std::string sysTemplate; std::string loTemplate; -#if ENABLE_DEBUG - static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT"); - if (clientPort) - ClientPortNumber = std::stoi(clientPort); - static const char* masterPort = std::getenv("LOOL_TEST_MASTER_PORT"); - if (masterPort) - MasterLocation = masterPort; -#endif - for (int i = 0; i < argc; ++i) { char *cmd = argv[i]; diff --git a/test/Makefile.am b/test/Makefile.am index 2b369465a..e10cb7d8c 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -2,12 +2,10 @@ export MAX_CONCURRENCY=4 AUTOMAKE_OPTION = serial-tests -# unittest: tests that do not need loolwsd running, and that are run during a -# normal build -# test: tests that need loolwsd running, and that are run via 'make check' -check_PROGRAMS = test fakesockettest +# unittest: tests that run a captive loolwsd as part of themselves. +check_PROGRAMS = fakesockettest -noinst_PROGRAMS = test fakesockettest unittest +noinst_PROGRAMS = fakesockettest unittest AM_CXXFLAGS = $(CPPUNIT_CFLAGS) -DTDOC=\"$(abs_top_srcdir)/test/data\" \ -I${top_srcdir}/common -I${top_srcdir}/net -I${top_srcdir}/wsd -I${top_srcdir}/kit @@ -87,10 +85,6 @@ unittest_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS unittest_SOURCES = $(test_base_source) test.cpp unittest_LDADD = $(CPPUNIT_LIBS) -test_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS -test_SOURCES = $(test_all_source) test.cpp -test_LDADD = $(CPPUNIT_LIBS) - fakesockettest_SOURCES = fakesockettest.cpp ../net/FakeSocket.cpp fakesockettest_LDADD = $(CPPUNIT_LIBS) diff --git a/test/helpers.hpp b/test/helpers.hpp index dd638d191..19bd8804d 100644 --- a/test/helpers.hpp +++ b/test/helpers.hpp @@ -187,12 +187,15 @@ Poco::Net::HTTPClientSession* createSession(const Poco::URI& uri) return new Poco::Net::HTTPClientSession(uri.getHost(), uri.getPort()); } +#ifndef UNIT_CLIENT_TESTS + inline int getClientPort() { - static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT"); - return clientPort? atoi(clientPort) : DEFAULT_CLIENT_PORT_NUMBER; + return DEFAULT_CLIENT_PORT_NUMBER; } +#endif + inline std::shared_ptr<Poco::Net::StreamSocket> createRawSocket() { return diff --git a/test/integration-http-server.cpp b/test/integration-http-server.cpp index 4db04737a..5d9342b76 100644 --- a/test/integration-http-server.cpp +++ b/test/integration-http-server.cpp @@ -92,15 +92,13 @@ public: // A server URI which was not added to loolwsd.xml as post_allow IP or a wopi storage host Poco::URI getNotAllowedTestServerURI() { - static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT"); - static std::string serverURI( #if ENABLE_SSL - "https://165.227.162.232:" + "https://165.227.162.232:9980" #else - "http://165.227.162.232:" + "http://165.227.162.232:9980" #endif - + (clientPort? std::string(clientPort) : std::to_string(DEFAULT_CLIENT_PORT_NUMBER))); + ); return Poco::URI(serverURI); } diff --git a/test/run_unit.sh.in b/test/run_unit.sh.in index 5fb8a6936..4b893f23c 100755 --- a/test/run_unit.sh.in +++ b/test/run_unit.sh.in @@ -70,55 +70,10 @@ fi echo "Test output is '$test_output'" echo > $test_output -if test "z$tst" == "z"; then - # run the test on a dedicated port - export LOOL_TEST_CLIENT_PORT=9984 - export LOOL_TEST_MASTER_PORT=9985 - - echo "Executing external tests" - ${trace} \ - ${abs_top_builddir}/loolwsd --o:sys_template_path="$systemplate_path" \ - --o:child_root_path="$jails_path" \ - --o:storage.filesystem[@allow]=true \ - --o:logging.level=trace \ - --o:logging.file[@enable]=false \ - --o:ssl.key_file_path="${abs_top_builddir}/etc/key.pem" \ - --o:ssl.cert_file_path="${abs_top_builddir}/etc/cert.pem" \ - --o:ssl.ca_file_path="${abs_top_builddir}/etc/ca-chain.cert.pem" \ - --o:admin_console.username=admin --o:admin_console.password=admin \ - --o:storage.ssl.enable=false \ - > "$tst_log" 2>&1 & - if test "z${SLEEPFORDEBUGGER}${SLEEPKITFORDEBUGGER}" != "z"; then - echo "sleeping for debugger" - sleep ${SLEEPFORDEBUGGER:-0} - sleep ${SLEEPKITFORDEBUGGER:-0} - fi - - echo " executing test" - - oldpath=`pwd` - cd "${abs_top_builddir}/test" - if eval ${trace} ./test ${verbose}; then - echo "Test run_test.sh passed." - echo ":test-result: PASS run_test.sh" >> $oldpath/$test_output - retval=0 - else - echo ":test-result: FAIL run_test.sh" >> $oldpath/$test_output - retval=1 - fi - - if test "z${SLEEPFORDEBUGGER}${SLEEPKITFORDEBUGGER}" == "z"; then - echo "killing $!" - kill $! - fi - - exit $retval - -else # newer unit tests. - echo "Running $tst | $tst_log ..."; - if ${trace} \ +echo "Running $tst | $tst_log ..."; +if ${trace} \ ${abs_top_builddir}/loolwsd --o:sys_template_path="$systemplate_path" \ - --o:child_root_path="$jails_path" \ + --o:child_root_path="$jails_path" \ --o:storage.filesystem[@allow]=true \ --o:logging.level=trace \ --o:ssl.key_file_path="${abs_top_builddir}/etc/key.pem" \ @@ -129,7 +84,7 @@ else # newer unit tests. --unitlib="${abs_top_builddir}/test/.libs/$tst.so" 2> "$tst_log"; then echo "Test $tst passed." echo ":test-result: PASS $tst" >> $test_output - else +else cat $tst_log echo "=============================================================" echo "Test failed on unit: $tst re-run with:" @@ -147,7 +102,6 @@ else # newer unit tests. echo " $ less $tst_log # for detailed failure log files" echo "=============================================================" echo ":test-result: FAIL $tst" >> $test_output - fi fi # vim:set shiftwidth=4 expandtab: diff --git a/test/test.cpp b/test/test.cpp index 4a5b0f6d0..f80dab522 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -314,6 +314,11 @@ int getLoolKitProcessCount() return getKitPids().size(); } +int getClientPort() +{ + return LOOLWSD::getClientPortNumber(); +} + #endif // UNIT_CLIENT_TESTS /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/test.hpp b/test/test.hpp index 29b6e76d3..a5a2b2280 100644 --- a/test/test.hpp +++ b/test/test.hpp @@ -31,6 +31,9 @@ std::vector<int> getDocKitPids(); /// Get the PID of the forkit std::vector<int> getForKitPids(); +/// Which port should we connect to get to WSD. +int getClientPort(); + /// How many live loolkit processes do we have ? int getLoolKitProcessCount(); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 2122d9e8e..1dbc92d7e 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -1404,10 +1404,6 @@ void LOOLWSD::handleOption(const std::string& optionName, else if (optionName == "careerspan") careerSpanMs = std::stoi(value) * 1000; // Convert second to ms - static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT"); - if (clientPort) - ClientPortNumber = std::stoi(clientPort); - static const char* latencyMs = std::getenv("LOOL_DELAY_SOCKET_MS"); if (latencyMs) SimulatedLatencyMs = std::stoi(latencyMs); @@ -3670,6 +3666,11 @@ std::vector<std::shared_ptr<DocumentBroker>> LOOLWSD::getBrokersTestOnly() return result; } +int LOOLWSD::getClientPortNumber() +{ + return ClientPortNumber; +} + std::vector<int> LOOLWSD::getKitPids() { std::vector<int> pids; diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp index 71ddb2155..35c283288 100644 --- a/wsd/LOOLWSD.hpp +++ b/wsd/LOOLWSD.hpp @@ -80,6 +80,8 @@ public: static std::set<const Poco::Util::AbstractConfiguration*> PluginConfigurations; static std::chrono::time_point<std::chrono::system_clock> StartTime; + /// For testing only [!] + static int getClientPortNumber(); /// For testing only [!] static std::vector<int> getKitPids(); /// For testing only [!] DocumentBrokers are mostly single-threaded with their own thread commit 536195be9027a8a4dc003dc1418c129115c1f0cd Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Fri Jan 17 20:49:38 2020 +0000 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Sat Jan 18 16:40:05 2020 +0000 test: switch to parallel tests based on Unit framework. Increase a few timeouts, bin old-style standalone unit tests, fix a number of bugs. Change-Id: Ia3d59466ecb9a9443807ba3445d04dd5f77e3dba diff --git a/common/Unit.cpp b/common/Unit.cpp index 5dc01d4ea..875c97686 100644 --- a/common/Unit.cpp +++ b/common/Unit.cpp @@ -26,7 +26,7 @@ #include <common/SigUtil.hpp> UnitBase *UnitBase::Global = nullptr; - +char * UnitBase::UnitLibPath; static std::thread TimeoutThread; static std::atomic<bool> TimeoutThreadRunning(false); std::timed_mutex TimeoutThreadMutex; @@ -41,6 +41,9 @@ UnitBase *UnitBase::linkAndCreateUnit(UnitType type, const std::string &unitLibP return nullptr; } + // avoid std:string de-allocation during failure / exit. + UnitLibPath = strdup(unitLibPath.c_str()); + const char *symbol = nullptr; switch (type) { diff --git a/common/Unit.hpp b/common/Unit.hpp index 63975514b..c403100c2 100644 --- a/common/Unit.hpp +++ b/common/Unit.hpp @@ -138,11 +138,17 @@ public: return *Global; } + static std::string getUnitLibPath() { return std::string(UnitLibPath); } + private: - void setHandle(void *dlHandle) { _dlHandle = dlHandle; } + void setHandle(void *dlHandle) + { + _dlHandle = dlHandle; + } static UnitBase *linkAndCreateUnit(UnitType type, const std::string& unitLibPath); void *_dlHandle; + static char *UnitLibPath; bool _setRetValue; int _retValue; int _timeoutMilliSeconds; diff --git a/test/Makefile.am b/test/Makefile.am index f1d4963b1..2b369465a 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -13,9 +13,10 @@ AM_CXXFLAGS = $(CPPUNIT_CFLAGS) -DTDOC=\"$(abs_top_srcdir)/test/data\" \ -I${top_srcdir}/common -I${top_srcdir}/net -I${top_srcdir}/wsd -I${top_srcdir}/kit noinst_LTLIBRARIES = \ + unit-base.la unit-tiletest.la \ + unit-integration.la unit-httpws.la unit-crash.la \ unit-convert.la unit-typing.la unit-copy-paste.la \ - unit-timeout.la unit-prefork.la \ - unit-storage.la unit-client.la \ + unit-timeout.la unit-prefork.la unit-storage.la \ unit-admin.la unit-tilecache.la \ unit-fuzz.la unit-oob.la unit-http.la unit-oauth.la \ unit-wopi.la unit-wopi-saveas.la \ @@ -82,13 +83,6 @@ test_base_source = \ DeltaTests.cpp \ $(wsd_sources) -test_all_source = \ - $(test_base_source) \ - TileCacheTests.cpp \ - integration-http-server.cpp \ - httpwstest.cpp \ - httpcrashtest.cpp - unittest_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS unittest_SOURCES = $(test_base_source) test.cpp unittest_LDADD = $(CPPUNIT_LIBS) @@ -100,6 +94,18 @@ test_LDADD = $(CPPUNIT_LIBS) fakesockettest_SOURCES = fakesockettest.cpp ../net/FakeSocket.cpp fakesockettest_LDADD = $(CPPUNIT_LIBS) +# old-style unit tests - bootstrapped via UnitClient +unit_base_la_SOURCES = UnitClient.cpp ${test_base_source} +unit_base_la_LIBADD = $(CPPUNIT_LIBS) +unit_tiletest_la_SOURCES = UnitClient.cpp TileCacheTests.cpp +unit_tiletest_la_LIBADD = $(CPPUNIT_LIBS) +unit_integration_la_SOURCES = UnitClient.cpp integration-http-server.cpp +unit_integration_la_LIBADD = $(CPPUNIT_LIBS) +unit_httpws_la_SOURCES = UnitClient.cpp httpwstest.cpp +unit_httpws_la_LIBADD = $(CPPUNIT_LIBS) +unit_crash_la_SOURCES = UnitClient.cpp httpcrashtest.cpp +unit_crash_la_LIBADD = $(CPPUNIT_LIBS) + # unit test modules: unit_oob_la_SOURCES = UnitOOB.cpp unit_http_la_SOURCES = UnitHTTP.cpp @@ -107,8 +113,6 @@ unit_http_la_LIBADD = $(CPPUNIT_LIBS) unit_fuzz_la_SOURCES = UnitFuzz.cpp 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_typing_la_SOURCES = UnitTyping.cpp unit_typing_la_LIBADD = $(CPPUNIT_LIBS) unit_copy_paste_la_SOURCES = UnitCopyPaste.cpp @@ -183,16 +187,20 @@ if HAVE_LO_PATH check-local: ./fakesockettest @fc-cache "@LO_PATH@"/share/fonts/truetype - ./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-copy-paste.la unit-typing.la unit-convert.la unit-prefork.la unit-tilecache.la unit-timeout.la \ +TESTS = \ + unit-base.la unit-tiletest.la \ + unit-integration.la unit-httpws.la unit-crash.la \ + \ + unit-copy-paste.la unit-typing.la 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 unit_wopi_renamefile.la unit_wopi_watermark.la \ unit-http.la \ unit-tiff-load.la \ - unit-large-paste.la \ + unit.large-paste.la \ unit-paste.la \ unit-load-torture.la \ unit-rendering-options.la \ @@ -209,9 +217,8 @@ TESTS = unit-copy-paste.la unit-typing.la unit-convert.la unit-prefork.la unit-t unit-bad-doc-load.la \ unit-hosting.la \ unit-wopi-loadencoded.la unit-wopi-temp.la -# TESTS = unit-client.la -# TESTS += unit-admin.la -# TESTS += unit-storage.la +# TESTS += unit-admin.test +# TESTS += unit-storage.test else TESTS = ${top_builddir}/test/test endif diff --git a/test/UnitClient.cpp b/test/UnitClient.cpp index e3580f90e..03682fdc5 100644 --- a/test/UnitClient.cpp +++ b/test/UnitClient.cpp @@ -7,7 +7,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -// Runs client tests in their own thread inside a WSD process. +// Runs old-style CPPUNIT tests in their own thread inside a WSD process. +// Depending which cppunit objects this is linked with this runs different +// tests. #include <config.h> diff --git a/test/helpers.hpp b/test/helpers.hpp index f09de8bbb..dd638d191 100644 --- a/test/helpers.hpp +++ b/test/helpers.hpp @@ -390,8 +390,12 @@ inline bool isDocumentLoaded(LOOLWebSocket& ws, const std::string& testname, bool isView = true) { const std::string prefix = isView ? "status:" : "statusindicatorfinish:"; - const auto message = getResponseString(ws, prefix, testname); - return LOOLProtocol::matchPrefix(prefix, message); + // Allow 20 secs to load + const auto message = getResponseString(ws, prefix, testname, 30 * 1000); + bool success = LOOLProtocol::matchPrefix(prefix, message); + if (!success) + TST_LOG("ERR: Timed out loading document"); + return success; } inline diff --git a/test/test.cpp b/test/test.cpp index cc00b2fde..4a5b0f6d0 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -183,7 +183,8 @@ bool runClientTests(bool standalone, bool verbose) std::cerr << " (cd test; CPPUNIT_TEST_NAME=\"" << (*failures.begin())->failedTestName() << "\" gdb --args " << cmd << ")\n\n"; } #else - std::cerr << "(cd test; CPPUNIT_TEST_NAME=\"" << (*failures.begin())->failedTestName() << "\" make check)\n\n"; + std::cerr << "(cd test; CPPUNIT_TEST_NAME=\"" << (*failures.begin())->failedTestName() << + "\" ./run_unit.sh --test-name " << UnitBase::get().getUnitLibPath() << ")\n\n"; #endif } diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 9bf7088c6..876436abe 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -309,9 +309,9 @@ void DocumentBroker::pollThread() if (!_isLoaded && (limit_load_secs > 0) && (now > loadDeadline)) { - LOG_WRN("Doc [" << _docKey << "] is taking too long to load. Will kill process [" - << _childProcess->getPid() << "]. per_document.limit_load_secs set to " - << limit_load_secs << " secs."); + LOG_ERR("Doc [" << _docKey << "] is taking too long to load. Will kill process [" + << _childProcess->getPid() << "]. per_document.limit_load_secs set to " + << limit_load_secs << " secs."); broadcastMessage("error: cmd=load kind=docloadtimeout"); // Brutal but effective. diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index d47253e93..2122d9e8e 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -3283,14 +3283,19 @@ private: std::shared_ptr<ServerSocket> socket = getServerSocket( ClientListenAddr, port, WebServerPoll, factory); + + while (!socket && #ifdef BUILDING_TESTS - while (!socket) + true +#else + UnitWSD::isUnitTesting() +#endif + ) { ++port; LOG_INF("Client port " << (port - 1) << " is busy, trying " << port << "."); - socket = getServerSocket(port, WebServerPoll, factory); + socket = getServerSocket(ClientListenAddr, port, WebServerPoll, factory); } -#endif if (!socket) { commit f13d626a03ecd16d8e33d19c7d1771dcbf0f6981 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Sat Jan 18 15:56:01 2020 +0000 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Sat Jan 18 16:40:05 2020 +0000 copyFile: de-poco-ize and handle EINTR and short writes. Change-Id: I2046881c786a9f31f45c53f282de9ddd9a9cebcf diff --git a/common/FileUtil.cpp b/common/FileUtil.cpp index 5066d2938..cb8f07f13 100644 --- a/common/FileUtil.cpp +++ b/common/FileUtil.cpp @@ -88,6 +88,79 @@ namespace FileUtil return name; } + void copyFileTo(const std::string &fromPath, const std::string &toPath) + { + int from = -1, to = -1; + try { + from = open(fromPath.c_str(), O_RDONLY); + if (from < 0) + { + LOG_SYS("Failed to open src " << anonymizeUrl(fromPath)); + throw; + } + + struct stat st; + if (fstat(from, &st) != 0) + { + LOG_SYS("Failed to fstat src " << anonymizeUrl(fromPath)); + throw; + } + + to = open(toPath.c_str(), O_CREAT | O_TRUNC | O_WRONLY, st.st_mode); + if (to < 0) + { + LOG_SYS("Failed to fstat dest " << anonymizeUrl(toPath)); + throw; + } + + LOG_INF("Copying " << st.st_size << " bytes from " << anonymizeUrl(fromPath) << " to " << anonymizeUrl(toPath)); + + char buffer[4096*64]; + + int n; + off_t bytesIn = 0; + do { + while ((n = ::read(from, buffer, sizeof(buffer))) < 0 && errno == EINTR) + LOG_TRC("EINTR reading from " << anonymizeUrl(fromPath)); + if (n < 0) + { + LOG_SYS("Failed to read from " << anonymizeUrl(fromPath) << " at " << bytesIn << " bytes in"); + throw; + } + bytesIn += n; + if (n == 0) // EOF + break; + // Handle short writes and EINTR + for (int j = 0; j < n;) + { + int written; + while ((written = ::write(to, buffer + j, sizeof(buffer) - j)) < 0 && errno == EINTR) + LOG_TRC("EINTR writing to " << anonymizeUrl(toPath)); + if (written < 0) + { + LOG_SYS("Failed to write " << n << " bytes to " << anonymizeUrl(toPath) << " at " << + bytesIn << " bytes into " << anonymizeUrl(fromPath)); + throw; + } + j += written; + } + } while(true); + if (bytesIn != st.st_size) + { + LOG_WRN("Unusual: file " << anonymizeUrl(fromPath) << " changed size " + "during copy from " << st.st_size << " to " << bytesIn); + } + } + catch (...) + { + LOG_SYS("Failed to copy from " << anonymizeUrl(fromPath) << " to " << anonymizeUrl(toPath)); + close(from); + close(to); + unlink(toPath.c_str()); + throw Poco::Exception("failed to copy"); + } + } + std::string getTempFilePath(const std::string& srcDir, const std::string& srcFilename, const std::string& dstFilenamePrefix) { const std::string srcPath = srcDir + '/' + srcFilename; @@ -100,7 +173,7 @@ namespace FileUtil fileDeleter.registerForDeletion(dstPath); #else const std::string dstPath = Poco::Path(Poco::Path::temp(), dstFilename).toString(); - Poco::File(srcPath).copyTo(dstPath); + copyFileTo(srcPath, dstPath); Poco::TemporaryFile::registerForDeletion(dstPath); #endif diff --git a/common/FileUtil.hpp b/common/FileUtil.hpp index a57aa5414..864188308 100644 --- a/common/FileUtil.hpp +++ b/common/FileUtil.hpp @@ -80,6 +80,9 @@ namespace FileUtil removeFile(path.toString(), recursive); } + /// Copy a file from @fromPath to @toPath, throws on failure. + void copyFileTo(const std::string &fromPath, const std::string &toPath); + /// Make a temp copy of a file, and prepend it with a prefix. std::string getTempFilePath(const std::string& srcDir, const std::string& srcFilename, const std::string& dstFilenamePrefix); diff --git a/test/run_unit.sh.in b/test/run_unit.sh.in index d05e2f48f..5fb8a6936 100755 --- a/test/run_unit.sh.in +++ b/test/run_unit.sh.in @@ -56,16 +56,6 @@ echo " $cmd_line" # drop .la suffix tst=`echo $tst | sed "s/\.la//"`; -if test "z$tst" != "z" && test "z$CPPUNIT_TEST_NAME" != "z"; then - # $tst is not empty, but $CPPUNIT_TEST_NAME is set, exit early if they - # don't match. - if test "z$tst" != "z$CPPUNIT_TEST_NAME"; then - touch $tst_log - echo ":test-result: SKIP $tst (disabled by CPPUNIT_TEST_NAME)" > $test_output - exit 0; - fi -fi - export LOOL_LOGLEVEL=trace if test "z$enable_debug" != "ztrue"; then diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp index e1efa864c..f35bf7c3e 100644 --- a/wsd/Storage.cpp +++ b/wsd/Storage.cpp @@ -331,8 +331,7 @@ std::string LocalStorage::loadStorageFileToLocal(const Authorization& /*auth*/, // Fallback to copying. if (!Poco::File(getRootFilePath()).exists()) { - LOG_INF("Copying " << LOOLWSD::anonymizeUrl(publicFilePath) << " to " << getRootFilePathAnonym()); - Poco::File(publicFilePath).copyTo(getRootFilePath()); + FileUtil::copyFileTo(publicFilePath, getRootFilePath()); _isCopy = true; } } @@ -371,10 +370,7 @@ StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Authorization LOG_TRC("Saving local file to local file storage (isCopy: " << _isCopy << ") for " << getRootFilePathAnonym()); // Copy the file back. if (_isCopy && Poco::File(getRootFilePath()).exists()) - { - LOG_INF("Copying " << getRootFilePathAnonym() << " to " << LOOLWSD::anonymizeUrl(getUri().getPath())); - Poco::File(getRootFilePath()).copyTo(getUri().getPath()); - } + FileUtil::copyFileTo(getRootFilePath(), getUri().getPath()); // update its fileinfo object. This is used later to check if someone else changed the // document while we are/were editing it diff --git a/wsd/TraceFile.hpp b/wsd/TraceFile.hpp index 82f5a75ce..e95fbb2e2 100644 --- a/wsd/TraceFile.hpp +++ b/wsd/TraceFile.hpp @@ -25,6 +25,7 @@ #include "Protocol.hpp" #include "Log.hpp" #include "Util.hpp" +#include "FileUtil.hpp" /// Dumps commands and notification trace. class TraceFileRecord @@ -141,8 +142,7 @@ public: filename += '.' + origPath.getExtension(); snapshot = Poco::Path(_path, filename).toString(); - LOG_TRC("TraceFile: Copying local file [" << localPath << "] to snapshot [" << snapshot << "]."); - Poco::File(localPath).copyTo(snapshot); + FileUtil::copyFileTo(localPath, snapshot); snapshot = Poco::URI(Poco::URI("file://"), snapshot).toString(); LOG_TRC("TraceFile: Mapped URL " << url << " to " << snapshot); commit b8fee3f24f8cdac32460223b24af562f4ed08330 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Fri Jan 17 22:31:41 2020 +0000 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Sat Jan 18 16:40:05 2020 +0000 Move file url anonymization down from LOOLWSD into FileUtil. Change-Id: I415c73b10621d5c7c942367bbf38a3bbd9bf8f27 diff --git a/common/FileUtil.cpp b/common/FileUtil.cpp index f94911606..5066d2938 100644 --- a/common/FileUtil.cpp +++ b/common/FileUtil.cpp @@ -343,6 +343,30 @@ namespace FileUtil return true; } + namespace { + bool AnonymizeUserData = false; + std::uint64_t AnonymizationSalt = 82589933; + } + + void setUrlAnonymization(bool anonymize, const std::uint64_t salt) + { + AnonymizeUserData = anonymize; + AnonymizationSalt = salt; + } + + /// Anonymize the basename of filenames, preserving the path and extension. + std::string anonymizeUrl(const std::string& url) + { + return AnonymizeUserData ? Util::anonymizeUrl(url, AnonymizationSalt) : url; + } + + /// Anonymize user names and IDs. + /// Will use the Obfuscated User ID if one is provied via WOPI. + std::string anonymizeUsername(const std::string& username) + { + return AnonymizeUserData ? Util::anonymize(username, AnonymizationSalt) : username; + } + } // namespace FileUtil /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/common/FileUtil.hpp b/common/FileUtil.hpp index 84b42f65c..a57aa5414 100644 --- a/common/FileUtil.hpp +++ b/common/FileUtil.hpp @@ -17,6 +17,16 @@ namespace FileUtil { + /// Used for anonymizing URLs + void setUrlAnonymization(bool anonymize, const std::uint64_t salt); + + /// Anonymize the basename of filenames, preserving the path and extension. + std::string anonymizeUrl(const std::string& url); + + /// Anonymize user names and IDs. + /// Will use the Obfuscated User ID if one is provied via WOPI. + std::string anonymizeUsername(const std::string& username); + /// Create a secure, random directory path. std::string createRandomDir(const std::string& path); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 3b45fd3de..d47253e93 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -715,8 +715,7 @@ std::string LOOLWSD::HostIdentifier; std::string LOOLWSD::ConfigFile = LOOLWSD_CONFIGDIR "/loolwsd.xml"; std::string LOOLWSD::ConfigDir = LOOLWSD_CONFIGDIR "/conf.d"; std::string LOOLWSD::LogLevel = "trace"; -bool LOOLWSD::AnonymizeUserData = false; -std::uint64_t LOOLWSD::AnonymizationSalt = 82589933; +bool LOOLWSD::AnonymizeUserData = 82589933; #if ENABLE_SSL Util::RuntimeConstant<bool> LOOLWSD::SSLEnabled; Util::RuntimeConstant<bool> LOOLWSD::SSLTermination; @@ -1003,14 +1002,16 @@ void LOOLWSD::initialize(Application& self) } } + std::uint64_t anonymizationSalt; LOG_INF("Anonymization of user-data is " << (AnonymizeUserData ? "enabled." : "disabled.")); if (AnonymizeUserData) { // Get the salt, if set, otherwise default, and set as envar, so the kits inherit it. - AnonymizationSalt = getConfigValue<std::uint64_t>(conf, "logging.anonymize.anonymization_salt", 82589933); - const std::string sAnonymizationSalt = std::to_string(AnonymizationSalt); - setenv("LOOL_ANONYMIZATION_SALT", sAnonymizationSalt.c_str(), true); + anonymizationSalt = getConfigValue<std::uint64_t>(conf, "logging.anonymize.anonymization_salt", 82589933); + const std::string anonymizationSaltStr = std::to_string(anonymizationSalt); + setenv("LOOL_ANONYMIZATION_SALT", anonymizationSaltStr.c_str(), true); } + FileUtil::setUrlAnonymization(AnonymizeUserData, anonymizationSalt); { std::string proto = getConfigValue<std::string>(conf, "net.proto", ""); diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp index 6e35bd796..71ddb2155 100644 --- a/wsd/LOOLWSD.hpp +++ b/wsd/LOOLWSD.hpp @@ -24,6 +24,7 @@ #include <Poco/Util/ServerApplication.h> #include "Util.hpp" +#include "FileUtil.hpp" class ChildProcess; class TraceFileWriter; @@ -67,7 +68,6 @@ public: static std::string HostIdentifier; ///< A unique random hash that identifies this server static std::string LogLevel; static bool AnonymizeUserData; - static std::uint64_t AnonymizationSalt; static std::atomic<unsigned> NumConnections; static std::unique_ptr<TraceFileWriter> TraceDumper; #if !MOBILEAPP @@ -184,14 +184,14 @@ public: /// Anonymize the basename of filenames, preserving the path and extension. static std::string anonymizeUrl(const std::string& url) { - return AnonymizeUserData ? Util::anonymizeUrl(url, AnonymizationSalt) : url; + return FileUtil::anonymizeUrl(url); } /// Anonymize user names and IDs. /// Will use the Obfuscated User ID if one is provied via WOPI. static std::string anonymizeUsername(const std::string& username) { - return AnonymizeUserData ? Util::anonymize(username, AnonymizationSalt) : username; + return FileUtil::anonymizeUsername(username); } /// get correct server URL with protocol + port number for this running server commit 64be08cb5a94734bedb291588d988d949dc7740e Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Sat Jan 18 16:36:50 2020 +0000 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Sat Jan 18 16:40:05 2020 +0000 cleanup JWTAuth token before shutdown. valgrind had some exciting double-free action on unclean shutdown. Change-Id: Id7dd3d8ff60387ae51521bd2c74e4d6bcc30ff2e diff --git a/wsd/Auth.cpp b/wsd/Auth.cpp index 01f9e4f43..2ffa17b39 100644 --- a/wsd/Auth.cpp +++ b/wsd/Auth.cpp @@ -37,7 +37,14 @@ using Poco::Base64Decoder; using Poco::Base64Encoder; using Poco::OutputLineEndingConverter; -const Poco::Crypto::RSAKey JWTAuth::_key(Poco::Crypto::RSAKey(Poco::Crypto::RSAKey::KL_2048, Poco::Crypto::RSAKey::EXP_LARGE)); +std::unique_ptr<Poco::Crypto::RSAKey> JWTAuth::_key( + new Poco::Crypto::RSAKey(Poco::Crypto::RSAKey(Poco::Crypto::RSAKey::KL_2048, Poco::Crypto::RSAKey::EXP_LARGE))); + +// avoid obscure doublef rees on exit. +void JWTAuth::cleanup() +{ + _key.reset(); +} const std::string JWTAuth::getAccessToken() { diff --git a/wsd/Auth.hpp b/wsd/Auth.hpp index 1220aee21..00256fb1f 100644 --- a/wsd/Auth.hpp +++ b/wsd/Auth.hpp @@ -13,6 +13,7 @@ #include <cassert> #include <string> +#include <memory> #if !MOBILEAPP #include <Poco/Crypto/RSADigestEngine.h> @@ -43,7 +44,7 @@ public: : _name(name), _sub(sub), _aud(aud), - _digestEngine(_key, "SHA256") + _digestEngine(*_key, "SHA256") { } @@ -51,6 +52,8 @@ public: bool verify(const std::string& accessToken) override; + static void cleanup(); + private: const std::string createHeader(); @@ -65,7 +68,7 @@ private: const std::string _sub; const std::string _aud; - static const Poco::Crypto::RSAKey _key; + static std::unique_ptr<Poco::Crypto::RSAKey> _key; Poco::Crypto::RSADigestEngine _digestEngine; }; diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 0538759ba..3b45fd3de 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -3592,6 +3592,7 @@ void LOOLWSD::cleanup() { #if !MOBILEAPP FileServerRequestHandler::uninitialize(); + JWTAuth::cleanup(); #if ENABLE_SSL // Finally, we no longer need SSL. commit b72f37826edf742eb7d9cf4fd0ac73b2a2e2096b Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Fri Jan 17 19:03:23 2020 +0000 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Sat Jan 18 17:23:02 2020 +0100 sighandler: break infinite loop with corrupted heap ignoring the segv can lead to not making progress, while churning debug. Change-Id: I97af266cec3feefe2dcbd9adb8dbf4b13a4d69bd Reviewed-on: https://gerrit.libreoffice.org/c/online/+/87002 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Michael Meeks <michael.me...@collabora.com> diff --git a/common/SigUtil.cpp b/common/SigUtil.cpp index 8fe38b721..881326df5 100644 --- a/common/SigUtil.cpp +++ b/common/SigUtil.cpp @@ -237,19 +237,16 @@ namespace SigUtil void handleFatalSignal(const int signal) { SigHandlerTrap guard; - if (!guard.isExclusive()) - { - Log::signalLogPrefix(); - Log::signalLog(" Fatal double signal received: "); - Log::signalLog(signalName(signal)); - Log::signalLog("\n Already handling a signal; will ignore this."); - return; - } + bool bReEntered = !guard.isExclusive(); Log::signalLogPrefix(); - Log::signalLog(" Fatal signal received: "); + + // Heap corruption can re-enter through backtrace. + if (bReEntered) + Log::signalLog(" Fatal double signal received: "); + else + Log::signalLog(" Fatal signal received: "); Log::signalLog(signalName(signal)); - Log::signalLog("\n"); struct sigaction action; @@ -259,7 +256,8 @@ namespace SigUtil sigaction(signal, &action, nullptr); - dumpBacktrace(); + if (!bReEntered) + dumpBacktrace(); // let default handler process the signal kill(getpid(), signal); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits