[Libreoffice-commits] online.git: 3 commits - wsd/LOOLWSD.cpp wsd/ProofKey.cpp

Michael Meeks (via logerrit) logerrit at kemper.freedesktop.org
Tue May 12 14:29:34 UTC 2020


 wsd/LOOLWSD.cpp  |  256 +++++++++++++++++++++++++++++++++++--------------------
 wsd/ProofKey.cpp |    1 
 2 files changed, 166 insertions(+), 91 deletions(-)

New commits:
commit dc201a81a793fc6f8bd15c6a3c44e93578b3488d
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Tue May 12 15:04:40 2020 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Tue May 12 15:29:07 2020 +0100

    Simplify and centralize document URI decoding.
    
    Change-Id: I758ef20fa1efa79703ecefee8836cbf00176e659

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 453ae46ed..a528238e5 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -320,6 +320,15 @@ public:
             _pathSegs = StringVector(_uriString, tokens);
         }
     }
+    // matches the WOPISrc if used. For load balancing
+    // must be 2nd element in the path after /lool/<here>
+    std::string getDocumentURI() const
+    {
+        assert(equals(0, "lool"));
+        std::string decodedUri;
+        Poco::URI::decode(_pathSegs[1], decodedUri);
+        return decodedUri;
+    }
     std::string getURI() const
     {
         return _uriString;
@@ -2450,18 +2459,12 @@ private:
             }
 
             else if (requestDetails.isProxy() && requestDetails.equals(2, "ws"))
-            {
-                std::string decodedUri; // WOPISrc
-                Poco::URI::decode(requestDetails[1], decodedUri);
-                handleClientProxyRequest(request, decodedUri, message, disposition);
-            }
+                handleClientProxyRequest(request, requestDetails.getDocumentURI(), message, disposition);
+
             else if (requestDetails.equals(0, "lool") &&
                      requestDetails.equals(2, "ws") && requestDetails.isWebSocket())
