net/Socket.cpp | 2 +- test/httpwstest.cpp | 14 +++++++------- test/integration-http-server.cpp | 36 ++++++++++++++++++++++++++++++------ wsd/LOOLWSD.cpp | 26 ++++++++++++++------------ 4 files changed, 52 insertions(+), 26 deletions(-)
New commits: commit c3632491a47f0fe7d0cac435caad213eff6218c1 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Mon Oct 21 00:48:51 2019 -0400 Commit: Ashod Nakashian <ashnak...@gmail.com> CommitDate: Tue Oct 29 02:32:35 2019 +0100 test: improve HTTPServerTest::testConvertToWithForwardedClientIP Extend the timeout, as often DNS lookup takes several seconds and that delays the response from WSD. Reviewed-on: https://gerrit.libreoffice.org/81198 Reviewed-by: Andras Timar <andras.ti...@collabora.com> Tested-by: Andras Timar <andras.ti...@collabora.com> (cherry picked from commit ae085428dfb11b7965b73df0f40ac4fd1ec98a75) Change-Id: Ie51bff31782fa33eb5559d28af1477e1947382a3 Reviewed-on: https://gerrit.libreoffice.org/81574 Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> Tested-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/net/Socket.cpp b/net/Socket.cpp index cf7087170..cbe5a0a52 100644 --- a/net/Socket.cpp +++ b/net/Socket.cpp @@ -785,7 +785,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 8f36c8205..cf4efcdb4 100644 --- a/test/integration-http-server.cpp +++ b/test/integration-http-server.cpp @@ -370,11 +370,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")); @@ -406,12 +412,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")); @@ -442,12 +455,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")); @@ -472,6 +492,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 e43b827ec..f6d5132dc 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -2037,6 +2037,8 @@ public: break; } } + + init = true; } return hosts.match(address); } @@ -2056,7 +2058,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) { @@ -2538,7 +2540,7 @@ private: // Validate sender - FIXME: should do this even earlier. 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: " << Util::getHttpTimeNow() << "\r\n" @@ -2579,19 +2581,19 @@ private: bFullSheetPreview = false; } - // 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. commit c3bd1b8bf66e9628fc811b2e9667fb065fd5b2ca Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Mon Oct 21 00:47:55 2019 -0400 Commit: Ashod Nakashian <ashnak...@gmail.com> CommitDate: Tue Oct 29 02:32:22 2019 +0100 test: improve HTTPWSTest::testUndoConflict Failure rate is now reduced. Change-Id: I46c26582126c7dba2a3fc86dbbf6ec5f12aa740f Reviewed-on: https://gerrit.libreoffice.org/81197 Reviewed-by: Andras Timar <andras.ti...@collabora.com> Tested-by: Andras Timar <andras.ti...@collabora.com> (cherry picked from commit a12092c85d3b8e8e3b3c54b0eabb6bd3725b9542) Reviewed-on: https://gerrit.libreoffice.org/81573 Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> Tested-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp index a6e2c2f59..40b0d604f 100644 --- a/test/httpwstest.cpp +++ b/test/httpwstest.cpp @@ -2343,22 +2343,22 @@ void HTTPWSTest::testUndoConflict() response = getResponseString(socket1, "invalidatecursor:", testname + "1 "); // edit first view - sendTextFrame(socket0, "key type=input char=97 key=0", testname); - response = getResponseString(socket0, "invalidatecursor:", testname + "0 "); + sendChar(socket0, 'A', skNone, testname + "0 "); + response = getResponseString(socket0, "invalidateviewcursor: ", testname + "0 "); // edit second view - sendTextFrame(socket1, "key type=input char=98 key=0", testname); - response = getResponseString(socket1, "invalidatecursor:", testname + "1 "); + sendChar(socket1, 'B', skNone, testname + "1 "); + response = getResponseString(socket1, "invalidateviewcursor: ", testname + "1 "); // try to undo first view sendTextFrame(socket0, "uno .uno:Undo", testname); // undo conflict response = getResponseString(socket0, "unocommandresult:", testname + "0 "); Poco::JSON::Object::Ptr objJSON = parser.parse(response.substr(17)).extract<Poco::JSON::Object::Ptr>(); - CPPUNIT_ASSERT_EQUAL(objJSON->get("commandName").toString(), std::string(".uno:Undo")); - CPPUNIT_ASSERT_EQUAL(objJSON->get("success").toString(), std::string("true")); + CPPUNIT_ASSERT_EQUAL(std::string(".uno:Undo"), objJSON->get("commandName").toString()); + CPPUNIT_ASSERT_EQUAL(std::string("true"), objJSON->get("success").toString()); CPPUNIT_ASSERT(objJSON->has("result")); const Poco::Dynamic::Var parsedResultJSON = objJSON->get("result"); const auto& resultObj = parsedResultJSON.extract<Poco::JSON::Object::Ptr>(); - CPPUNIT_ASSERT_EQUAL(resultObj->get("type").toString(), std::string("long")); + CPPUNIT_ASSERT_EQUAL(std::string("long"), resultObj->get("type").toString()); CPPUNIT_ASSERT(Poco::strToInt(resultObj->get("value").toString(), conflict, 10)); CPPUNIT_ASSERT(conflict > 0); /*UNDO_CONFLICT*/ } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits