loolwsd/LOOLProtocol.hpp | 24 ++++++++++++++++++++++++ loolwsd/test/helpers.hpp | 37 ++++++++++++------------------------- 2 files changed, 36 insertions(+), 25 deletions(-)
New commits: commit 452888060d73e1283fead168249998ba9d9c757e Author: Ashod Nakashian <[email protected]> Date: Fri Oct 21 12:59:03 2016 -0400 loolwsd: cleanup and improve getResponseMessage Proper parsing of incoming messages and better prefix matching is done, as well as slightly better logging when we timeout or hit exceptions. Change-Id: I11adbf24a5505f6cda4a18ba855898dc5f82e238 Reviewed-on: https://gerrit.libreoffice.org/30190 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/LOOLProtocol.hpp b/loolwsd/LOOLProtocol.hpp index e98d1ea..7fd0f4a 100644 --- a/loolwsd/LOOLProtocol.hpp +++ b/loolwsd/LOOLProtocol.hpp @@ -103,6 +103,30 @@ namespace LOOLProtocol return getFirstToken(message.data(), message.size(), delim); } + inline + bool matchPrefix(const std::string& prefix, const std::string& message) + { + return (message.size() >= prefix.size() && + message.compare(0, prefix.size(), prefix) == 0); + } + + inline + bool matchPrefix(const std::string& prefix, const std::string& message, const bool ignoreWhitespace) + { + if (ignoreWhitespace) + { + const auto posPre = prefix.find_first_not_of(' '); + const auto posMsg = message.find_first_not_of(' '); + + return matchPrefix(posPre == std::string::npos ? prefix : prefix.substr(posPre), + posMsg == std::string::npos ? message : message.substr(posMsg)); + } + else + { + return matchPrefix(prefix, message); + } + } + /// Returns true if the token is a user-interaction token. /// Currently this excludes commands sent automatically. /// Notice that this doesn't guarantee editing activity, diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp index cdc7515..cb96a87 100644 --- a/loolwsd/test/helpers.hpp +++ b/loolwsd/test/helpers.hpp @@ -186,42 +186,24 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string int bytes = ws.receiveFrame(response.data(), response.size(), flags); response.resize(bytes >= 0 ? bytes : 0); std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl; - auto message = LOOLProtocol::getAbbreviatedMessage(response); + const auto message = LOOLProtocol::getFirstLine(response); if (bytes > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE) { - // FIXME: This is wrong in two ways: - - // 1) The message variable is the return value from getAbbreviatedMessage(), - // That is a string that is intended for human-readable logging only. It is not - // intended to be used for actually decoding the protocol. Sure, at the moment - // it happens to work, but is still wrong. - - // 2) Using std::string::find() like this is silly. If message does not start - // with prefix, the find() function will search through the whole buffer. That - // is a waste of cycles. Use the LOOLProtocol functions to manipulate and - // inspect LOOL protocol frames, that is what they are there - // for. getFirstToken() should be quite efficient, and it doesn't incur the - // (potential) overhead of setting up a StringTokenizer. Or, if this is actually - // used to look for not just a first token, then introduce a startsWith() - // function. - - if (message.find(prefix) == 0) + if (LOOLProtocol::matchPrefix(prefix, message)) { return response; } - else if (message.find("nextmessage") == 0) + else if (LOOLProtocol::matchPrefix("nextmessage", message)) { - Poco::StringTokenizer tokens(message, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM); int size = 0; - if (tokens.count() == 2 && - tokens[0] == "nextmessage:" && LOOLProtocol::getTokenInteger(tokens[1], "size", size) && size > 0) + if (LOOLProtocol::getTokenIntegerFromMessage(message, "size", size) && size > 0) { response.resize(size); bytes = ws.receiveFrame(response.data(), response.size(), flags); response.resize(bytes >= 0 ? bytes : 0); std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl; - message = LOOLProtocol::getAbbreviatedMessage(response); - if (bytes > 0 && message.find(prefix) == 0) + if (bytes > 0 && + LOOLProtocol::matchPrefix(prefix, LOOLProtocol::getFirstLine(response))) { return response; } @@ -259,10 +241,15 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string } } while (retries > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE); + + if (timedout) + { + std::cerr << std::endl; + } } catch (const Poco::Net::WebSocketException& exc) { - std::cerr << exc.message(); + std::cerr << std::endl << exc.message(); } return std::vector<char>(); _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
