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 <[email protected]> AuthorDate: Mon Oct 21 00:48:51 2019 -0400 Commit: Andras Timar <[email protected]> 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 <[email protected]> Tested-by: Andras Timar <[email protected]> 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. _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