-            {
-                std::string decodedUri; // WOPISrc
-                Poco::URI::decode(requestDetails[1], decodedUri);
-                handleClientWsUpgrade(request, decodedUri, disposition, socket);
-            }
+                handleClientWsUpgrade(request, requestDetails.getDocumentURI(), disposition, socket);
+
             else if (!requestDetails.isWebSocket() && requestDetails.equals(0, "lool"))
             {
                 // All post requests have url prefix 'lool'.
@@ -2897,8 +2900,7 @@ private:
 
                 // Validate the docKey
                 std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
-                std::string decodedUri;
-                URI::decode(requestDetails[1], decodedUri);
+                std::string decodedUri = requestDetails.getDocumentURI();
                 const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
                 auto docBrokerIt = DocBrokers.find(docKey);
 
@@ -2930,8 +2932,7 @@ private:
             // TODO: Check that the user in question has access to this file!
 
             // 1. Validate the dockey
-            std::string decodedUri;
-            URI::decode(requestDetails[1], decodedUri);
+            std::string decodedUri = requestDetails.getDocumentURI();
             const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
             std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
             auto docBrokerIt = DocBrokers.find(docKey);
commit 823a2dbafea4cfb68b56742f0d5ef533e6ac5319
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Tue May 12 14:39:26 2020 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Tue May 12 15:29:07 2020 +0100

    killpoco: remove last StringTokenizer.
    
    NB. indices shrink by one.
    
    Change-Id: Ibce8c01622a4ed27f9fe10608f929a93aef2bab4

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index f261cd355..453ae46ed 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -93,7 +93,6 @@ using Poco::Net::PartHandler;
 #include <Poco/Path.h>
 #include <Poco/SAX/InputSource.h>
 #include <Poco/StreamCopier.h>
-#include <Poco/StringTokenizer.h>
 #include <Poco/TemporaryFile.h>
 #include <Poco/URI.h>
 #include <Poco/Util/AbstractConfiguration.h>
@@ -161,7 +160,6 @@ using Poco::Net::MessageHeader;
 using Poco::Net::NameValueCollection;
 using Poco::Path;
 using Poco::StreamCopier;
-using Poco::StringTokenizer;
 using Poco::TemporaryFile;
 using Poco::URI;
 using Poco::Util::Application;
@@ -354,6 +352,10 @@ public:
     {
         return _pathSegs[index];
     }
+    size_t size() const
+    {
+        return _pathSegs.size();
+    }
     std::string toString() const
     {
         std::ostringstream oss;
@@ -2463,7 +2465,7 @@ private:
             else if (!requestDetails.isWebSocket() && requestDetails.equals(0, "lool"))
             {
                 // All post requests have url prefix 'lool'.
-                handlePostRequest(request, message, disposition, socket);
+                handlePostRequest(requestDetails, request, message, disposition, socket);
             }
             else
             {
@@ -2802,19 +2804,19 @@ private:
             return false;
     }
 
-    void handlePostRequest(const Poco::Net::HTTPRequest& request,
+    void handlePostRequest(const RequestDetails &requestDetails,
+                           const Poco::Net::HTTPRequest& request,
                            Poco::MemoryInputStream& message,
                            SocketDisposition& disposition,
                            const std::shared_ptr<StreamSocket>& socket)
     {
         assert(socket && "Must have a valid socket");
 
-        LOG_INF("Post request: [" << LOOLWSD::anonymizeUrl(request.getURI()) << "]");
+        LOG_INF("Post request: [" << LOOLWSD::anonymizeUrl(requestDetails.getURI()) << "]");
 
         Poco::Net::HTTPResponse response;
 
-        StringTokenizer tokens(request.getURI(), "/?");
-        if (tokens.count() > 2 && tokens[2] == "convert-to")
+        if (requestDetails.equals(1, "convert-to"))
         {
             // Validate sender - FIXME: should do this even earlier.
             if (!allowConvertTo(socket->clientAddress(), request, true))
@@ -2840,8 +2842,8 @@ private:
             bool bFullSheetPreview = sFullSheetPreview == "true" ? true : false;
 
             // prefer what is in the URI
-            if (tokens.count() > 3)
-                format = tokens[3];
+            if (requestDetails.size() > 2)
+                format = requestDetails[2];
 
             std::string fromPath = handler.getFilename();
             LOG_INF("Conversion request for URI [" << fromPath << "] format [" << format << "].");
@@ -2881,7 +2883,7 @@ private:
             }
             return;
         }
-        else if (tokens.count() >= 4 && tokens[3] == "insertfile")
+        else if (requestDetails.equals(2, "insertfile"))
         {
             LOG_INF("Insert file request.");
 
@@ -2896,7 +2898,7 @@ private:
                 // Validate the docKey
                 std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
                 std::string decodedUri;
-                URI::decode(tokens[2], decodedUri);
+                URI::decode(requestDetails[1], decodedUri);
                 const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
                 auto docBrokerIt = DocBrokers.find(docKey);
 
@@ -2922,14 +2924,14 @@ private:
                 }
             }
         }
-        else if (tokens.count() >= 6)
+        else if (requestDetails.size() >= 5)
         {
             LOG_INF("File download request.");
             // TODO: Check that the user in question has access to this file!
 
             // 1. Validate the dockey
             std::string decodedUri;
-            URI::decode(tokens[2], decodedUri);
+            URI::decode(requestDetails[1], decodedUri);
             const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
             std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
             auto docBrokerIt = DocBrokers.find(docKey);
@@ -2939,7 +2941,7 @@ private:
             }
 
             // 2. Cross-check if received child id is correct
-            if (docBrokerIt->second->getJailId() != tokens[3])
+            if (docBrokerIt->second->getJailId() != requestDetails[2])
             {
                 throw BadRequestException("ChildId does not correspond to docKey");
             }
@@ -2947,16 +2949,16 @@ private:
             // 3. Don't let user download the file in main doc directory containing
             // the document being edited otherwise we will end up deleting main directory
             // after download finishes
-            if (docBrokerIt->second->getJailId() == tokens[4])
+            if (docBrokerIt->second->getJailId() == requestDetails[3])
             {
                 throw BadRequestException("RandomDir cannot be equal to ChildId");
             }
             docBrokersLock.unlock();
 
             std::string fileName;
-            URI::decode(tokens[5], fileName);
-            const Path filePath(LOOLWSD::ChildRoot + tokens[3]
-                                + JAILED_DOCUMENT_ROOT + tokens[4] + "/" + fileName);
+            URI::decode(requestDetails[4], fileName);
+            const Path filePath(LOOLWSD::ChildRoot + requestDetails[2]
+                                + JAILED_DOCUMENT_ROOT + requestDetails[3] + "/" + fileName);
             const std::string filePathAnonym = LOOLWSD::anonymizeUrl(filePath.toString());
             LOG_INF("HTTP request for: " << filePathAnonym);
             if (filePath.isAbsolute() && File(filePath).exists())
commit 82fec145eb3c5886d7009471080fe5126ad80df7
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Tue May 12 13:41:26 2020 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Tue May 12 15:29:07 2020 +0100

    RequestDetails - move into a single class & simplify flow.
    
    Change-Id: Ic9148350e04fca7876ec1b5985b467524c6894e1

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index d024ee436..f261cd355 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -278,6 +278,95 @@ void alertAllUsersInternal(const std::string& msg)
 
 } // end anonymous namespace
 
