[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - wsd/LOOLWSD.cpp

[Ashod Nakashian]

 wsd/LOOLWSD.cpp |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

New commits:
commit 1890d2ee134ace63a28b05361724d5b8a56b67bb
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Apr 9 18:28:00 2017 -0400

    wsd: clear the incoming buffer before upgrading to WS
    
    There was an interesting race when we cleared the
    inBuffer after the WS upgrade. Since during the
    upgrade we also transfer the socket to the DocBroker,
    which has its own poll thread, the DocBroker poll
    could trigger a POLLIN event if data comes
    while the handler (that is handling the WS upgrad
    and transfer to DocBroker) hasn't got to the point
    where it clears the inBuffer of the data we just
    read (i.e. the HTTP GET request). Even if not
    the case, after transfering a socket to another
    poll thread the socket buffers should not be
    touched.
    
    Here we move the inBuffer clearing to be as soon
    as we have successfully parsed the request and
    are ready to process it.
    
    Also, we don't clear the full buffer, in case
    we had read into the buffer both the requst
    and the first message, if the thread was switched
    out right after getting the POLLIN but before
    reading from the socket, giving enough time to
    receive more data and reading it together with
    first read (which is the request).
    
    Change-Id: I9888d4c2b70d2e433824818bbe7f69f13742486c
    Reviewed-on: https://gerrit.libreoffice.org/36326
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>
    (cherry picked from commit 9a761ffe68b24e180dd74c0737e20cabe9dc9e1b)
    Reviewed-on: https://gerrit.libreoffice.org/36340
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index aa00d0ba..7b54bfd2 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1525,6 +1525,7 @@ private:
     {
         auto socket = _socket.lock();
         std::vector<char>& in = socket->_inBuffer;
+        LOG_TRC("#" << socket->getFD() << " handling incoming " << in.size() << " bytes.");
 
         // Find the end of the header, if any.
         static const std::string marker("\r\n\r\n");
@@ -1532,7 +1533,7 @@ private:
                                   marker.begin(), marker.end());
         if (itBody == in.end())
         {
-            LOG_TRC("#" << socket->getFD() << " doesn't have enough data yet.");
+            LOG_DBG("#" << socket->getFD() << " doesn't have enough data yet.");
             return SocketHandlerInterface::SocketOwnership::UNCHANGED;
         }
 
@@ -1570,6 +1571,10 @@ private:
                 LOG_DBG("Not enough content yet: ContentLength: " << contentLength << ", available: " << available);
                 return SocketHandlerInterface::SocketOwnership::UNCHANGED;
             }
+
+            // if we succeeded - remove the request from our input buffer
+            // we expect one request per socket
+            in.erase(in.begin(), itBody);
         }
         catch (const std::exception& exc)
         {
@@ -1626,7 +1631,8 @@ private:
                     // All post requests have url prefix 'lool'.
                     socketOwnership = handlePostRequest(request, message);
                 }
-                else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws")
+                else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws" &&
+                         request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0)
                 {
                     socketOwnership = handleClientWsUpgrade(request, reqPathTokens[1]);
                 }
@@ -1645,15 +1651,12 @@ private:
                     socket->shutdown();
                 }
             }
-
-            // if we succeeded - remove the request from our input buffer
-            // we expect one request per socket
-            in.clear();
         }
         catch (const std::exception& exc)
         {
             // TODO: Send back failure.
             // NOTE: Check _wsState to choose between HTTP response or WebSocket (app-level) error.
+            LOG_ERR("#" << socket->getFD() << " Exception while processing incoming request: [" << LOOLProtocol::getAbbreviatedMessage(in) << "]");
         }
 
         return socketOwnership;