[Libreoffice-commits] online.git: Branch 'distro/cib/libreoffice-6-2' - 4 commits - net/WebSocketHandler.hpp test/UnitWOPISaveAs.cpp test/WopiTestServer.hpp wsd/ClientSession.cpp

Miklos Vajna (via logerrit) logerrit at kemper.freedesktop.org
Tue Jul 30 06:11:41 UTC 2019


 net/WebSocketHandler.hpp |  271 ++++++++++++++++++++++++++++++++---------------
 test/UnitWOPISaveAs.cpp  |    6 -
 test/WopiTestServer.hpp  |    2 
 wsd/ClientSession.cpp    |   39 +++++-
 4 files changed, 220 insertions(+), 98 deletions(-)

New commits:
commit 6104e71d6a64108edd03a5e54508db7776beda07
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon May 27 09:06:38 2019 +0200
Commit:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Tue Jul 30 08:08:37 2019 +0200

    net: avoid UB in WebSocketHandler::readPayload()
    
    Seen when closing a Writer document.
    
    /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_vector.h:798:9: runtime error: reference binding to null pointer of type 'char'
        #0 0x6ff633 in std::vector<char, std::allocator<char> >::operator[](unsigned long) /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_vector.h:798:2
        #1 0x770d0c in WebSocketHandler::readPayload(unsigned char*, unsigned long, unsigned char*, std::vector<char, std::allocator<char> >&) /home/vmiklos/lode/dev/online/./net/WebSocketHandler.hpp:611:29
        #2 0x759324 in WebSocketHandler::handleTCPStream(std::shared_ptr<StreamSocket> const&) /home/vmiklos/lode/dev/online/./net/WebSocketHandler.hpp:251:13
        #3 0x6f820d in WebSocketHandler::handleIncomingMessage(SocketDisposition&) /home/vmiklos/lode/dev/online/./net/WebSocketHandler.hpp:419:20
        #4 0xb2da64 in ClientSession::handleIncomingMessage(SocketDisposition&) /home/vmiklos/lode/dev/online/wsd/ClientSession.cpp:74:14
        #5 0xa70a61 in StreamSocket::handlePoll(SocketDisposition&, std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l> > >, int) /home/vmiklos/lode/dev/online/./net/Socket.hpp:1037:29
        #6 0x6ec83d in SocketPoll::poll(int) /home/vmiklos/lode/dev/online/./net/Socket.hpp:570:34
        #7 0x830019 in DocumentBroker::pollThread() /home/vmiklos/lode/dev/online/wsd/DocumentBroker.cpp:286:16
        #8 0x8fdb38 in DocumentBroker::DocumentBrokerPoll::pollingThread() /home/vmiklos/lode/dev/online/wsd/DocumentBroker.cpp:165:20
        #9 0xe00e75 in SocketPoll::pollingThreadEntry() /home/vmiklos/lode/dev/online/net/Socket.cpp:184:9
        #10 0xe49cfd in void std::__invoke_impl<void, void (SocketPoll::*)(), SocketPoll*>(std::__invoke_memfun_deref, void (SocketPoll::*&&)(), SocketPoll*&&) /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/invoke.h:73:14
        #11 0xe4980a in std::__invoke_result<void (SocketPoll::*)(), SocketPoll*>::type std::__invoke<void (SocketPoll::*)(), SocketPoll*>(void (SocketPoll::*&&)(), SocketPoll*&&) /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/invoke.h:95:14
        #12 0xe496bd in decltype(std::__invoke(_S_declval<0ul>(), _S_declval<1ul>())) std::thread::_Invoker<std::tuple<void (SocketPoll::*)(), SocketPoll*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/thread:234:13
        #13 0xe494c7 in std::thread::_Invoker<std::tuple<void (SocketPoll::*)(), SocketPoll*> >::operator()() /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/thread:243:11
        #14 0xe4888a in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (SocketPoll::*)(), SocketPoll*> > >::_M_run() /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/thread:186:13
        #15 0x7f2c5805fe2e in execute_native_thread_routine /home/vmiklos/lode/packages/gccbuild/x86_64-pc-linux-gnu/libstdc++-v3/src/c++11/../../../../../gcc-7.3.0/libstdc++-v3/src/c++11/thread.cc:83
        #16 0x7f2c57a3c558 in start_thread (/lib64/libpthread.so.0+0x7558)
        #17 0x7f2c5715082e in clone (/lib64/libc.so.6+0xf882e)
    
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/vmiklos/lode/opt_private/gcc-7.3.0/lib/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_vector.h:798:9 in
    
    Change-Id: Ifaf6b193e9bba480587c2e184df55aa0728bb370
    Reviewed-on: https://gerrit.libreoffice.org/76331
    Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    Tested-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    (cherry picked from commit 43457a0aaf317c2c2c9594778aef891f58fc5827)

diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index e20ff5d5a..ad1547a8c 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -600,9 +600,12 @@ protected:
         {
             size_t end = payload.size();
             payload.resize(end + dataLen);
-            char* wsData = &payload[end];
-            for (size_t i = 0; i < dataLen; ++i)
-                *wsData++ = data[i] ^ mask[i % 4];
+            if (dataLen > 0)
+            {
+                char* wsData = &payload[end];
+                for (size_t i = 0; i < dataLen; ++i)
+                    *wsData++ = data[i] ^ mask[i % 4];
+            }
         }
         else
             payload.insert(payload.end(), data, data + dataLen);
commit 6513aab76da4c2249b41345ce1d399cf07f68991
Author:     Eduard Ardeleanu <eduard-andrei.ardeleanu at 1and1.ro>
AuthorDate: Tue Mar 19 16:00:38 2019 +0200
Commit:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Tue Jul 30 08:08:31 2019 +0200

    fix: saveAs breaks when '%' character is used within the filename
    
    Change-Id: I2df059abd67be88acae8bd44ae2c74be7778a595
    Reviewed-on: https://gerrit.libreoffice.org/69424
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/76329
    Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    Tested-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    (cherry picked from commit d54344b6ee0cab224c5184b1d4fe15edf733be77)

diff --git a/test/UnitWOPISaveAs.cpp b/test/UnitWOPISaveAs.cpp
index 7d9be30f9..4fd6733d9 100644
--- a/test/UnitWOPISaveAs.cpp
+++ b/test/UnitWOPISaveAs.cpp
@@ -34,7 +34,7 @@ public:
     void assertPutRelativeFileRequest(const Poco::Net::HTTPRequest& request) override
     {
         // spec says UTF-7...
-        CPPUNIT_ASSERT_EQUAL(std::string("/jan/hole+AWE-ovsk+AP0-/hello world.pdf"), request.get("X-WOPI-SuggestedTarget"));
+        CPPUNIT_ASSERT_EQUAL(std::string("/jan/hole+AWE-ovsk+AP0-/hello world+ACU-1.pdf"), request.get("X-WOPI-SuggestedTarget"));
 
         // make sure it is a pdf - or at least that it is larger than what it
         // used to be
@@ -44,7 +44,7 @@ public:
     bool filterSendMessage(const char* data, const size_t len, const WSOpCode /* code */, const bool /* flush */, int& /*unitReturn*/) override
     {
         const std::string message(data, len);
-        const std::string expected("saveas: url=" + helpers::getTestServerURI() + "/something%20wopi/files/1?access_token=anything filename=hello%20world.pdf");
+        const std::string expected("saveas: url=" + helpers::getTestServerURI() + "/something%20wopi/files/1?access_token=anything filename=hello%20world%251.pdf");
         if (message.find(expected) == 0)
         {
             // successfully exit the test if we also got the outgoing message
@@ -66,7 +66,7 @@ public:
                 initWebsocket("/wopi/files/0?access_token=anything");
 
                 helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "load url=" + _wopiSrc, testName);
-                helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "saveas url=wopi:///jan/hole%C5%A1ovsk%C3%BD/hello%20world.pdf", testName);
+                helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "saveas url=wopi:///jan/hole%C5%A1ovsk%C3%BD/hello%20world%251.pdf", testName);
                 SocketPoll::wakeupWorld();
 
                 _phase = Phase::Polling;
diff --git a/test/WopiTestServer.hpp b/test/WopiTestServer.hpp
index cc951f9c6..3e95c3b8b 100644
--- a/test/WopiTestServer.hpp
+++ b/test/WopiTestServer.hpp
@@ -166,7 +166,7 @@ protected:
             assertPutRelativeFileRequest(request);
 
             std::string wopiURL = helpers::getTestServerURI() + "/something wopi/files/1?access_token=anything";
-            std::string content = "{ \"Name\":\"hello world.pdf\", \"Url\":\"" + wopiURL + "\" }";
+            std::string content = "{ \"Name\":\"hello world%1.pdf\", \"Url\":\"" + wopiURL + "\" }";
 
             std::ostringstream oss;
             oss << "HTTP/1.1 200 OK\r\n"
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index b6bb60b20..102fc0325 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -816,12 +816,12 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
             return false;
         }
 
-        std::string url, wopiFilename;
-        Poco::URI::decode(encodedURL, url);
+        // Save-as completed, inform the ClientSession.
+        std::string wopiFilename;
         Poco::URI::decode(encodedWopiFilename, wopiFilename);
 
-        // Save-as completed, inform the ClientSession.
-        Poco::URI resultURL(url);
+        // URI constructor implicitly decodes when it gets std::string as param
+        Poco::URI resultURL(encodedURL);
         if (resultURL.getScheme() == "file")
         {
             std::string relative(resultURL.getPath());
@@ -832,7 +832,11 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
             const Path path(docBroker->getJailRoot(), relative);
             if (Poco::File(path).exists())
             {
-                resultURL.setPath(path.toString());
+                // Encode path for special characters (i.e '%') since Poco::URI::setPath implicitly decodes the input param
+                std::string encodedPath;
+                Poco::URI::encode(path.toString(), "", encodedPath);
+
+                resultURL.setPath(encodedPath);
             }
             else
             {
commit 5b1483301821b70d35f05c55e9e347d377bf3a42
Author:     Gabriel Masei <gabriel.masei at 1and1.ro>
AuthorDate: Fri Mar 8 10:21:17 2019 +0200
Commit:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Tue Jul 30 08:08:23 2019 +0200

    Added support for defragmentation of incoming websocket fragmented messages and handled some protocol error cases
    
    Change-Id: I4d11a6527b6b131c65101fd53b71015529645f74
    Reviewed-on: https://gerrit.libreoffice.org/68901
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/76327
    Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    Tested-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    (cherry picked from commit 1a9e34581c281b4d43c60aff2628bcc996fdc025)

diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index a037cb6b5..e20ff5d5a 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -37,6 +37,8 @@ private:
     std::atomic<bool> _shuttingDown;
     bool _isClient;
     bool _isMasking;
+    bool _inFragmentBlock;
+    bool _isManualDefrag;
 
 protected:
     struct WSFrameMask
@@ -50,16 +52,29 @@ protected:
 
 public:
     /// Perform upgrade ourselves, or select a client web socket.
-    WebSocketHandler(bool isClient = false, bool isMasking = true) :
+    /// Parameters:
+    /// isClient: the instance should behave like a client (true) or like a server (false)
+    ///           (from websocket perspective)
+    /// isMasking: a client should mask (true) or not (false) outgoing frames
+    /// isManualDefrag: the message handler should be called for every fragment of a message and
+    ///                 defragmentation should be handled inside message handler (true) or the message handler
+    ///                 should be called after all fragments of a message were received and the message
+    ///                 was defragmented (false).
+    WebSocketHandler(bool isClient = false, bool isMasking = true, bool isManualDefrag = false) :
         _lastPingSentTime(std::chrono::steady_clock::now()),
         _pingTimeUs(0),
         _shuttingDown(false),
         _isClient(isClient),
-        _isMasking(isClient && isMasking)
+        _isMasking(isClient && isMasking),
+        _inFragmentBlock(false),
+        _isManualDefrag(isManualDefrag)
     {
     }
 
     /// Upgrades itself to a websocket directly.
+    /// Parameters:
+    /// socket: the TCP socket which received the upgrade request
+    /// request: the HTTP upgrade request to WebSocket
     WebSocketHandler(const std::weak_ptr<StreamSocket>& socket,
                      const Poco::Net::HTTPRequest& request) :
         _socket(socket),
@@ -69,7 +84,9 @@ public:
         _pingTimeUs(0),
         _shuttingDown(false),
         _isClient(false),
-        _isMasking(false)
+        _isMasking(false),
+        _inFragmentBlock(false),
+        _isManualDefrag(false)
     {
         upgradeToWebSocket(request);
     }
@@ -99,8 +116,8 @@ public:
         RESERVED_TLS_FAILURE    = 1015
     };
 
-    /// Sends WS shutdown message to the peer.
-    void shutdown(const StatusCodes statusCode = StatusCodes::NORMAL_CLOSE, const std::string& statusMessage = "")
+    /// Sends WS Close frame to the peer.
+    void sendCloseFrame(const StatusCodes statusCode = StatusCodes::NORMAL_CLOSE, const std::string& statusMessage = "")
     {
         std::shared_ptr<StreamSocket> socket = _socket.lock();
         if (socket == nullptr)
@@ -126,7 +143,22 @@ public:
 #endif
     }
 
-    bool handleOneIncomingMessage(const std::shared_ptr<StreamSocket>& socket)
+    void shutdown(const StatusCodes statusCode = StatusCodes::NORMAL_CLOSE, const std::string& statusMessage = "")
+    {
+        if (!_shuttingDown)
+            sendCloseFrame(statusCode, statusMessage);
+        std::shared_ptr<StreamSocket> socket = _socket.lock();
+        if (socket)
+        {
+            socket->closeConnection();
+            socket->getInBuffer().clear();
+        }
+        _wsPayload.clear();
+        _inFragmentBlock = false;
+        _shuttingDown = false;
+    }
+
+    bool handleTCPStream(const std::shared_ptr<StreamSocket>& socket)
     {
         assert(socket && "Expected a valid socket instance.");
 
@@ -177,7 +209,7 @@ public:
             headerLen += 8;
         }
 
-        unsigned char *data, *mask;
+        unsigned char *data, *mask = nullptr;
 
         if (hasMask)
         {
@@ -187,117 +219,165 @@ public:
 
         if (payloadLen + headerLen > len)
         { // partial read wait for more data.
-            LOG_TRC("#" << socket->getFD() << ": Still incomplete WebSocket message, have " << len << " bytes, message is " << payloadLen + headerLen << " bytes");
+            LOG_TRC("#" << socket->getFD() << ": Still incomplete WebSocket frame, have " << len << " bytes, frame is " << payloadLen + headerLen << " bytes");
             return false;
         }
 
+        if (hasMask && _isClient)
+        {
+            LOG_ERR("#" << socket->getFD() << ": Servers should not send masked frames. Only clients.");
+            shutdown(StatusCodes::PROTOCOL_ERROR);
+            return true;
+        }
+
         LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket data of " << len << " bytes: " << Util::stringifyHexLine(socket->getInBuffer(), 0, std::min((size_t)32, len)));
 
         data = p + headerLen;
 
-        if (hasMask)
+        if (isControlFrame(code))
         {
-            const size_t end = _wsPayload.size();
-            _wsPayload.resize(end + payloadLen);
-            char* wsData = &_wsPayload[end];
-            for (size_t i = 0; i < payloadLen; ++i)
-                *wsData++ = data[i] ^ mask[i % 4];
-        } else
-            _wsPayload.insert(_wsPayload.end(), data, data + payloadLen);
-#else
-        unsigned char * const p = reinterpret_cast<unsigned char*>(&socket->getInBuffer()[0]);
-        _wsPayload.insert(_wsPayload.end(), p, p + len);
-        const size_t headerLen = 0;
-        const size_t payloadLen = len;
-#endif
-
-        assert(_wsPayload.size() >= payloadLen);
-
-        socket->getInBuffer().erase(socket->getInBuffer().begin(), socket->getInBuffer().begin() + headerLen + payloadLen);
+            //Process control frames
 
-#ifndef MOBILEAPP
+            std::vector<char> ctrlPayload;
 
-        // FIXME: fin, aggregating payloads into _wsPayload etc.
-        LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " << static_cast<unsigned>(code) <<
-                ", fin? " << fin << ", mask? " << hasMask << ", payload length: " << _wsPayload.size() <<
+            readPayload(data, payloadLen, mask, ctrlPayload);
+            socket->getInBuffer().erase(socket->getInBuffer().begin(), socket->getInBuffer().begin() + headerLen + payloadLen);
+            LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket frame code " << static_cast<unsigned>(code) <<
+                ", fin? " << fin << ", mask? " << hasMask << ", payload length: " << payloadLen <<
                 ", residual socket data: " << socket->getInBuffer().size() << " bytes.");
 
-        bool doClose = false;
-
-        switch (code)
-        {
-        case WSOpCode::Pong:
-        {
-            if (_isClient)
+            // All control frames MUST NOT be fragmented and MUST have a payload length of 125 bytes or less
+            if (!fin)
             {
-                LOG_ERR("#" << socket->getFD() << ": Servers should not send pongs, only clients");
-                doClose = true;
-                break;
+                LOG_ERR("#" << socket->getFD() << ": A control frame cannot be fragmented.");
+                shutdown(StatusCodes::PROTOCOL_ERROR);
+                return true;
             }
-            else
+            if (payloadLen > 125)
             {
-                _pingTimeUs = std::chrono::duration_cast<std::chrono::microseconds>
-                    (std::chrono::steady_clock::now() - _lastPingSentTime).count();
-                LOG_TRC("#" << socket->getFD() << ": Pong received: " << _pingTimeUs << " microseconds");
-                break;
+                LOG_ERR("#" << socket->getFD() << ": The payload length of a control frame must not exceed 125 bytes.");
+                shutdown(StatusCodes::PROTOCOL_ERROR);
+                return true;
             }
-        }
-        case WSOpCode::Ping:
-            if (_isClient)
+
+            switch (code)
             {
-                auto now = std::chrono::steady_clock::now();
-                _pingTimeUs = std::chrono::duration_cast<std::chrono::microseconds>
-                                        (now - _lastPingSentTime).count();
-                sendPong(now, &_wsPayload[0], payloadLen, socket);
+            case WSOpCode::Pong:
+                if (_isClient)
+                {
+                    LOG_ERR("#" << socket->getFD() << ": Servers should not send pongs, only clients");
+                    shutdown(StatusCodes::POLICY_VIOLATION);
+                    return true;
+                }
+                else
+                {
+                    _pingTimeUs = std::chrono::duration_cast<std::chrono::microseconds>
+                        (std::chrono::steady_clock::now() - _lastPingSentTime).count();
+                    LOG_TRC("#" << socket->getFD() << ": Pong received: " << _pingTimeUs << " microseconds");
+                }
+                break;
+            case WSOpCode::Ping:
+                if (_isClient)
+                {
+                    auto now = std::chrono::steady_clock::now();
+                    _pingTimeUs = std::chrono::duration_cast<std::chrono::microseconds>
+                                            (now - _lastPingSentTime).count();
+                    sendPong(now, &ctrlPayload[0], payloadLen, socket);
+                }
+                else
+                {
+                    LOG_ERR("#" << socket->getFD() << ": Clients should not send pings, only servers");
+                    shutdown(StatusCodes::POLICY_VIOLATION);
+                    return true;
+                }
+                break;
+            case WSOpCode::Close:
+                {
+                    std::string message;
+                    StatusCodes statusCode = StatusCodes::NORMAL_CLOSE;
+                    if (!_shuttingDown)
+                    {
+                        // Peer-initiated shutdown must be echoed.
+                        // Otherwise, this is the echo to _our_ shutdown message, which we should ignore.
+                        LOG_TRC("#" << socket->getFD() << ": Peer initiated socket shutdown. Code: " << static_cast<int>(statusCode));
+                        if (ctrlPayload.size())
+                        {
+                            statusCode = static_cast<StatusCodes>((((uint64_t)(unsigned char)ctrlPayload[0]) << 8) +
+                                                                (((uint64_t)(unsigned char)ctrlPayload[1]) << 0));
+                            if (ctrlPayload.size() > 2)
+                                message.assign(&ctrlPayload[2], &ctrlPayload[2] + ctrlPayload.size() - 2);
+                        }
+                    }
+                    shutdown(statusCode, message);
+                    return true;
+                }
+            default:
+                LOG_ERR("#" << socket->getFD() << ": Received unknown control code");
+                shutdown(StatusCodes::PROTOCOL_ERROR);
                 break;
             }
-            else
+
+            return true;
+        }
+
+        // Check data frames for errors
+        if (_inFragmentBlock)
+        {
+            if (code != WSOpCode::Continuation)
             {
-                LOG_ERR("#" << socket->getFD() << ": Clients should not send pings, only servers");
-                doClose = true;
+                LOG_ERR("#" << socket->getFD() << ": A fragment that is not the first fragment of a message must have the opcode equal to 0.");
+                shutdown(StatusCodes::PROTOCOL_ERROR);
+                return true;
             }
-            break;
-        case WSOpCode::Close:
-            doClose = true;
-            break;
-        default:
-            handleMessage(fin, code, _wsPayload);
-            break;
+        }
+        else if (code == WSOpCode::Continuation)
+        {
+            LOG_ERR("#" << socket->getFD() << ": An unfragmented message or the first fragment of a fragmented message must have the opcode different than 0.");
+            shutdown(StatusCodes::PROTOCOL_ERROR);
+            return true;
         }
 
+        //Process data frame
+        readPayload(data, payloadLen, mask, _wsPayload);
 #else
-        handleMessage(true, WSOpCode::Binary, _wsPayload);
-
+        unsigned char * const p = reinterpret_cast<unsigned char*>(&socket->getInBuffer()[0]);
+        _wsPayload.insert(_wsPayload.end(), p, p + len);
+        const size_t headerLen = 0;
+        const size_t payloadLen = len;
 #endif
 
-#ifndef MOBILEAPP
-        if (doClose)
+        socket->getInBuffer().erase(socket->getInBuffer().begin(), socket->getInBuffer().begin() + headerLen + payloadLen);
+
+#if !MOBILEAPP
+
+        LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket frame code " << static_cast<unsigned>(code) <<
+                ", fin? " << fin << ", mask? " << hasMask << ", payload length: " << payloadLen <<
+                ", residual socket data: " << socket->getInBuffer().size() << " bytes.");
+
+        if (fin)
+        {
+            //If is final fragment then process the accumulated message.
+            handleMessage(fin, code, _wsPayload);
+            _inFragmentBlock = false;
+        }
+        else
         {
-            if (!_shuttingDown)
+            if (_isManualDefrag)
             {
-                // Peer-initiated shutdown must be echoed.
-                // Otherwise, this is the echo to _our_ shutdown message, which we should ignore.
-                const StatusCodes statusCode = static_cast<StatusCodes>((((uint64_t)(unsigned char)_wsPayload[0]) << 8) +
-                                                                        (((uint64_t)(unsigned char)_wsPayload[1]) << 0));
-                LOG_TRC("#" << socket->getFD() << ": Client initiated socket shutdown. Code: " << static_cast<int>(statusCode));
-                if (_wsPayload.size() > 2)
-                {
-                    const std::string message(&_wsPayload[2], &_wsPayload[2] + _wsPayload.size() - 2);
-                    shutdown(statusCode, message);
-                }
-                else
-                {
-                    shutdown(statusCode);
-                }
+                //If the user wants to process defragmentation on its own then let him process it.
+                handleMessage(fin, code, _wsPayload);
+                _inFragmentBlock = true;
             }
             else
             {
-                LOG_TRC("#" << socket->getFD() << ": Client responded to our shutdown.");
+                _inFragmentBlock = true;
+                //If is not final fragment then wait for next fragment.
+                return false;
             }
-
-            // TCP Close.
-            socket->closeConnection();
         }
