[Libreoffice-commits] online.git: Branch 'feature/paralleltest' - 6 commits - common/FileUtil.cpp common/FileUtil.hpp common/SigUtil.cpp common/Unit.cpp common/Unit.hpp kit/ForKit.cpp test/helpers.hpp test/integration-http-server.cpp test/Makefile.am test/run_unit.sh.in test/test.cpp test/test.hpp test/UnitClient.cpp wsd/Auth.cpp wsd/Auth.hpp wsd/DocumentBroker.cpp wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp wsd/Storage.cpp wsd/TraceFile.hpp
Michael Meeks (via logerrit)
logerrit at kemper.freedesktop.org
Sat Jan 18 16:40:54 UTC 2020
Rebased ref, commits from common ancestor:
commit e9c3e0752b8c00cab931dcf109ed81418f801242
Author: Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Fri Jan 17 21:18:42 2020 +0000
Commit: Michael Meeks <michael.meeks at 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.meeks at collabora.com>
AuthorDate: Fri Jan 17 20:49:38 2020 +0000
Commit: Michael Meeks <michael.meeks at 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.meeks at collabora.com>
AuthorDate: Sat Jan 18 15:56:01 2020 +0000
Commit: Michael Meeks <michael.meeks at 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.meeks at collabora.com>
AuthorDate: Fri Jan 17 22:31:41 2020 +0000
Commit: Michael Meeks <michael.meeks at 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.meeks at collabora.com>
AuthorDate: Sat Jan 18 16:36:50 2020 +0000
Commit: Michael Meeks <michael.meeks at 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.meeks at collabora.com>
AuthorDate: Fri Jan 17 19:03:23 2020 +0000
Commit: Michael Meeks <michael.meeks at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Michael Meeks <michael.meeks at 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);
More information about the Libreoffice-commits
mailing list