[Libreoffice-commits] online.git: 7 commits - net/Socket.cpp net/Socket.hpp net/SslSocket.hpp net/WebSocketHandler.hpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/LOOLWSD.cpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Wed Mar 15 04:05:34 UTC 2017


 net/Socket.cpp           |    3 
 net/Socket.hpp           |   57 +++++---
 net/SslSocket.hpp        |    6 
 net/WebSocketHandler.hpp |   21 +--
 wsd/ClientSession.cpp    |   60 +++++++++
 wsd/ClientSession.hpp    |    7 -
 wsd/LOOLWSD.cpp          |  300 ++++++++++++++++++++++-------------------------
 7 files changed, 261 insertions(+), 193 deletions(-)

New commits:
commit 969fcbefb3173b3e4b1700956f21787322c176f4
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Mar 14 23:58:49 2017 -0400

    wsd: move convert-to handling into ConvertToHandler
    
    ClientSession still isn't getting the notification,
    so document is not uploaded to the client just yet.
    
    Change-Id: Ifda8ed394f6df1ec1a5bc1975d296dea496c0aed

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index e5f6ece..056b2e2 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1618,6 +1618,92 @@ private:
     }
 };
 
+/// Handles dispatching socket traffic to the ClientSession.
+class ConvertToHandler : public SocketHandlerInterface
+{
+public:
+    ConvertToHandler(const std::shared_ptr<ClientSession>& clientSession) :
+        _clientSession(clientSession)
+    {
+    }
+
+private:
+
+    /// Set the socket associated with this ResponseClient.
+    void onConnect(const std::weak_ptr<StreamSocket>& socket) override
+    {
+        LOG_ERR("onConnect");
+        _socket = socket;
+    }
+
+    void onDisconnect() override
+    {
+        LOG_ERR("onDisconnect");
+    }
+
+    void handleIncomingMessage() override
+    {
+        LOG_ERR("handleIncomingMessage");
+    }
+
+    bool hasQueuedWrites() const override
+    {
+        LOG_ERR("hasQueuedWrites");
+        return true;
+    }
+
+    void performWrites() override
+    {
+        LOG_ERR("performWrites");
+        auto socket = _socket.lock();
+
+        // Send it back to the client.
+        try
+        {
+            Poco::URI resultURL(_clientSession->getSaveAsUrl(COMMAND_TIMEOUT_MS));
+            LOG_TRC("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_TRC("Sending file: " << encodedFilePath);
+                HttpHelper::sendFile(socket, encodedFilePath, mimeType);
+            }
+        }
+        catch (const std::exception& ex)
+        {
+            LOG_ERR("Failed to get save-as url: " << ex.what());
+        }
+
+        // auto docLock = docBroker->getLock();
+        // sessionsCount = docBroker->removeSession(_id);
+        // if (sessionsCount == 0)
+        // {
+        //     // At this point we're done.
+        //     LOG_DBG("Removing DocumentBroker for docKey [" << docKey << "].");
+        //     DocBrokers.erase(docKey);
+        //     docBroker->terminateChild(docLock, "");
+        //     LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after removing [" << docKey << "].");
+        // }
+        // else
+        // {
+        //     LOG_ERR("Multiple sessions during conversion. " << sessionsCount << " sessions remain.");
+        // }
+
+        // Clean up the temporary directory the HTMLForm ctor created.
+        // Path tempDirectory(fromPath);
+        // tempDirectory.setFileName("");
+        // FileUtil::removeFile(tempDirectory, /*recursive=*/true);
+    }
+
+private:
+    // The socket that owns us (we can't own it).
+    std::weak_ptr<StreamSocket> _socket;
+    std::shared_ptr<ClientSession> _clientSession;
+};
+
 /// Handles incoming connections and dispatches to the appropriate handler.
 class ClientRequestDispatcher : public SocketHandlerInterface
 {
@@ -1954,75 +2040,45 @@ private:
                     LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << "].");
 
                     // Load the document.