+class RequestDetails {
+    Poco::URI _uri;
+    bool _isGet;
+    bool _isHead;
+    bool _isProxy;
+    bool _isWebSocket;
+    std::string _uriString;
+    StringVector _pathSegs;
+public:
+    RequestDetails(Poco::Net::HTTPRequest &request)
+    {
+        // Check and remove the ServiceRoot from the request.getURI()
+        if (!Util::startsWith(request.getURI(), LOOLWSD::ServiceRoot))
+            throw BadRequestException("The request does not start with prefix: " + LOOLWSD::ServiceRoot);
+
+        // re-writes ServiceRoot out of request
+        _uriString = request.getURI().substr(LOOLWSD::ServiceRoot.length());
+        request.setURI(_uriString);
+        _uri = Poco::URI(_uriString);
+        const std::string &method = request.getMethod();
+        _isGet = method == "GET";
+        _isHead = method == "HEAD";
+        _isProxy = request.has("ProxyPrefix");
+        auto it = request.find("Upgrade");
+        _isWebSocket = it != request.end() && (Poco::icompare(it->second, "websocket") == 0);
+
+        std::vector<StringToken> tokens;
+        if (_uriString.size() > 0)
+        {
+            size_t i, start;
+            for (i = start = 0; i < _uriString.size(); ++i)
+            {
+                if (_uriString[i] == '/' || _uriString[i] == '?')
+                {
+                    if (i - start > 1) // ignore empty
+                        tokens.emplace_back(start, i - start);
+                    start = i + 1;
+                }
+            }
+            if (i - start > 1) // ignore empty
+                tokens.emplace_back(start, i - start);
+            _pathSegs = StringVector(_uriString, tokens);
+        }
+    }
+    std::string getURI() const
+    {
+        return _uriString;
+    }
+    bool isProxy() const
+    {
+        return _isProxy;
+    }
+    bool isWebSocket() const
+    {
+        return _isWebSocket;
+    }
+    bool isGet(const char *path) const
+    {
+        return _isGet && _uriString == path;
+    }
+    bool isGetOrHead(const char *path) const
+    {
+        return (_isGet || _isHead) && _uriString == path;
+    }
+    bool startsWith(const char *path)
+    {
+        return !strncmp(_uriString.c_str(), path, strlen(path));
+    }
+    bool equals(size_t index, const char *string) const
+    {
+        return _pathSegs.equals(index, string);
+    }
+    std::string operator[](size_t index) const
+    {
+        return _pathSegs[index];
+    }
+    std::string toString() const
+    {
+        std::ostringstream oss;
+        oss << _uriString << " " << (_isGet?"G":"")
+            << (_isHead?"H":"") << (_isProxy?"Proxy":"")
+            << (_isWebSocket?"WebSocket":"");
+        oss << " path: " << _pathSegs.size();
+        for (size_t i = 0; i < _pathSegs.size(); ++i)
+            oss << " '" << _pathSegs[i] << "'";
+        return oss.str();
+    }
+};
+
 void LOOLWSD::checkSessionLimitsAndWarnClients()
 {
 #if !ENABLE_SUPPORT_KEY
@@ -2256,12 +2345,12 @@ private:
             // update the read cursor - headers are not altered by chunks.
             message.seekg(startmessage.tellg(), std::ios::beg);
 
-            // Check and remove the ServiceRoot from the request.getURI()
-            if (!Util::startsWith(request.getURI(), LOOLWSD::ServiceRoot))
-                throw BadRequestException("The request does not start with prefix: " + LOOLWSD::ServiceRoot);
+            // re-write ServiceRoot and cache.
+            RequestDetails requestDetails(request);
+//            LOG_TRC("Request details " << requestDetails.toString());
 
             // Config & security ...
-            if (request.has("ProxyPrefix"))
+            if (requestDetails.isProxy())
             {
                 if (!LOOLWSD::IsProxyPrefixEnabled)
                     throw BadRequestException("ProxyPrefix present but net.proxy_prefix is not enabled");
@@ -2269,24 +2358,18 @@ private:
                     throw BadRequestException("ProxyPrefix request from non-local socket");
             }
 
-            std::string requestURIString(request.getURI().substr(LOOLWSD::ServiceRoot.length()));
-            request.setURI(requestURIString);
-
             // Routing
-            Poco::URI requestUri(request.getURI());
-            std::vector<std::string> reqPathSegs;
-            requestUri.getPathSegments(reqPathSegs);
-
             if (UnitWSD::get().handleHttpRequest(request, message, socket))
             {
                 // Unit testing, nothing to do here
             }
-            else if (reqPathSegs.size() >= 1 && reqPathSegs[0] == "loleaflet")
+            else if (requestDetails.equals(0, "loleaflet"))
             {
                 // File server
                 handleFileServerRequest(request, message, socket);
             }
-            else if (reqPathSegs.size() >= 2 && reqPathSegs[0] == "lool" && reqPathSegs[1] == "adminws")
+            else if (requestDetails.equals(0, "lool") &&
+                     requestDetails.equals(1, "adminws"))
             {
                 // Admin connections
                 LOG_INF("Admin request: " << request.getURI());
@@ -2299,9 +2382,10 @@ private:
                 }
 
             }
