[Libreoffice-commits] online.git: loolwsd/Common.hpp loolwsd/LOOLSession.cpp loolwsd/LOOLWSD.cpp

[Pranav Kant]

 loolwsd/Common.hpp      |    5 +++++
 loolwsd/LOOLSession.cpp |   11 +++++++++--
 loolwsd/LOOLWSD.cpp     |   43 +++++++++++++++++++------------------------
 3 files changed, 33 insertions(+), 26 deletions(-)

New commits:
commit 7ffd4b45dba12c01c5bd076ebb7080367462d6cd
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Wed Jan 27 14:35:54 2016 +0530

    bccu#1468: handle 'nextmessage:' in prisoner -> master text frames
    
    There are cases when prisoner would send huge text data but
    without preceeding 'nextmessage' frames making the master throw
    WebSocketException.
    
    Also move the 'nextmessage' frame interpretation logic up in the
    'else if' ladder to be able to detect and handle such huge text frames.
    
    Change-Id: Ibe44b69f4ab75c1b8096648c6006316c88366e7c
    Reviewed-on: https://gerrit.libreoffice.org/21835
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/Common.hpp b/loolwsd/Common.hpp
index 57903f2..daec8cb 100644
--- a/loolwsd/Common.hpp
+++ b/loolwsd/Common.hpp
@@ -27,6 +27,11 @@ constexpr int POLL_TIMEOUT_MS = 1000;
 /// Should be large enough for ethernet packets
 /// which can be 1500 bytes long.
 constexpr int READ_BUFFER_SIZE = 2048;
+/// Size after which messages will be sent preceded with
+/// 'nextmessage' frame to let the receiver know in advance
+/// the size of larger coming message. All messages up to this
+/// size are considered small messages.
+constexpr int SMALL_MESSAGE_SIZE = READ_BUFFER_SIZE / 2;
 
 static const std::string JailedDocumentRoot = "/user/docs/";
 
diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index 821835d..9a59292 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -50,6 +50,7 @@
 #include <Poco/Net/SocketAddress.h>
 #include <Poco/FileStream.h>
 
+#include "Common.hpp"
 #include "LOOLProtocol.hpp"
 #include "LOOLSession.hpp"
 #include "TileCache.hpp"
@@ -112,8 +113,14 @@ void LOOLSession::sendTextFrame(const std::string& text)
         Log::trace(getName() + " Send: " + getAbbreviatedMessage(text.c_str(), text.size()));
 
     std::unique_lock<std::mutex> lock(_mutex);
+    const int length = text.size();
+    if ( length > SMALL_MESSAGE_SIZE )
+    {
+        const std::string nextmessage = "nextmessage: size=" + std::to_string(length);
+        _ws->sendFrame(nextmessage.data(), nextmessage.size());
+    }
 
-    _ws->sendFrame(text.data(), text.size());
+    _ws->sendFrame(text.data(), length);
 }
 
 void LOOLSession::sendBinaryFrame(const char *buffer, int length)
@@ -128,7 +135,7 @@ void LOOLSession::sendBinaryFrame(const char *buffer, int length)
 
     std::unique_lock<std::mutex> lock(_mutex);
 
-    if (length > 1000)
+    if ( length > SMALL_MESSAGE_SIZE )
     {
         const std::string nextmessage = "nextmessage: size=" + std::to_string(length);
         _ws->sendFrame(nextmessage.data(), nextmessage.size());
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index da474c9..b5ef7e0 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -239,6 +239,9 @@ void SocketProcessor(std::shared_ptr<WebSocket> ws,
                 {
                     assert(n > 0);
                     const std::string firstLine = getFirstLine(buffer, n);
+                    StringTokenizer tokens(firstLine, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
+                    int size;
+
                     if (firstLine == "eof")
                     {
                         Log::info("Received EOF. Finishing.");
@@ -263,38 +266,30 @@ void SocketProcessor(std::shared_ptr<WebSocket> ws,
                             }
                         }
                     }
-                    else if (firstLine.size() == static_cast<std::string::size_type>(n))
-                    {
-                        handler(firstLine.c_str(), firstLine.size(), true);
-                    }
-                    else
+                    else if (tokens.count() == 2 &&
+                             tokens[0] == "nextmessage:" && getTokenInteger(tokens[1], "size", size) && size > 0)
                     {
                         // Check if it is a "nextmessage:" and in that case read the large
                         // follow-up message separately, and handle that only.
-                        StringTokenizer tokens(firstLine, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
 
-                        int size;
-                        if (tokens.count() == 2 &&
-                            tokens[0] == "nextmessage:" && getTokenInteger(tokens[1], "size", size) && size > 0)
-                        {
-                            char largeBuffer[size];     //FIXME: Security risk! Flooding may segfault us.
+                        char largeBuffer[size];     //FIXME: Security risk! Flooding may segfault us.
 
-                            n = ws->receiveFrame(largeBuffer, size, flags);
-                            if (n > 0 && !handler(largeBuffer, n, false))
-                            {
-                                Log::info("Socket handler flagged for finishing.");
-                                break;
-                            }
-                        }
-                        else
+                        n = ws->receiveFrame(largeBuffer, size, flags);
+                        if (n > 0 && !handler(largeBuffer, n, false))
                         {
-                            if (!handler(buffer, n, false))
-                            {
-                                Log::info("Socket handler flagged for finishing.");
-                                break;
-                            }
+                            Log::info("Socket handler flagged for finishing.");
+                            break;
                         }
                     }
+                    else if (firstLine.size() == static_cast<std::string::size_type>(n))
+                    {
+                        handler(firstLine.c_str(), firstLine.size(), true);
+                    }
+                    else if (!handler(buffer, n, false))
+                    {
+                        Log::info("Socket handler flagged for finishing.");
+                        break;
+                    }
                 }
             }
         }