+#else
+        handleMessage(true, WSOpCode::Binary, _wsPayload);
+
 #endif
 
         _wsPayload.clear();
@@ -328,7 +408,7 @@ public:
 #endif
         else
         {
-            while (handleOneIncomingMessage(socket))
+            while (handleTCPStream(socket))
                 ; // might have multiple messages in the accumulated buffer.
         }
     }
@@ -512,6 +592,22 @@ private:
 
 protected:
 
+    bool isControlFrame(WSOpCode code){ return code >= WSOpCode::Close; }
+
+    void readPayload(unsigned char *data, size_t dataLen, unsigned char* mask, std::vector<char>& payload)
+    {
+        if (mask)
+        {
+            size_t end = payload.size();
+            payload.resize(end + dataLen);
+            char* wsData = &payload[end];
+            for (size_t i = 0; i < dataLen; ++i)
+                *wsData++ = data[i] ^ mask[i % 4];
+        }
+        else
+            payload.insert(payload.end(), data, data + dataLen);
+    }
+
     /// To be overriden to handle the websocket messages the way you need.
     virtual void handleMessage(bool /*fin*/, WSOpCode /*code*/, std::vector<char> &/*data*/)
     {
commit a05b144ae8b822d072ae18fa7b0f1a0e5aada31d
Author:     Eduard Ardeleanu <eduard-andrei.ardeleanu at 1and1.ro>
AuthorDate: Fri May 24 09:57:06 2019 +0300
Commit:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Tue Jul 30 08:08:15 2019 +0200

    detecting password protected files on convertTo
    
    Fail-fast when a file cannot be converted, using convertTo REST API, if the file is password protected and the password wasn't received.
    
    Change-Id: I32d807bcecbbe72a38a70fec74caf13638803e1d
    Reviewed-on: https://gerrit.libreoffice.org/72891
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/76330
    Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    Tested-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    (cherry picked from commit 5fe0a1455c95e5e23699801b951a000e5f8439e6)

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 8f6186a5e..b6bb60b20 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -680,6 +680,9 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
         return false;
     }
 