-            else if (reqPathSegs.size() >= 2 && reqPathSegs[0] == "lool" && reqPathSegs[1] == "getMetrics")
+            else if (requestDetails.equals(0, "lool") &&
+                     requestDetails.equals(1, "getMetrics"))
             {
-                //See metrics.txt
+                // See metrics.txt
                 std::shared_ptr<Poco::Net::HTTPResponse> response(new Poco::Net::HTTPResponse());
 
                 if (!LOOLWSD::AdminEnabled)
@@ -2340,72 +2424,61 @@ private:
                             Admin::instance().sendMetricsAsync(streamSocket, response);
                         });
             }
-            // Client post and websocket connections
-            else if ((request.getMethod() == HTTPRequest::HTTP_GET ||
-                      request.getMethod() == HTTPRequest::HTTP_HEAD) &&
-                     request.getURI() == "/")
-            {
+            else if (requestDetails.isGetOrHead("/"))
                 handleRootRequest(request, socket);
-            }
-            else if (request.getMethod() == HTTPRequest::HTTP_GET && request.getURI() == "/favicon.ico")
-            {
+
+            else if (requestDetails.isGet("/favicon.ico"))
                 handleFaviconRequest(request, socket);
+
+            else if (requestDetails.isGet("/hosting/discovery") ||
+                     requestDetails.isGet("/hosting/discovery/"))
+                handleWopiDiscoveryRequest(request, socket);
+
+            else if (requestDetails.isGet(CAPABILITIES_END_POINT))
+                handleCapabilitiesRequest(request, socket);
+
+            else if (requestDetails.isGet("/robots.txt"))
+                handleRobotsTxtRequest(request, socket);
+
+            else if (requestDetails.equals(0, "lool") &&
+                     requestDetails.equals(1, "clipboard"))
+            {
+//              Util::dumpHex(std::cerr, "clipboard:\n", "", socket->getInBuffer()); // lots of data ...
+                handleClipboardRequest(request, message, disposition, socket);
             }
-            else if (request.getMethod() == HTTPRequest::HTTP_GET && (request.getURI() == "/hosting/discovery" || request.getURI() == "/hosting/discovery/"))
+
+            else if (requestDetails.isProxy() && requestDetails.equals(2, "ws"))
             {
-                handleWopiDiscoveryRequest(request, socket);
+                std::string decodedUri; // WOPISrc
+                Poco::URI::decode(requestDetails[1], decodedUri);
+                handleClientProxyRequest(request, decodedUri, message, disposition);
             }
-            else if (request.getMethod() == HTTPRequest::HTTP_GET && request.getURI() == CAPABILITIES_END_POINT)
+            else if (requestDetails.equals(0, "lool") &&
+                     requestDetails.equals(2, "ws") && requestDetails.isWebSocket())
             {
-                handleCapabilitiesRequest(request, socket);
+                std::string decodedUri; // WOPISrc
+                Poco::URI::decode(requestDetails[1], decodedUri);
+                handleClientWsUpgrade(request, decodedUri, disposition, socket);
             }
-            else if (request.getMethod() == HTTPRequest::HTTP_GET && request.getURI() == "/robots.txt")
+            else if (!requestDetails.isWebSocket() && requestDetails.equals(0, "lool"))
             {
-                handleRobotsTxtRequest(request, socket);
+                // All post requests have url prefix 'lool'.
+                handlePostRequest(request, message, disposition, socket);
             }
             else
             {
-                StringTokenizer reqPathTokens(request.getURI(), "/?", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
-                if (reqPathTokens.count() > 1 && reqPathTokens[0] == "lool" && reqPathTokens[1] == "clipboard")
-                {
-//                    Util::dumpHex(std::cerr, "clipboard:\n", "", socket->getInBuffer()); // lots of data ...
-                    handleClipboardRequest(request, message, disposition, socket);
-                }
-                else if (request.has("ProxyPrefix") && reqPathTokens.count() > 3 &&
-                         (reqPathTokens[reqPathTokens.count()-3] == "ws"))
-                {
-                    std::string decodedUri; // WOPISrc
-                    Poco::URI::decode(reqPathTokens[1], decodedUri);
-                    handleClientProxyRequest(request, decodedUri, message, disposition);
-                }
-                else if (!(request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0) &&
-                    reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
-                {
-                    // All post requests have url prefix 'lool'.
-                    handlePostRequest(request, message, disposition, socket);
-                }
-                else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws" &&
-                         request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0)
-                {
-                    std::string decodedUri; // WOPISrc
-                    Poco::URI::decode(reqPathTokens[1], decodedUri);
-                    handleClientWsUpgrade(request, decodedUri, disposition, socket);
-                }
-                else
-                {
-                    LOG_ERR("Unknown resource: " << request.getURI());
+                LOG_ERR("Unknown resource: " << requestDetails.toString());
 
-                    // Bad request.
-                    std::ostringstream oss;
-                    oss << "HTTP/1.1 400\r\n"
-                        "Date: " << Util::getHttpTimeNow() << "\r\n"
-                        "User-Agent: " WOPI_AGENT_STRING "\r\n"
-                        "Content-Length: 0\r\n"
-                        "\r\n";
-                    socket->send(oss.str());
-                    socket->shutdown();
-                    return;
-                }
+                // Bad request.
+                std::ostringstream oss;
+                oss << "HTTP/1.1 400\r\n"
+                    "Date: " << Util::getHttpTimeNow() << "\r\n"
+                    "User-Agent: " WOPI_AGENT_STRING "\r\n"
+                    "Content-Length: 0\r\n"
+                    "\r\n";
+                socket->send(oss.str());
+                socket->shutdown();
+                return;
             }
         }
         catch (const std::exception& exc)
diff --git a/wsd/ProofKey.cpp b/wsd/ProofKey.cpp
index ed49fb0bf..f3bff7599 100644
--- a/wsd/ProofKey.cpp
+++ b/wsd/ProofKey.cpp
@@ -30,7 +30,6 @@
 #include <Poco/Net/HTTPRequest.h>
 #include <Poco/Net/HTTPResponse.h>
 #include <Poco/Net/NetException.h>
-#include <Poco/StringTokenizer.h>
 #include <Poco/Timestamp.h>
 #include <Poco/URI.h>
 #include <Poco/Util/Application.h>


More information about the Libreoffice-commits mailing list