[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4-0' - net/Socket.cpp test/integration-http-server.cpp wsd/LOOLWSD.cpp
Ashod Nakashian (via logerrit)
logerrit at kemper.freedesktop.org
Tue Oct 22 17:04:12 UTC 2019
net/Socket.cpp | 2 +-
test/integration-http-server.cpp | 36 ++++++++++++++++++++++++++++++------
wsd/LOOLWSD.cpp | 26 ++++++++++++++------------
3 files changed, 45 insertions(+), 19 deletions(-)
New commits:
commit ae085428dfb11b7965b73df0f40ac4fd1ec98a75
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Mon Oct 21 00:48:51 2019 -0400
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Tue Oct 22 19:03:53 2019 +0200
test: improve HTTPServerTest::testConvertToWithForwardedClientIP
Extend the timeout, as often DNS lookup takes several seconds
and that delays the response from WSD.
Change-Id: Ie51bff31782fa33eb5559d28af1477e1947382a3
Reviewed-on: https://gerrit.libreoffice.org/81198
Reviewed-by: Andras Timar <andras.timar at collabora.com>
Tested-by: Andras Timar <andras.timar at collabora.com>
diff --git a/net/Socket.cpp b/net/Socket.cpp
index 9d647dd43..1766eb41f 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -590,7 +590,7 @@ bool StreamSocket::parseHeader(const char *clientName,
}
catch (const std::exception& exc)
{
- LOG_DBG("parseHeader exception caught.");
+ LOG_DBG("parseHeader exception caught: " << exc.what());
// Probably don't have enough data just yet.
// TODO: timeout if we never get enough.
return false;
diff --git a/test/integration-http-server.cpp b/test/integration-http-server.cpp
index 08e755eaf..c38f65261 100644
--- a/test/integration-http-server.cpp
+++ b/test/integration-http-server.cpp
@@ -361,11 +361,17 @@ void HTTPServerTest::testConvertTo()
void HTTPServerTest::testConvertToWithForwardedClientIP()
{
+ const std::string testname = "convertToWithForwardedClientIP-";
+ constexpr int TimeoutSeconds = 10; // Sometimes dns resolving is slow.
+
// Test a forwarded IP which is not allowed to use convert-to feature
+ try
{
- const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", "testConvertToWithForwardedClientIP_");
+ TST_LOG("Converting from a disallowed IP.");
+
+ const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", testname + "disallowed");
std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
- session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds.
+ session->setTimeout(Poco::Timespan(TimeoutSeconds, 0));
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to");
CPPUNIT_ASSERT(!request.has("X-Forwarded-For"));
@@ -388,12 +394,19 @@ void HTTPServerTest::testConvertToWithForwardedClientIP()
std::string actualString = actualStream.str();
CPPUNIT_ASSERT(actualString.empty()); // <- we did not get the converted file
}
+ catch(const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText() + ": " + (exc.nested() ? exc.nested()->displayText() : ""));
+ }
// Test a forwarded IP which is allowed to use convert-to feature
+ try
{
- const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", "testConvertToWithForwardedClientIP_");
+ TST_LOG("Converting from an allowed IP.");
+
+ const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", testname + "allowed");
std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
- session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds.
+ session->setTimeout(Poco::Timespan(TimeoutSeconds, 0));
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to");
CPPUNIT_ASSERT(!request.has("X-Forwarded-For"));
@@ -424,12 +437,19 @@ void HTTPServerTest::testConvertToWithForwardedClientIP()
actualString = actualString.substr(3);
CPPUNIT_ASSERT_EQUAL(expectedStream.str(), actualString); // <- we got the converted file
}
+ catch(const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText() + ": " + (exc.nested() ? exc.nested()->displayText() : ""));
+ }
// Test a forwarded header with three IPs, one is not allowed -> request is denied.
+ try
{
- const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", "testConvertToWithForwardedClientIP_");
+ TST_LOG("Converting from multiple IPs, on disallowed.");
+
+ const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", testname + "disallowed-multi");
std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
- session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds.
+ session->setTimeout(Poco::Timespan(TimeoutSeconds, 0));
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to");
CPPUNIT_ASSERT(!request.has("X-Forwarded-For"));
@@ -454,6 +474,10 @@ void HTTPServerTest::testConvertToWithForwardedClientIP()
std::string actualString = actualStream.str();
CPPUNIT_ASSERT(actualString.empty()); // <- we did not get the converted file
}
+ catch(const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText() + ": " + (exc.nested() ? exc.nested()->displayText() : ""));
+ }
}
CPPUNIT_TEST_SUITE_REGISTRATION(HTTPServerTest);
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index af101523c..690ae41e5 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1984,6 +1984,8 @@ public:
break;
}
}
+
+ init = true;
}
return hosts.match(address);
}
@@ -2003,7 +2005,7 @@ public:
// Handle forwarded header and make sure all participating IPs are allowed
if(request.has("X-Forwarded-For"))
{
- std::string fowardedData = request.get("X-Forwarded-For");
+ const std::string fowardedData = request.get("X-Forwarded-For");
std::vector<std::string> tokens = LOOLProtocol::tokenize(fowardedData, ',');
for(std::string& token : tokens)
{
@@ -2370,7 +2372,7 @@ private:
if (!allowConvertTo(socket->clientAddress(), request, true))
{
- LOG_TRC("Conversion not allowed from this address");
+ LOG_WRN("Conversion requests not allowed from this address: " << socket->clientAddress());
std::ostringstream oss;
oss << "HTTP/1.1 403\r\n"
<< "Date: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n"
@@ -2394,19 +2396,19 @@ private:
Poco::URI uriPublic = DocumentBroker::sanitizeURI(fromPath);
const std::string docKey = DocumentBroker::getDocKey(uriPublic);
- // This lock could become a bottleneck.
- // In that case, we can use a pool and index by publicPath.
- std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
+ // This lock could become a bottleneck.
+ // In that case, we can use a pool and index by publicPath.
+ std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
- LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
- auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey);
- handler.takeFile();
+ LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
+ auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey);
+ handler.takeFile();
- cleanupDocBrokers();
+ cleanupDocBrokers();
- LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
- DocBrokers.emplace(docKey, docBroker);
- LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << "].");
+ LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
+ DocBrokers.emplace(docKey, docBroker);
+ LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << "].");
// Load the document.
// TODO: Move to DocumentBroker.
More information about the Libreoffice-commits
mailing list