-                    auto session = std::make_shared<ClientSession>(_id, docBroker, uriPublic);
-
-                    auto lock = docBroker->getLock();
-                    size_t sessionsCount = docBroker->queueSession(session);
-                    lock.unlock();
-                    LOG_TRC(docKey << ", ws_sessions++: " << sessionsCount);
-
-                    docBrokersLock.unlock();
-
-                    std::string encodedFrom;
-                    URI::encode(docBroker->getPublicUri().getPath(), "", encodedFrom);
-                    const std::string load = "load url=" + encodedFrom;
-                    std::vector<char> loadRequest(load.begin(), load.end());
-                    session->handleMessage(true, WebSocketHandler::WSOpCode::Text, loadRequest);
-
-                    // FIXME: Check for security violations.
-                    Path toPath(docBroker->getPublicUri().getPath());
-                    toPath.setExtension(format);
-                    const std::string toJailURL = "file://" + std::string(JAILED_DOCUMENT_ROOT) + toPath.getFileName();
-                    std::string encodedTo;
-                    URI::encode(toJailURL, "", encodedTo);
-
-                    // Convert it to the requested format.
-                    const auto saveas = "saveas url=" + encodedTo + " format=" + format + " options=";
-                    std::vector<char> saveasRequest(saveas.begin(), saveas.end());
-                    session->handleMessage(true, WebSocketHandler::WSOpCode::Text, saveasRequest);
-
-                    // Send it back to the client.
-                    try
-                    {
-                        Poco::URI resultURL(session->getSaveAsUrl(COMMAND_TIMEOUT_MS));
-                        LOG_TRC("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_TRC("Sending file: " << encodedFilePath);
-                            HttpHelper::sendFile(socket, encodedFilePath, mimeType);
-                            sent = true;
-                        }
-                    }
-                    catch (const std::exception& ex)
+                    // TODO: Move to DocumentBroker.
+                    const bool isReadOnly = true;
+                    auto clientSession = createNewClientSession(nullptr, _id, uriPublic, docBroker, isReadOnly);
+                    if (clientSession)
                     {
-                        LOG_ERR("Failed to get save-as url: " << ex.what());
-                    }
-
-                    docBrokersLock.lock();
-                    auto docLock = docBroker->getLock();
-                    sessionsCount = docBroker->removeSession(_id);
-                    if (sessionsCount == 0)
-                    {
-                        // At this point we're done.
-                        LOG_DBG("Removing DocumentBroker for docKey [" << docKey << "].");
-                        DocBrokers.erase(docKey);
-                        docBroker->terminateChild(docLock, "");
-                        LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after removing [" << docKey << "].");
+                        // Transfer the client socket to the DocumentBroker.
+                        // Move the socket into DocBroker.
+                        WebServerPoll.releaseSocket(socket);
+                        docBroker->addSocketToPoll(socket);
+
+                        auto convertToHandler = std::make_shared<ConvertToHandler>(clientSession);
+
+                        // Set the ConvertToHandler to handle Socket events.
+                        socket->setHandler(convertToHandler);
+                        docBroker->startThread();
+
+                        std::string encodedFrom;
+                        URI::encode(docBroker->getPublicUri().getPath(), "", encodedFrom);
+                        const std::string load = "load url=" + encodedFrom;
+                        std::vector<char> loadRequest(load.begin(), load.end());
+                        clientSession->handleMessage(true, WebSocketHandler::WSOpCode::Text, loadRequest);
+
+                        // FIXME: Check for security violations.
+                        Path toPath(docBroker->getPublicUri().getPath());
+                        toPath.setExtension(format);
+                        const std::string toJailURL = "file://" + std::string(JAILED_DOCUMENT_ROOT) + toPath.getFileName();
+                        std::string encodedTo;
+                        URI::encode(toJailURL, "", encodedTo);
+
+                        // Convert it to the requested format.
+                        const auto saveas = "saveas url=" + encodedTo + " format=" + format + " options=";
+                        std::vector<char> saveasRequest(saveas.begin(), saveas.end());
+                        clientSession->handleMessage(true, WebSocketHandler::WSOpCode::Text, saveasRequest);
+
+                        sent = true;
                     }
                     else
-                    {
-                        LOG_ERR("Multiple sessions during conversion. " << sessionsCount << " sessions remain.");
-                    }
+                        LOG_WRN("Failed to create Client Session with id [" << _id << "] on docKey [" << docKey << "].");
                 }
-
-                // Clean up the temporary directory the HTMLForm ctor created.
-                Path tempDirectory(fromPath);
-                tempDirectory.setFileName("");
-                FileUtil::removeFile(tempDirectory, /*recursive=*/true);
             }
 
             if (!sent)
commit 1288837401d2f8b8c5cf85f7885c3271f55e338a
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Mar 14 23:24:28 2017 -0400

    wsd: createNewClientSession takes optional WS to notify client of progress
    
    Change-Id: Ief64685dd8cc363cfd21cedda38f907b6787b609

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 0d8a5fb..e5f6ece 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1396,7 +1396,7 @@ static void removeDocBrokerSession(const std::shared_ptr<DocumentBroker>& docBro
     }
 }
 
-static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHandler& ws,
+static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHandler* ws,
                                                              const std::string& id,
                                                              const Poco::URI& uriPublic,
                                                              const std::shared_ptr<DocumentBroker>& docBroker,
@@ -1411,9 +1411,12 @@ static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHand
             LOG_WRN("DocBroker is marked to destroy, attempting to add session anyway.");
 
         // Now we have a DocumentBroker and we're ready to process client commands.
-        const std::string statusReady = "statusindicator: ready";
-        LOG_TRC("Sending to Client [" << statusReady << "].");
-        ws.sendFrame(statusReady);
+        if (ws)
+        {
+            const std::string statusReady = "statusindicator: ready";
+            LOG_TRC("Sending to Client [" << statusReady << "].");
+            ws->sendFrame(statusReady);
+        }
 
         // In case of WOPI, if this session is not set as readonly, it might be set so
         // later after making a call to WOPI host which tells us the permission on files
@@ -2193,7 +2196,7 @@ private:
         if (docBroker)
         {
             // TODO: Move to DocumentBroker.
-            auto clientSession = createNewClientSession(ws, _id, uriPublic, docBroker, isReadOnly);
+            auto clientSession = createNewClientSession(&ws, _id, uriPublic, docBroker, isReadOnly);
             if (clientSession)
             {
                 // Transfer the client socket to the DocumentBroker.
commit 6c8f3633dee4b551ca420d2c570e7ab7bf2a45e6
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Mar 14 22:19:51 2017 -0400

    wsd: ClientSession becomes SocketHandler for WS sockets
    
    Change-Id: I02706ccac186e4934b8ccdab5cdebdba7170fd46

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 467a514..75e16f5 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -678,4 +678,64 @@ bool ClientSession::forwardToClient(const std::shared_ptr<Message>& payload)
     return true;
 }
 
+void ClientSession::onDisconnect()
+{
+    LOG_INF(getName() << " Disconnected.");
+
+    const auto docBroker = getDocumentBroker();
+    LOG_CHECK_RET(docBroker && "Null DocumentBroker instance", );
+    const auto docKey = docBroker->getDocKey();
+
+    try
+    {
+        // Connection terminated. Destroy session.
+        LOG_DBG(getName() << " on docKey [" << docKey << "] terminated. Cleaning up.");
+
+        // We issue a force-save when last editable (non-readonly) session is going away
+        // and defer destroying the last session and the docBroker.
+        docBroker->removeSession(getId(), true);
+    }
+    catch (const UnauthorizedRequestException& exc)
+    {
+        LOG_ERR("Error in client request handler: " << exc.toString());
+        const std::string status = "error: cmd=internal kind=unauthorized";
+        LOG_TRC("Sending to Client [" << status << "].");
+        sendFrame(status);
+    }
+    catch (const std::exception& exc)
+    {
+        LOG_ERR("Error in client request handler: " << exc.what());
+    }
+
+    try
+    {
+        if (isCloseFrame())
+        {
+            LOG_TRC("Normal close handshake.");
+            // Client initiated close handshake
+            // respond with close frame
+            shutdown();
+        }
+        else if (!ShutdownRequestFlag)
+        {
+            // something wrong, with internal exceptions
+            LOG_TRC("Abnormal close handshake.");
+            closeFrame();
+            shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY);
+        }
+        else
+        {
+#if 0 // loolnb
+            std::lock_guard<std::mutex> lock(ClientWebSocketsMutex);
+            LOG_TRC("Capturing Client WS for [" << _id << "]");
+            // ClientWebSockets.push_back(ws); //FIXME
+#endif
+        }
+    }
+    catch (const std::exception& exc)
+    {
+        LOG_WRN(getName() << ": Exception while closing socket for docKey [" << docKey << "]: " << exc.what());
+    }
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 51b810a..b59c566 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -112,10 +112,15 @@ public:
     /// Set WOPI fileinfo object
     void setWopiFileInfo(std::unique_ptr<WopiStorage::WOPIFileInfo>& wopiFileInfo) { _wopiFileInfo = std::move(wopiFileInfo); }
 
+private:
+
+    /// SocketHandler: disconnection event.
+    void onDisconnect() override;
+    /// SocketHandler: have data to write.
     bool hasQueuedWrites() const override;
+    /// SocketHandler: write to socket.
     void performWrites() override;
 
-private:
     virtual bool _handleInput(const char* buffer, int length) override;
 
     bool loadDocument(const char* buffer, int length, const std::vector<std::string>& tokens,
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 6528ac9..0d8a5fb 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1638,9 +1638,8 @@ private:
 
     void onDisconnect() override
     {
-        if (_clientSession)
-            disposeSession();
-
+        // FIXME: Move to ClientSession (ideally, wrap in ConnectionCounter object
+        // to wrap this global NumConnections).
         const size_t curConnections = --LOOLWSD::NumConnections;
         LOG_TRC("Disconnected connection #" << _connectionNum << " (of " <<
                 (curConnections + 1) << ") as session [" << _id << "].");
@@ -1649,16 +1648,6 @@ private:
     /// Called after successful socket reads.
     void handleIncomingMessage() override
     {
-        if (_clientSession)
-        {
-            LOG_INF("Forwarding incoming message to client [" << _id << "]");
-
-            // TODO: might be better to reset the handler in the socket
-            // so we avoid this double-dispatching.
-            _clientSession->handleIncomingMessage();
-            return;
-        }
-
         auto socket = _socket.lock();
         std::vector<char>& in = socket->_inBuffer;
 
@@ -1788,19 +1777,12 @@ private:
 
     bool hasQueuedWrites() const override
     {
-        // FIXME: - the session should be owning the fd in DocumentBroker's _poll
-        if (_clientSession)
-            return _clientSession->hasQueuedWrites();
-
         LOG_TRC("ClientRequestDispatcher - asked for queued writes");
         return false;
     }
 
     void performWrites() override
     {
-        // FIXME: - the session should be owning the fd in DocumentBroker's _poll
-        if (_clientSession)
-            return _clientSession->performWrites();
     }
 
     void handleFileServerRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
@@ -2211,91 +2193,32 @@ private:
         if (docBroker)
         {
             // TODO: Move to DocumentBroker.
-            _clientSession = createNewClientSession(ws, _id, uriPublic, docBroker, isReadOnly);
-            if (_clientSession)
+            auto clientSession = createNewClientSession(ws, _id, uriPublic, docBroker, isReadOnly);
+            if (clientSession)
             {
                 // Transfer the client socket to the DocumentBroker.
                 auto socket = _socket.lock();
                 if (socket)
                 {
+                    // Move the socket into DocBroker.
                     WebServerPoll.releaseSocket(socket);
-                    _clientSession->onConnect(socket);
                     docBroker->addSocketToPoll(socket);
+
+                    // Set the ClientSession to handle Socket events.
+                    socket->setHandler(clientSession);
                 }
                 docBroker->startThread();
             }
-        }
-        if (!docBroker || !_clientSession)
-            LOG_WRN("Failed to connect DocBroker and Client Session.");
-    }
-
-    // this session went away - cleanup now.
-    void disposeSession()
-    {
-        LOG_CHECK_RET(_clientSession && "Null ClientSession instance", );
-        const auto docBroker = _clientSession->getDocumentBroker();
-        LOG_CHECK_RET(docBroker && "Null DocumentBroker instance", );
-        const auto docKey = docBroker->getDocKey();
-
-        try
-        {
-            // Connection terminated. Destroy session.
-            LOG_DBG("Client session [" << _id << "] on docKey [" << docKey << "] terminated. Cleaning up.");
-
-            // We issue a force-save when last editable (non-readonly) session is going away
-            // and defer destroying the last session and the docBroker.
-            docBroker->removeSession(_id, true);
-
-            LOG_INF("Finishing GET request handler for session [" << _id << "].");
-        }
-        catch (const UnauthorizedRequestException& exc)
-        {
-            LOG_ERR("Error in client request handler: " << exc.toString());
-            const std::string status = "error: cmd=internal kind=unauthorized";
-            LOG_TRC("Sending to Client [" << status << "].");
-            _clientSession->sendFrame(status);
-        }
-        catch (const std::exception& exc)
-        {
-            LOG_ERR("Error in client request handler: " << exc.what());
-        }
-
-        try
-        {
-            if (_clientSession->isCloseFrame())
-            {
-                LOG_TRC("Normal close handshake.");
-                // Client initiated close handshake
-                // respond with close frame
-                _clientSession->shutdown();
-            }
-            else if (!ShutdownRequestFlag)
-            {
-                // something wrong, with internal exceptions
-                LOG_TRC("Abnormal close handshake.");
-                _clientSession->closeFrame();
-                _clientSession->shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY);
-            }
             else
-            {
-#if 0 // loolnb
-                std::lock_guard<std::mutex> lock(ClientWebSocketsMutex);
-                LOG_TRC("Capturing Client WS for [" << _id << "]");
-                // ClientWebSockets.push_back(ws); //FIXME
-#endif
-            }
-        }
-        catch (const std::exception& exc)
-        {
-            LOG_WRN("Exception while closing socket for session [" << _id <<
-                    "] of docKey [" << docKey << "]: " << exc.what());
+                LOG_WRN("Failed to create Client Session with id [" << _id << "] on docKey [" << docKey << "].");
         }
+        else
+            LOG_WRN("Failed to create DocBroker with docKey [" << docKey << "].");
     }
 
 private:
     // The socket that owns us (we can't own it).
     std::weak_ptr<StreamSocket> _socket;
-    std::shared_ptr<ClientSession> _clientSession;
     std::string _id;
     size_t _connectionNum;
 };
commit fa1dc4e05153bdbdc683ac6bcbeef156d3fa5c00
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Mar 14 22:15:01 2017 -0400

    wsd: wrap polling thread and manage state correctly
    
    Change-Id: Iaa4eff1fac1cd7147603ba0c9d51fd8b6b0e96d2

diff --git a/net/Socket.cpp b/net/Socket.cpp
index 96734d0..5f13202 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -79,8 +79,7 @@ void SocketPoll::startThread()
 {
     if (!_threadStarted)
     {
-        _threadStarted = true;
-        _thread = std::thread(&SocketPoll::pollingThread, this);
+        _thread = std::thread(&SocketPoll::pollingThreadEntry, this);
         _owner = _thread.get_id();
     }
 }
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 2e69a91..1d67bb6 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -275,14 +275,10 @@ public:
     /// The default implementation of our polling thread
     virtual void pollingThread()
     {
-        Util::setThreadName(_name);
-        LOG_INF("Starting polling thread [" << _name << "].");
         while (continuePolling())
         {
             poll(DefaultPollTimeoutMs);
         }
-
-        _threadFinished = true;
     }
 
     /// Are we running in either shutdown, or the polling thread.
@@ -461,6 +457,28 @@ private:
         _pollFds[size].revents = 0;
     }
 