+
+    const bool isConvertTo = static_cast<bool>(_saveAsSocket);
+
 #ifndef MOBILEAPP
     LOOLWSD::dumpOutgoingTrace(docBroker->getJailId(), getId(), firstLine);
 #endif
@@ -736,13 +739,30 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
                     errorKind == "passwordrequired:to-modify" ||
                     errorKind == "wrongpassword")
                 {
-                    forwardToClient(payload);
+                    if (isConvertTo)
+                    {
+                        Poco::Net::HTTPResponse response;
+                        response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_UNAUTHORIZED);
+                        response.set("X-ERROR-KIND", errorKind);
+                        _saveAsSocket->send(response);
+
+                        // Conversion failed, cleanup fake session.
+                        LOG_TRC("Removing save-as ClientSession after conversion error.");
+                        // Remove us.
+                        docBroker->removeSession(getId());
+                        // Now terminate.
+                        docBroker->stop("Aborting saveas handler.");
+                    }
+                    else
+                    {
+                        forwardToClient(payload);
+                    }
                     return false;
                 }
             }
             else
             {
-                LOG_WRN("Other than load failure: " << errorKind);
+                LOG_WRN(errorCommand << " error failure: " << errorKind);
             }
         }
     }
@@ -774,7 +794,6 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
 #ifndef MOBILEAPP
     else if (tokens.size() == 3 && tokens[0] == "saveas:")
     {
-        bool isConvertTo = static_cast<bool>(_saveAsSocket);
 
         std::string encodedURL;
         if (!getTokenString(tokens[1], "url", encodedURL))


More information about the Libreoffice-commits mailing list