[Libreoffice-commits] online.git: common/Authorization.cpp common/Util.cpp common/Util.hpp

Gülşah Köse (via logerrit) logerrit at kemper.freedesktop.org
Tue Sep 1 21:17:21 UTC 2020


 common/Authorization.cpp |   33 ++++++++++++++++++++++++++++++---
 common/Util.cpp          |   34 +---------------------------------
 common/Util.hpp          |    7 -------
 3 files changed, 31 insertions(+), 43 deletions(-)

New commits:
commit 845554a6a3a76dc4161409f9d6648b8ff4133068
Author:     Gülşah Köse <gulsah.kose at collabora.com>
AuthorDate: Tue Sep 1 20:15:30 2020 +0300
Commit:     Gülşah Köse <gulsah.kose at collabora.com>
CommitDate: Tue Sep 1 23:17:03 2020 +0200

    Revert "wsd: parse headers with Poco::MessageHeader"
    
    This reverts commit dbc562d9abc997b196fd6d4e5e71f42d442488d0.
    
    tst-05694-05694 2020-08-26 12:59:14.343136 [ unittest ]
    ERR Invalid HTTP header [def]: Malformed message:
    Field name too long/no colon found| ../common/Util.cpp:980
    
    Following part of the code tests a request with corrupted http header:
        Authorization auth2(Authorization::Type::Header, "def");
        Poco::Net::HTTPRequest req2;
        auth2.authorizeRequest(req2);
        LOK_ASSERT(!req2.has("Authorization"));
    
    Poco library throws exception.
    
    Change-Id: Ic31a80c0e1e325de27c23059e2bcb3f00d39ad16
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101887
    Tested-by: Jenkins
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Gülşah Köse <gulsah.kose at collabora.com>

diff --git a/common/Authorization.cpp b/common/Authorization.cpp
index 5613aa91a..93c5704fc 100644
--- a/common/Authorization.cpp
+++ b/common/Authorization.cpp
@@ -45,15 +45,42 @@ void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const
     switch (_type)
     {
         case Type::Token:
-            Util::setHttpHeaders(request, "Authorization: Bearer " + _data);
-            assert(request.has("Authorization") && "HTTPRequest missing Authorization header");
+            request.set("Authorization", "Bearer " + _data);
             break;
         case Type::Header:
+        {
             // there might be more headers in here; like
             //   Authorization: Basic ....
             //   X-Something-Custom: Huh
-            Util::setHttpHeaders(request, _data);
+            // Split based on \n's or \r's and trim, to avoid nonsense in the
+            // headers
+            StringVector tokens(Util::tokenizeAnyOf(_data, "\n\r"));
+            for (auto it = tokens.begin(); it != tokens.end(); ++it)
+            {
+                std::string token = tokens.getParam(*it);
+
+                size_t separator = token.find_first_of(':');
+                if (separator != std::string::npos)
+                {
+                    size_t headerStart = token.find_first_not_of(' ', 0);
+                    size_t headerEnd = token.find_last_not_of(' ', separator - 1);
+
+                    size_t valueStart = token.find_first_not_of(' ', separator + 1);
+                    size_t valueEnd = token.find_last_not_of(' ');
+
+                    // set the header
+                    if (headerStart != std::string::npos && headerEnd != std::string::npos &&
+                            valueStart != std::string::npos && valueEnd != std::string::npos)
+                    {
+                        size_t headerLength = headerEnd - headerStart + 1;
+                        size_t valueLength = valueEnd - valueStart + 1;
+
+                        request.set(token.substr(headerStart, headerLength), token.substr(valueStart, valueLength));
+                    }
+                }
+            }
             break;
+        }
         default:
             // assert(false);
             throw BadRequestException("Invalid HTTP request type");
diff --git a/common/Util.cpp b/common/Util.cpp
index f1cd61b69..e0ce00250 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -59,7 +59,7 @@
 #include <Poco/Util/Application.h>
 
 #include "Common.hpp"
-#include <common/Log.hpp>
+#include "Log.hpp"
 #include "Protocol.hpp"
 #include "Util.hpp"
 
@@ -953,38 +953,6 @@ namespace Util
         return std::ctime(&t);
     }
 
-    void setHttpHeaders(Poco::Net::HTTPRequest& request, const std::string& headers)
-    {
-        // Look for either \r or \n and replace them with a single \r\n
-        // as prescribed by rfc2616 as valid header delimeter, removing
-        // any invalid line breaks, to avoid nonsense in the headers.
-        const StringVector tokens = Util::tokenizeAnyOf(headers, "\n\r");
-        const std::string header = tokens.cat("\r\n", 0);
-        try
-        {
-            // Now parse to preserve folded headers and other
-            // corner cases that is conformant to the rfc,
-            // detecting any errors and/or invalid entries.
-            // NB: request.read() expects full message and will fail.
-            Poco::Net::MessageHeader msgHeader;
-            std::istringstream iss(header);
-            msgHeader.read(iss);
-            for (const auto& entry : msgHeader)
-            {
-                // Set each header entry.
-                request.set(Util::trimmed(entry.first), Util::trimmed(entry.second));
-            }
-        }
-        catch (const Poco::Exception& ex)
-        {
-            LOG_ERR("Invalid HTTP header [" << header << "]: " << ex.displayText());
-        }
-        catch (const std::exception& ex)
-        {
-            LOG_ERR("Invalid HTTP header [" << header << "]: " << ex.what());
-        }
-    }
-
     bool isFuzzing()
     {
 #if LIBFUZZER
diff --git a/common/Util.hpp b/common/Util.hpp
index bb81cfb75..9dbfebe8b 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -35,7 +35,6 @@
 #include <Poco/File.h>
 #include <Poco/Path.h>
 #include <Poco/RegularExpression.h>
-#include <Poco/Net/HTTPRequest.h>
 
 #define LOK_USE_UNSTABLE_API
 #include <LibreOfficeKit/LibreOfficeKitEnums.h>
@@ -1083,12 +1082,6 @@ int main(int argc, char**argv)
     /// conversion from steady_clock for debugging / tracing
     std::string getSteadyClockAsString(const std::chrono::steady_clock::time_point &time);
 
-    /// Set the request header by splitting multiple entries by \r or \n.
-    /// Needed to sanitize user-provided http headers, after decoding.
-    /// This is much more tolerant to line breaks than the rfc allows.
-    /// Note: probably should move to a more appropriate home.
-    void setHttpHeaders(Poco::Net::HTTPRequest& request, const std::string& headers);
-
     /// Automatically execute code at end of current scope.
     /// Used for exception-safe code.
     class ScopeGuard


More information about the Libreoffice-commits mailing list