+    /// The polling thread entry.
+    /// Used to set the thread name and mark the thread as stopped when done.
+    void pollingThreadEntry()
+    {
+        _threadStarted = true;
+
+        try
+        {
+            Util::setThreadName(_name);
+            LOG_INF("Starting polling thread [" << _name << "].");
+
+            // Invoke the virtual implementation.
+            pollingThread();
+        }
+        catch (const std::exception& exc)
+        {
+            LOG_ERR("Exception in polling thread [" << _name << "]: " << exc.what());
+        }
+
+        _threadFinished = true;
+    }
+
 private:
     /// Debug name used for logging.
     const std::string _name;
commit c8bece208dd4537350b805dbd10696dafd10d37c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Mar 14 22:14:20 2017 -0400

    wsd: socket logs include socket FD for better traceability
    
    Change-Id: I994c7c5ab73b97be312a9d6abf3258dc5f4c08c1

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 9c52b2a..2e69a91 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -54,7 +54,7 @@ public:
     {
         // TODO: Should we shutdown here or up to the client?
 
-        LOG_TRC("#" << getFD() << " close socket.");
+        LOG_TRC("#" << getFD() << " Socket dtor.");
 
         // Doesn't block on sockets; no error handling needed.
         close(_fd);
@@ -102,7 +102,7 @@ public:
         _sendBufferSize = getSocketBufferSize();
         if (rc != 0 || _sendBufferSize < 0 )
         {
-            LOG_ERR("Error getting socket buffer size " << errno);
+            LOG_ERR("#" << _fd << ": Error getting socket buffer size " << errno);
             _sendBufferSize = DefaultSendBufferSize;
             return false;
         }
@@ -110,11 +110,12 @@ public:
         {
             if (_sendBufferSize > MaximumSendBufferSize * 2)
             {
-                LOG_TRC("Clamped send buffer size to " << MaximumSendBufferSize << " from " << _sendBufferSize);
+                LOG_TRC("#" << _fd << ": Clamped send buffer size to " <<
+                        MaximumSendBufferSize << " from " << _sendBufferSize);
                 _sendBufferSize = MaximumSendBufferSize;
             }
             else
-                LOG_TRC("Set socket buffer size to " << _sendBufferSize);
+                LOG_TRC("#" << _fd << ": Set socket buffer size to " << _sendBufferSize);
             return true;
         }
     }
