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

Ashod Nakashian ashod.nakashian at collabora.co.uk
Tue May 16 14:41:41 UTC 2017


 wsd/Exceptions.hpp |   15 ++++++
 wsd/LOOLWSD.cpp    |  129 +++++++++++++++++++++++++++++++----------------------
 wsd/Storage.cpp    |    3 -
 3 files changed, 93 insertions(+), 54 deletions(-)

New commits:
commit 5d6eebb15778569feb195149a81ab1fed91f17a6
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun May 14 22:58:02 2017 -0400

    wsd: handle WOPI access failure
    
    Show the user that authorization failed.
    
    Change-Id: Iad63c11ac2033eee80062ecd43dff76f776924c3
    Reviewed-on: https://gerrit.libreoffice.org/37610
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>
    (cherry picked from commit 6e63c80763ebbb8e41063358ce8b39c00f63b929)
    Reviewed-on: https://gerrit.libreoffice.org/37613
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>

diff --git a/wsd/Exceptions.hpp b/wsd/Exceptions.hpp
index b97263e5..11c9de59 100644
--- a/wsd/Exceptions.hpp
+++ b/wsd/Exceptions.hpp
@@ -58,6 +58,21 @@ public:
     using LoolException::LoolException;
 };
 
+/// An access-denied exception that is meant to signify
+/// a storage authentication error.
+class AccessDeniedException : public LoolException
+{
+public:
+    using LoolException::LoolException;
+};
+
+/// A service-unavailable exception that is meant to signify
+/// an internal error.
+class ServiceUnavailableException : public LoolException
+{
+public:
+    using LoolException::LoolException;
+};
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index da1e6535..a191cded 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2033,79 +2033,102 @@ private:
         // First Upgrade.
         WebSocketHandler ws(_socket, request);
 
-        if (LOOLWSD::NumConnections >= MAX_CONNECTIONS)
+        // Response to clients beyond this point is done via WebSocket.
+        try
         {
-            LOG_ERR("Limit on maximum number of connections of " << MAX_CONNECTIONS << " reached.");
-            shutdownLimitReached(ws);
-            return;
-        }
+            if (LOOLWSD::NumConnections >= MAX_CONNECTIONS)
+            {
+                LOG_ERR("Limit on maximum number of connections of " << MAX_CONNECTIONS << " reached.");
+                shutdownLimitReached(ws);
+                return;
+            }
 
-        LOG_INF("Starting GET request handler for session [" << _id << "] on url [" << url << "].");
+            LOG_INF("Starting GET request handler for session [" << _id << "] on url [" << url << "].");
 
-        // Indicate to the client that document broker is searching.
-        const std::string status("statusindicator: find");
-        LOG_TRC("Sending to Client [" << status << "].");
-        ws.sendMessage(status);
+            // Indicate to the client that document broker is searching.
+            const std::string status("statusindicator: find");
+            LOG_TRC("Sending to Client [" << status << "].");
+            ws.sendMessage(status);
 
-        const auto uriPublic = DocumentBroker::sanitizeURI(url);
-        const auto docKey = DocumentBroker::getDocKey(uriPublic);
-        LOG_INF("Sanitized URI [" << url << "] to [" << uriPublic.toString() <<
-                "] and mapped to docKey [" << docKey << "] for session [" << _id << "].");
+            const auto uriPublic = DocumentBroker::sanitizeURI(url);
+            const auto docKey = DocumentBroker::getDocKey(uriPublic);
+            LOG_INF("Sanitized URI [" << url << "] to [" << uriPublic.toString() <<
+                    "] and mapped to docKey [" << docKey << "] for session [" << _id << "].");
 
-        // Check if readonly session is required
-        bool isReadOnly = false;
-        for (const auto& param : uriPublic.getQueryParameters())
-        {
-            LOG_DBG("Query param: " << param.first << ", value: " << param.second);
-            if (param.first == "permission" && param.second == "readonly")
+            // Check if readonly session is required
+            bool isReadOnly = false;
+            for (const auto& param : uriPublic.getQueryParameters())
             {
-                isReadOnly = true;
+                LOG_DBG("Query param: " << param.first << ", value: " << param.second);
+                if (param.first == "permission" && param.second == "readonly")
+                {
+                    isReadOnly = true;
+                }
             }
-        }
 
-        LOG_INF("URL [" << url << "] is " << (isReadOnly ? "readonly" : "writable") << ".");
+            LOG_INF("URL [" << url << "] is " << (isReadOnly ? "readonly" : "writable") << ".");
 
-        // Request a kit process for this doc.
-        auto docBroker = findOrCreateDocBroker(ws, url, docKey, _id, uriPublic);
-        if (docBroker)
-        {
-            auto clientSession = createNewClientSession(&ws, _id, uriPublic, docBroker, isReadOnly);
-            if (clientSession)
+            // Request a kit process for this doc.
+            auto docBroker = findOrCreateDocBroker(ws, url, docKey, _id, uriPublic);
+            if (docBroker)
             {
-                // Transfer the client socket to the DocumentBroker when we get back to the poll:
-                disposition.setMove([docBroker, clientSession]
-                                    (const std::shared_ptr<Socket> &moveSocket)
+                auto clientSession = createNewClientSession(&ws, _id, uriPublic, docBroker, isReadOnly);
+                if (clientSession)
                 {
-                    // Make sure the thread is running before adding callback.
-                    docBroker->startThread();
-
-                    // We no longer own this socket.
-                    moveSocket->setThreadOwner(std::thread::id(0));
-
-                    docBroker->addCallback([docBroker, moveSocket, clientSession]()
+                    // Transfer the client socket to the DocumentBroker when we get back to the poll:
+                    disposition.setMove([docBroker, clientSession]
+                                        (const std::shared_ptr<Socket> &moveSocket)
                     {
-                        auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
-
-                        // Set the ClientSession to handle Socket events.
-                        streamSocket->setHandler(clientSession);
-                        LOG_DBG("Socket #" << moveSocket->getFD() << " handler is " << clientSession->getName());
+                        // Make sure the thread is running before adding callback.
+                        docBroker->startThread();
 
-                        // Move the socket into DocBroker.
-                        docBroker->addSocketToPoll(moveSocket);
+                        // We no longer own this socket.
+                        moveSocket->setThreadOwner(std::thread::id(0));
 
-                        // Add and load the session.
-                        docBroker->addSession(clientSession);
+                        docBroker->addCallback([docBroker, moveSocket, clientSession]()
+                        {
+                            try
+                            {
+                                auto streamSocket = std::static_pointer_cast<StreamSocket>(moveSocket);
+
+                                // Set the ClientSession to handle Socket events.
+                                streamSocket->setHandler(clientSession);
+                                LOG_DBG("Socket #" << moveSocket->getFD() << " handler is " << clientSession->getName());
+
+                                // Move the socket into DocBroker.
+                                docBroker->addSocketToPoll(moveSocket);
+
+                                // Add and load the session.
+                                docBroker->addSession(clientSession);
+                            }
+                            catch (const std::exception& exc)
+                            {
+                                LOG_ERR("Error while handling loading : " << exc.what());
+                                const std::string msg = "error: cmd=internal kind=unauthorized";
+                                clientSession->sendMessage(msg);
+                                docBroker->stop();
+                            }
+                        });
                     });
-                });
+                }
+                else
+                {
+                    LOG_WRN("Failed to create Client Session with id [" << _id << "] on docKey [" << docKey << "].");
+                    cleanupDocBrokers();
+                }
             }
             else
             {
-                LOG_WRN("Failed to create Client Session with id [" << _id << "] on docKey [" << docKey << "].");
-                cleanupDocBrokers();
+                throw ServiceUnavailableException("Failed to create DocBroker with docKey [" + docKey + "].");
             }
         }
-        else
-            LOG_WRN("Failed to create DocBroker with docKey [" << docKey << "].");
+        catch (const std::exception& exc)
+        {
+            LOG_ERR("Error while handling Client WS Request: " << exc.what());
+            const std::string msg = "error: cmd=internal kind=load";
+            ws.sendMessage(msg);
+            ws.shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY, msg);
+        }
     }
 
 private:
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 10849e48..d0c3d3f4 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -493,7 +493,8 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const st
     }
     else
     {
-        LOG_ERR("WOPI::CheckFileInfo is missing JSON payload");
+        LOG_ERR("WOPI::CheckFileInfo failed and no JSON payload returned. Access denied.");
+        throw UnauthorizedRequestException("Access denied.");
     }
 
     Poco::Timestamp modifiedTime = Poco::Timestamp::fromEpochTime(0);


More information about the Libreoffice-commits mailing list