loolwsd/ChildSession.hpp | 17 ++++++++++++++ loolwsd/ClientSession.hpp | 5 ++-- loolwsd/DocumentBroker.cpp | 17 +++++++++++--- loolwsd/DocumentBroker.hpp | 1 loolwsd/LOOLKit.cpp | 36 ++++++++++++++++++++++++++----- loolwsd/LOOLSession.hpp | 2 + loolwsd/LOOLWSD.cpp | 29 ++++++++++++++++++------ loolwsd/MessageQueue.cpp | 17 ++++++++++++-- loolwsd/MessageQueue.hpp | 3 +- loolwsd/Util.hpp | 24 ++++++++++++++++++++ loolwsd/test/WhiteBoxTests.cpp | 5 ++++ loolwsd/test/httpwstest.cpp | 16 +------------ loolwsd/test/integration-http-server.cpp | 1 13 files changed, 137 insertions(+), 36 deletions(-)
New commits: commit 1edd37ea2dbe1e3d140115396b85bc09be90af4b Author: Ashod Nakashian <[email protected]> Date: Sun Oct 9 13:42:30 2016 -0400 loolwsd: fix convert-to handling Change-Id: I24b8c0b7129bee2692696809613ee49a38024c98 Reviewed-on: https://gerrit.libreoffice.org/29649 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp index 2cbfd88..2b3b205 100644 --- a/loolwsd/LOOLWSD.cpp +++ b/loolwsd/LOOLWSD.cpp @@ -432,7 +432,9 @@ private: auto uriPublic = DocumentBroker::sanitizeURI(fromPath); const auto docKey = DocumentBroker::getDocKey(uriPublic); + Log::debug("New DocumentBroker for docKey [" + docKey + "]."); auto docBroker = std::make_shared<DocumentBroker>(uriPublic, docKey, LOOLWSD::ChildRoot, child); + child->setDocumentBroker(docBroker); // This lock could become a bottleneck. // In that case, we can use a pool and index by publicPath. @@ -474,14 +476,24 @@ private: session->handleInput(saveas.data(), saveas.size()); // Send it back to the client. - Poco::URI resultURL(session->getSaveAsUrl(COMMAND_TIMEOUT_MS)); - if (!resultURL.getPath().empty()) + try + { + Poco::URI resultURL(session->getSaveAsUrl(COMMAND_TIMEOUT_MS)); + Log::trace("Save-as URL: " + resultURL.toString()); + + if (!resultURL.getPath().empty()) + { + const std::string mimeType = "application/octet-stream"; + std::string encodedFilePath; + URI::encode(resultURL.getPath(), "", encodedFilePath); + Log::trace("Sending file: " + encodedFilePath); + response.sendFile(encodedFilePath, mimeType); + sent = true; + } + } + catch (const std::exception& ex) { - const std::string mimeType = "application/octet-stream"; - std::string encodedFilePath; - URI::encode(resultURL.getPath(), "", encodedFilePath); - response.sendFile(encodedFilePath, mimeType); - sent = true; + Log::error(std::string("Failed to get save-as url: ") + ex.what()); } lock.lock(); @@ -495,6 +507,8 @@ private: { Log::error("Multiple sessions during conversion. " + std::to_string(sessionsCount) + " sessions remain."); } + + session->shutdownPeer(WebSocket::WS_NORMAL_CLOSE, ""); } // Clean up the temporary directory the HTMLForm ctor created. commit 366e0e21d5733808fda7080a013bbb6c4096b669 Author: Ashod Nakashian <[email protected]> Date: Sun Oct 9 11:59:00 2016 -0400 loolwsd: support timeout on MessageQueue get Change-Id: Iaad39aaa06c59cdacdd4a864599ef6a4a12976f8 Reviewed-on: https://gerrit.libreoffice.org/29648 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp index e9bc176..3be6277 100644 --- a/loolwsd/ClientSession.hpp +++ b/loolwsd/ClientSession.hpp @@ -39,9 +39,9 @@ public: * Return the URL of the saved-as document when it's ready. If called * before it's ready, the call blocks till then. */ - std::string getSaveAsUrl() + std::string getSaveAsUrl(const unsigned timeoutMs) { - const auto payload = _saveAsQueue.get(); + const auto payload = _saveAsQueue.get(timeoutMs); return std::string(payload.data(), payload.size()); } diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp index 1889987..2cbfd88 100644 --- a/loolwsd/LOOLWSD.cpp +++ b/loolwsd/LOOLWSD.cpp @@ -474,8 +474,7 @@ private: session->handleInput(saveas.data(), saveas.size()); // Send it back to the client. - //TODO: Should have timeout to avoid waiting forever. - Poco::URI resultURL(session->getSaveAsUrl()); + Poco::URI resultURL(session->getSaveAsUrl(COMMAND_TIMEOUT_MS)); if (!resultURL.getPath().empty()) { const std::string mimeType = "application/octet-stream"; diff --git a/loolwsd/MessageQueue.cpp b/loolwsd/MessageQueue.cpp index ffa5f79..3729018 100644 --- a/loolwsd/MessageQueue.cpp +++ b/loolwsd/MessageQueue.cpp @@ -32,10 +32,23 @@ void MessageQueue::put(const Payload& value) _cv.notify_one(); } -MessageQueue::Payload MessageQueue::get() +MessageQueue::Payload MessageQueue::get(const unsigned timeoutMs) { std::unique_lock<std::mutex> lock(_mutex); - _cv.wait(lock, [this] { return wait_impl(); }); + + if (timeoutMs > 0) + { + if (!_cv.wait_for(lock, std::chrono::milliseconds(timeoutMs), + [this] { return wait_impl(); })) + { + throw std::runtime_error("Timed out waiting to get queue item."); + } + } + else + { + _cv.wait(lock, [this] { return wait_impl(); }); + } + return get_impl(); } diff --git a/loolwsd/MessageQueue.hpp b/loolwsd/MessageQueue.hpp index 8ce2287..95a5654 100644 --- a/loolwsd/MessageQueue.hpp +++ b/loolwsd/MessageQueue.hpp @@ -43,7 +43,8 @@ public: } /// Thread safe obtaining of the message. - Payload get(); + /// timeoutMs can be 0 to signify infinity. + Payload get(const unsigned timeoutMs = 0); /// Thread safe removal of all the pending messages. void clear(); diff --git a/loolwsd/test/integration-http-server.cpp b/loolwsd/test/integration-http-server.cpp index 6899476..25aea88 100644 --- a/loolwsd/test/integration-http-server.cpp +++ b/loolwsd/test/integration-http-server.cpp @@ -243,6 +243,7 @@ void HTTPServerTest::testConvertTo() { const auto srcPath = Util::getTempFilePath(TDOC, "hello.odt"); std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri)); + session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds. Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to"); Poco::Net::HTMLForm form; commit 128cd73154894cc63cb16cc1d47c5b905cb1f7a6 Author: Ashod Nakashian <[email protected]> Date: Sun Oct 9 11:28:22 2016 -0400 loolwsd: send child messages to client via unified wsd-kit WS Change-Id: I237120e5a81a2e6d8772a2b6f1e98b1ba567f97e Reviewed-on: https://gerrit.libreoffice.org/29647 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp index e8e23ac..d8d68bd 100644 --- a/loolwsd/ChildSession.hpp +++ b/loolwsd/ChildSession.hpp @@ -50,6 +50,9 @@ public: std::mutex& getMutex() = 0; virtual std::shared_ptr<TileQueue>& getTileQueue() = 0; + + virtual + bool sendTextFrame(const std::string& message) = 0; }; /// Represents a session to the WSD process, in a Kit process. Note that this is not a singleton. @@ -77,6 +80,20 @@ public: void loKitCallback(const int nType, const std::string& rPayload); + bool sendTextFrame(const char* buffer, const int length) override + { + const auto msg = "client-" + getId() + ' ' + std::string(buffer, length); + + const auto lock = getLock(); + + return _docManager.sendTextFrame(msg); + } + + bool sendTextFrame(const std::string& text) + { + return sendTextFrame(text.data(), text.size()); + } + private: bool loadDocument(const char *buffer, int length, Poco::StringTokenizer& tokens); diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp index 7508802..e9bc176 100644 --- a/loolwsd/ClientSession.hpp +++ b/loolwsd/ClientSession.hpp @@ -30,6 +30,7 @@ public: bool isReadOnly() const { return _isReadOnly; } void setPeer(const std::shared_ptr<PrisonerSession>& peer) { _peer = peer; } + std::shared_ptr<PrisonerSession> getPeer() const { return _peer.lock(); } bool shutdownPeer(Poco::UInt16 statusCode, const std::string& message); void setUserName(const std::string& userName) { _userName = userName; } diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp index 3201170..02686fa 100644 --- a/loolwsd/DocumentBroker.cpp +++ b/loolwsd/DocumentBroker.cpp @@ -756,7 +756,15 @@ bool DocumentBroker::forwardToClient(const std::string& prefix, const std::vecto const auto it = _sessions.find(sid); if (it != _sessions.end()) { - return it->second->sendTextFrame(message); + const auto peer = it->second->getPeer(); + if (peer) + { + return peer->handleInput(message.data(), message.size()); + } + else + { + Log::warn() << "Client session [" << sid << "] has no peer to forward message: " << message << Log::end; + } } else { diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp index 66ec29c..d8ccee4 100644 --- a/loolwsd/DocumentBroker.hpp +++ b/loolwsd/DocumentBroker.hpp @@ -64,6 +64,7 @@ public: void setDocumentBroker(const std::shared_ptr<DocumentBroker>& docBroker) { + assert(docBroker && "Invalid DocumentBroker instance."); _docBroker = docBroker; } diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp index 65f8fa2..3bb689d 100644 --- a/loolwsd/LOOLKit.cpp +++ b/loolwsd/LOOLKit.cpp @@ -40,6 +40,7 @@ #include <Poco/Net/HTTPRequest.h> #include <Poco/Net/HTTPResponse.h> #include <Poco/Net/NetException.h> +#include <Poco/Net/Socket.h> #include <Poco/Net/WebSocket.h> #include <Poco/NotificationQueue.h> #include <Poco/Process.h> @@ -78,6 +79,7 @@ using Poco::Net::HTTPClientSession; using Poco::Net::HTTPRequest; using Poco::Net::HTTPResponse; using Poco::Net::NetException; +using Poco::Net::Socket; using Poco::Net::WebSocket; using Poco::Path; using Poco::Process; @@ -799,11 +801,34 @@ public: ws->sendFrame(response.data(), length, WebSocket::FRAME_BINARY); } - void sendTextFrame(const std::string& message) + bool sendTextFrame(const std::string& message) override { - std::lock_guard<std::mutex> lock(_mutex); + try + { + if (!_ws || _ws->poll(Poco::Timespan(0), Socket::SelectMode::SELECT_ERROR)) + { + Log::error("Child Doc: Bad socket while sending [" + getAbbreviatedMessage(message) + "]."); + return false; + } + + const auto length = message.size(); + if (length > SMALL_MESSAGE_SIZE) + { + const std::string nextmessage = "nextmessage: size=" + std::to_string(length); + _ws->sendFrame(nextmessage.data(), nextmessage.size()); + } - _ws->sendFrame(message.data(), message.size()); + _ws->sendFrame(message.data(), length); + return true; + } + catch (const Exception& exc) + { + Log::error() << "Document::sendTextFrame: " + << "Exception: " << exc.displayText() + << (exc.nested() ? "( " + exc.nested()->displayText() + ")" : ""); + } + + return false; } static void GlobalCallback(const int nType, const char* pPayload, void* pData) diff --git a/loolwsd/LOOLSession.hpp b/loolwsd/LOOLSession.hpp index 0606f4e..b07b2c6 100644 --- a/loolwsd/LOOLSession.hpp +++ b/loolwsd/LOOLSession.hpp @@ -42,7 +42,9 @@ public: const std::string& getName() const { return _name; } bool isDisconnected() const { return _disconnected; } + virtual bool sendBinaryFrame(const char *buffer, int length); + virtual bool sendTextFrame(const char* buffer, const int length); bool sendTextFrame(const std::string& text) { diff --git a/loolwsd/test/WhiteBoxTests.cpp b/loolwsd/test/WhiteBoxTests.cpp index d17cc22..45db538 100644 --- a/loolwsd/test/WhiteBoxTests.cpp +++ b/loolwsd/test/WhiteBoxTests.cpp @@ -192,6 +192,11 @@ public: { return _tileQueue; } + + bool sendTextFrame(const std::string& /*message*/) override + { + return true; + } }; void WhiteBoxTests::testEmptyCellCursor() commit 22a5db2fc09a123f9c00135f7fc44c4d257c7a9d Author: Ashod Nakashian <[email protected]> Date: Sun Oct 9 11:27:01 2016 -0400 loolwsd: trim forwarded messages to avoid leading whitespace Change-Id: I932baf3ec41789d89bf897fcbf25a1ee1d27f89d Reviewed-on: https://gerrit.libreoffice.org/29646 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp index 6c6cbb8..3201170 100644 --- a/loolwsd/DocumentBroker.cpp +++ b/loolwsd/DocumentBroker.cpp @@ -725,7 +725,7 @@ void DocumentBroker::setModified(const bool value) bool DocumentBroker::forwardToChild(const std::string& viewId, const char *buffer, int length) { const auto message = std::string(buffer, length); - Log::warn() << "Forwarding payload to child [" << viewId << "]: " << message << Log::end; + Log::trace() << "Forwarding payload to child [" << viewId << "]: " << message << Log::end; const auto it = _sessions.find(viewId); if (it != _sessions.end()) @@ -745,8 +745,9 @@ bool DocumentBroker::forwardToChild(const std::string& viewId, const char *buffe bool DocumentBroker::forwardToClient(const std::string& prefix, const std::vector<char>& payload) { - const std::string message(payload.data() + prefix.size(), payload.size() - prefix.size()); - Log::warn("Forwarding payload to client: " + message); + std::string message(payload.data() + prefix.size(), payload.size() - prefix.size()); + Util::ltrim(message); + Log::trace("Forwarding payload to " + prefix + ' ' + message); std::string name; std::string sid; diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp index 41d39fc..65f8fa2 100644 --- a/loolwsd/LOOLKit.cpp +++ b/loolwsd/LOOLKit.cpp @@ -1227,8 +1227,9 @@ private: bool forwardToChild(const std::string& prefix, const std::vector<char>& payload) { - const std::string message(payload.data() + prefix.size(), payload.size() - prefix.size()); - Log::trace("Forwarding payload to client: " + message); + std::string message(payload.data() + prefix.size(), payload.size() - prefix.size()); + Util::ltrim(message); + Log::trace("Forwarding payload to " + prefix + ' ' + message); std::string name; std::string value; diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp index bd1a5a3..df4f60c 100644 --- a/loolwsd/Util.hpp +++ b/loolwsd/Util.hpp @@ -153,6 +153,30 @@ namespace Util /// Return a string that is unique across processes and calls. std::string UniqueId(); + /// Trim spaces from the left. Just spaces. + inline + void ltrim(std::string& s) + { + const auto pos = s.find_first_not_of(" "); + if (pos != std::string::npos) + { + s = s.substr(pos); + } + } + + /// Trim spaces from the left and copy. Just spaces. + inline + std::string ltrimmed(const std::string& s) + { + const auto pos = s.find_first_not_of(" "); + if (pos != std::string::npos) + { + return s.substr(pos); + } + + return s; + } + /// Given one or more patterns to allow, and one or more to deny, /// the match member will return true if, and only if, the subject /// matches the allowed list, but not the deny. commit 0f2f49823ef02c9cebb32f32d4bcd4dd44e9351c Author: Ashod Nakashian <[email protected]> Date: Sat Oct 8 23:51:07 2016 -0400 loolwsd: unittest cleanup Change-Id: I90e4fa7d5377b7ecf427c87ce068efdb4bffe933 Reviewed-on: https://gerrit.libreoffice.org/29645 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp index a6f0654..6876218 100644 --- a/loolwsd/test/httpwstest.cpp +++ b/loolwsd/test/httpwstest.cpp @@ -338,21 +338,9 @@ void HTTPWSTest::loadDoc(const std::string& documentURL, const std::string& test // Don't replace with helpers, so we catch status. Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, documentURL); auto socket = connectLOKit(_uri, request, _response, testname); - sendTextFrame(*socket, "load url=" + documentURL, testname); + sendTextFrame(socket, "load url=" + documentURL, testname); - SocketProcessor(testname, socket, [&](const std::string& msg) - { - const std::string prefix = "status: "; - if (msg.find(prefix) == 0) - { - const auto status = msg.substr(prefix.length()); - // Might be too strict, consider something flexible instread. - CPPUNIT_ASSERT_EQUAL(std::string("type=text parts=1 current=0 width=12808 height=16408 viewid=0"), status); - return false; - } - - return true; - }); + assertResponseString(socket, "status:", testname); } catch (const Poco::Exception& exc) { _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