@@ -220,7 +221,7 @@ protected:
 
         const int oldSize = getSocketBufferSize();
         setSocketBufferSize(0);
-        LOG_TRC("Socket #" << _fd << " buffer size: " << getSendBufferSize() << " (was " << oldSize << ")");
+        LOG_TRC("#" << _fd << ": Buffer size: " << getSendBufferSize() << " (was " << oldSize << ")");
 #endif
 
     }
@@ -548,7 +549,7 @@ public:
     virtual void shutdown() override
     {
         _shutdownSignalled = true;
-        LOG_TRC("#" << getFD() << ": shutdown signalled");
+        LOG_TRC("#" << getFD() << ": Async shutdown requested.");
     }
 
     /// Perform the real shutdown.
@@ -707,7 +708,7 @@ protected:
 
         if (closed)
         {
-            LOG_TRC("#" << getFD() << ": closed.");
+            LOG_TRC("#" << getFD() << ": Closed. Firing onDisconnect.");
             _closed = true;
             _socketHandler->onDisconnect();
         }
@@ -734,12 +735,12 @@ protected:
 
                 auto& log = Log::logger();
                 if (log.trace() && len > 0) {
-                    LOG_TRC("#" << getFD() << ": Wrote outgoing data " << len << " bytes");
+                    LOG_TRC("#" << getFD() << ": Wrote outgoing data " << len << " bytes.");
                     // log.dump("", &_outBuffer[0], len);
                 }
 
                 if (len <= 0)
-                    LOG_SYS("#" << getFD() << ": Wrote outgoing data " << len << " bytes");
+                    LOG_SYS("#" << getFD() << ": Wrote outgoing data " << len << " bytes.");
             }
             while (len < 0 && errno == EINTR);
 
@@ -807,7 +808,7 @@ namespace HttpHelper
         struct stat st;
         if (stat(path.c_str(), &st) != 0)
         {
-            LOG_WRN("Failed to stat [" << path << "]. File will not be sent.");
+            LOG_WRN("#" << socket->getFD() << ": Failed to stat [" << path << "]. File will not be sent.");
             throw Poco::FileNotFoundException("Failed to stat [" + path + "]. File will not be sent.");
             return;
         }
@@ -828,7 +829,7 @@ namespace HttpHelper
         std::ostringstream oss;
         response.write(oss);
         const std::string header = oss.str();
-        LOG_TRC("Sending file [" << path << "]: " << header);
+        LOG_TRC("#" << socket->getFD() << ": Sending file [" << path << "]: " << header);
         socket->send(header);
 
         std::ifstream file(path, std::ios::binary);
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index b573d44..9a72f2a 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -96,7 +96,7 @@ public:
         if (socket == nullptr)
             return;
 
-        LOG_TRC("#" << socket->getFD() << " shutdown websocket.");
+        LOG_TRC("#" << socket->getFD() << ": Shutdown websocket.");
 
         const size_t len = statusMessage.size();
         std::vector<char> buf(2 + len);
@@ -123,7 +123,7 @@ public:
         if (len == 0)
             return false; // avoid logging.
 
-        LOG_TRC("Incoming WebSocket data of " << len << " bytes to socket #" << socket->getFD());
+        LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket data of " << len << " bytes.");
 
         if (len < 2) // partial read
             return false;
@@ -185,7 +185,7 @@ public:
         socket->_inBuffer.erase(socket->_inBuffer.begin(), socket->_inBuffer.begin() + headerLen + payloadLen);
 
         // FIXME: fin, aggregating payloads into _wsPayload etc.
-        LOG_TRC("Incoming WebSocket message code " << code << " fin? " << fin << " payload length " << _wsPayload.size());
+        LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " << code << " fin? " << fin << " payload length " << _wsPayload.size());
 
         if (code & WSOpCode::Close)
         {
@@ -227,7 +227,9 @@ public:
 
     bool hasQueuedWrites() const override
     {
-        LOG_TRC("WebSocket - asked for queued writes");
+        auto socket = _socket.lock();
+        if (socket != nullptr)
+            LOG_TRC("#" << socket->getFD() << ": WebSocket - asked for queued writes");
         return false;
     }
 
@@ -345,19 +347,19 @@ protected:
     /// Upgrade the http(s) connection to a websocket.
     void upgradeToWebSocket(const Poco::Net::HTTPRequest& req)
     {
-        LOG_TRC("Upgrading to WebSocket");
-        assert(_wsState == WSState::HTTP);
-
         auto socket = _socket.lock();
         if (socket == nullptr)
             throw std::runtime_error("Invalid socket while upgrading to WebSocket. Request: " + req.getURI());
 
+        LOG_TRC("#" << socket->getFD() << ": Upgrading to WebSocket.");
+        assert(_wsState == WSState::HTTP);
+
         // create our websocket goodness ...
         const int wsVersion = std::stoi(req.get("Sec-WebSocket-Version", "13"));
         const std::string wsKey = req.get("Sec-WebSocket-Key", "");
         const std::string wsProtocol = req.get("Sec-WebSocket-Protocol", "chat");
         // FIXME: other sanity checks ...
-        LOG_INF("WebSocket version " << wsVersion << " key '" << wsKey << "'.");
+        LOG_INF("#" << socket->getFD() << ": WebSocket version " << wsVersion << " key '" << wsKey << "'.");
 
         std::ostringstream oss;
         oss << "HTTP/1.1 101 Switching Protocols\r\n"
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 8ae2035..6528ac9 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1511,7 +1511,7 @@ private:
             auto logger = Log::info();
             if (logger.enabled())
             {
-                logger << "Prisoner HTTP Request from #" << socket->getFD() << ": "
+                logger << "#" << socket->getFD() << ": Prisoner HTTP Request: "
                        << request.getMethod() << ' '
                        << request.getURI() << ' '
                        << request.getVersion();
@@ -1684,7 +1684,7 @@ private:
             auto logger = Log::info();
             if (logger.enabled())
             {
-                logger << "Client HTTP Request: #" << socket->getFD() << ": "
+                logger << "#" << socket->getFD() << ": Client HTTP Request: "
                        << request.getMethod() << ' '
                        << request.getURI() << ' '
                        << request.getVersion();
commit aa19964b037e0693b7dfef1c7d951d31b6cebe8d
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Mar 14 21:13:36 2017 -0400

    wsd: SSL logging improvement
    
    Change-Id: I99d08a764fd43ab0c7eb50a1f017202e6bbc3477

diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp
index bbcf3b9..7dc11d8 100644
--- a/net/SslSocket.hpp
+++ b/net/SslSocket.hpp
@@ -229,6 +229,8 @@ private:
             {
                 if (sslError == SSL_ERROR_SSL)
                     LOG_TRC("Socket #" << getFD() << " SSL error: SSL (" << sslError << ").");
+                else if (sslError == SSL_ERROR_SYSCALL)
+                    LOG_TRC("Socket #" << getFD() << " SSL error: SYSCALL (" << sslError << ").");
 #if 0 // Recent OpenSSL only
                 else if (sslError == SSL_ERROR_WANT_ASYNC)
                     LOG_TRC("Socket #" << getFD() << " SSL error: WANT_ASYNC (" << sslError << ").");
@@ -236,7 +238,7 @@ private:
                     LOG_TRC("Socket #" << getFD() << " SSL error: WANT_ASYNC_JOB (" << sslError << ").");
 #endif
                 else
-                    LOG_TRC("Socket #" << getFD() << " SSL error: UKNOWN (" << sslError << ").");
+                    LOG_TRC("Socket #" << getFD() << " SSL error: UNKNOWN (" << sslError << ").");
 
                 // The error is comming from BIO. Find out what happened.
                 const long bioError = ERR_get_error();
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index d89f49b..b573d44 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -33,7 +33,6 @@ protected:
         Mask = 0x80
     };
 
-
 public:
     WebSocketHandler() :
         _shuttingDown(false),
@@ -226,7 +225,6 @@ public:
             ; // can have multiple msgs in one recv'd packet.
     }
 
-
     bool hasQueuedWrites() const override
     {
         LOG_TRC("WebSocket - asked for queued writes");
@@ -238,6 +236,7 @@ public:
         assert(false && "performWrites not implemented");
     }
 
+    /// Sends a WebSocket Text message.
     void sendFrame(const std::string& msg) const
     {
         sendMessage(msg.data(), msg.size(), WSOpCode::Text);
commit 45e1c7763ca5e3334f01555c7fa88fd30075b747
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Mar 14 20:49:17 2017 -0400

    wsd: Socket stores SocketHandler shared_ptr
    
    Change-Id: I5b460069eb4d91cee2d58833791f961fd6f42b39

diff --git a/net/Socket.hpp b/net/Socket.hpp
index a5405a8..9c52b2a 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -517,7 +517,7 @@ class StreamSocket : public Socket, public std::enable_shared_from_this<StreamSo
 {
 public:
     /// Create a StreamSocket from native FD and take ownership of handler instance.
-    StreamSocket(const int fd, std::unique_ptr<SocketHandlerInterface> socketHandler) :
+    StreamSocket(const int fd, std::shared_ptr<SocketHandlerInterface> socketHandler) :
         Socket(fd),
         _socketHandler(std::move(socketHandler)),
         _closed(false),
@@ -626,7 +626,7 @@ public:
     }
 
     /// Replace the existing SocketHandler with a new one.
-    void setHandler(std::unique_ptr<SocketHandlerInterface> handler)
+    void setHandler(std::shared_ptr<SocketHandlerInterface> handler)
     {
         _socketHandler = std::move(handler);
         _socketHandler->onConnect(shared_from_this());
@@ -637,7 +637,7 @@ public:
     /// but we can't have a shared_ptr in the ctor.
     template <typename TSocket>
     static
-    std::shared_ptr<TSocket> create(const int fd, std::unique_ptr<SocketHandlerInterface> handler)
+    std::shared_ptr<TSocket> create(const int fd, std::shared_ptr<SocketHandlerInterface> handler)
     {
         SocketHandlerInterface* pHandler = handler.get();
         auto socket = std::make_shared<TSocket>(fd, std::move(handler));
@@ -777,7 +777,7 @@ protected:
 
 protected:
     /// Client handling the actual data.
-    std::unique_ptr<SocketHandlerInterface> _socketHandler;
+    std::shared_ptr<SocketHandlerInterface> _socketHandler;
 
     /// True if we are already closed.
     bool _closed;
diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp
index 6cf95b5..bbcf3b9 100644
--- a/net/SslSocket.hpp
+++ b/net/SslSocket.hpp
@@ -19,7 +19,7 @@
 class SslStreamSocket : public StreamSocket
 {
 public:
-    SslStreamSocket(const int fd, std::unique_ptr<SocketHandlerInterface> responseClient) :
+    SslStreamSocket(const int fd, std::shared_ptr<SocketHandlerInterface> responseClient) :
         StreamSocket(fd, std::move(responseClient)),
         _ssl(nullptr),
         _sslWantsTo(SslWantsTo::Neither),


More information about the Libreoffice-commits mailing list