common/Unit.cpp              |    2 
 common/Unit.hpp              |   16 +++++
 test/Makefile.am             |   13 +++-
 test/UnitWOPIHttpHeaders.cpp |  116 +++++++++++++++++++++++++++++++++++++++++++
 test/WopiTestServer.hpp      |   13 ++++
 wsd/FileServer.cpp           |    3 +
 wsd/Storage.cpp              |    2 
 7 files changed, 159 insertions(+), 6 deletions(-)

New commits:
commit e68be80496e775e18edb5cbc7d3f229603fa2a2c
Author:     Ashod Nakashian <ashod.nakash...@collabora.co.uk>
AuthorDate: Mon Jun 22 08:24:11 2020 -0400
Commit:     Ashod Nakashian <ashnak...@gmail.com>
CommitDate: Wed Jul 1 07:37:10 2020 +0200

    wsd: add http-headers unit-test
    
    This is to defend the sneaking of extra http-headers
    in the access_header URI param that was recently fixed.
    
    Change-Id: Ic28cf58854847ac278bed8043f398b107f7992b3
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/96862
    Tested-by: Jenkins
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/common/Unit.cpp b/common/Unit.cpp
index 19cacc8aa..ff94d63f5 100644
--- a/common/Unit.cpp
+++ b/common/Unit.cpp
@@ -203,7 +203,7 @@ void UnitBase::exitTest(TestResult result)
         return;
     }
 
-    LOG_INF("exitTest: " << (int)result << ". Flagging to shutdown.");
+    LOG_INF("exitTest: " << testResultAsString(result) << ". Flagging to 
shutdown.");
     _setRetValue = true;
     _retValue = result == TestResult::Ok ? EX_OK : EX_SOFTWARE;
 #if !MOBILEAPP
diff --git a/common/Unit.hpp b/common/Unit.hpp
index 9b6a88ece..3e65d7b23 100644
--- a/common/Unit.hpp
+++ b/common/Unit.hpp
@@ -70,6 +70,22 @@ protected:
         TimedOut
     };
 
+    static const std::string testResultAsString(TestResult res)
+    {
+        switch (res)
+        {
+            case TestResult::Failed:
+                return "Failed";
+            case TestResult::Ok:
+                return "Ok";
+            case TestResult::TimedOut:
+                return "TimedOut";
+        }
+
+        assert(!"Unknown TestResult entry.");
+        return std::to_string(static_cast<int>(res));
+    }
+
     /// Encourages the process to exit with this value (unless hooked)
     void exitTest(TestResult result);
 
diff --git a/test/Makefile.am b/test/Makefile.am
index e05dfdf3f..c12038b0d 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -41,7 +41,9 @@ noinst_LTLIBRARIES = \
        unit-close.la \
        unit-bad-doc-load.la \
        unit-hosting.la \
-       unit-wopi-loadencoded.la unit-wopi-temp.la
+       unit-wopi-loadencoded.la \
+       unit-wopi-temp.la \
+       unit-wopi-httpheaders.la
 
 MAGIC_TO_FORCE_SHLIB_CREATION = -rpath /dummy
 AM_LDFLAGS = -pthread -module $(MAGIC_TO_FORCE_SHLIB_CREATION) $(ZLIB_LIBS)
@@ -148,6 +150,8 @@ unit_wopi_loadencoded_la_SOURCES = UnitWOPILoadEncoded.cpp
 unit_wopi_loadencoded_la_LIBADD = $(CPPUNIT_LIBS)
 unit_wopi_temp_la_SOURCES = UnitWOPITemplate.cpp
 unit_wopi_temp_la_LIBADD = $(CPPUNIT_LIBS)
+unit_wopi_httpheaders_la_SOURCES = UnitWOPIHttpHeaders.cpp
+unit_wopi_httpheaders_la_LIBADD = $(CPPUNIT_LIBS)
 unit_tiff_load_la_SOURCES = UnitTiffLoad.cpp
 unit_tiff_load_la_LIBADD = $(CPPUNIT_LIBS)
 unit_large_paste_la_SOURCES = UnitLargePaste.cpp
@@ -225,7 +229,9 @@ TESTS = \
        unit-close.la \
        unit-bad-doc-load.la \
        unit-hosting.la \
-       unit-wopi-loadencoded.la unit-wopi-temp.la
+       unit-wopi-loadencoded.la \
+       unit-wopi-temp.la \
+       unit-wopi-httpheaders
 # TESTS += unit-admin.test
 # TESTS += unit-storage.test
 
@@ -276,9 +282,10 @@ unit-convert.log : group0.log
 unit-typing.log : group0.log
 unit-tilecache.log : group0.log
 unit-timeout.log : group0.log
+unit-wopi-httpheaders.log: group0.log
 unit-base.log: group0.log
 
-group1.log: unit-crash.log unit-tiletest.log unit-insert-delete.log 
unit-each-view.log unit-httpws.log unit-close.log 
unit-wopi-documentconflict.log unit-prefork.log unit-wopi-versionrestore.log 
unit-wopi-temp.log unit_wopi_renamefile.log unit_wopi_watermark.log 
unit-wopi.log unit-wopi-ownertermination.log unit-load-torture.log 
unit-wopi-saveas.log unit-password-protected.log unit-http.log 
unit-tiff-load.log unit-render-shape.log unit-oauth.log unit-large-paste.log 
unit-paste.log unit-rendering-options.log unit-session.log unit-uno-command.log 
unit-load.log unit-cursor.log unit-calc.log unit-bad-doc-load.log 
unit-hosting.log unit-wopi-loadencoded.log unit-integration.log 
unit-convert.log unit-typing.log unit-tilecache.log unit-timeout.log 
unit-base.log
+group1.log: unit-crash.log unit-tiletest.log unit-insert-delete.log 
unit-each-view.log unit-httpws.log unit-close.log 
unit-wopi-documentconflict.log unit-prefork.log unit-wopi-versionrestore.log 
unit-wopi-temp.log unit_wopi_renamefile.log unit_wopi_watermark.log 
unit-wopi.log unit-wopi-ownertermination.log unit-load-torture.log 
unit-wopi-saveas.log unit-password-protected.log unit-http.log 
unit-tiff-load.log unit-render-shape.log unit-oauth.log unit-large-paste.log 
unit-paste.log unit-rendering-options.log unit-session.log unit-uno-command.log 
unit-load.log unit-cursor.log unit-calc.log unit-bad-doc-load.log 
unit-hosting.log unit-wopi-loadencoded.log unit-integration.log 
unit-convert.log unit-typing.log unit-tilecache.log unit-timeout.log 
unit-base.log unit-wopi-httpheaders.log
        $(CLEANUP_COMMAND)
        touch $@
 
diff --git a/test/UnitWOPIHttpHeaders.cpp b/test/UnitWOPIHttpHeaders.cpp
new file mode 100644
index 000000000..acf2d7471
--- /dev/null
+++ b/test/UnitWOPIHttpHeaders.cpp
@@ -0,0 +1,116 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; 
fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <config.h>
+
+#include "WopiTestServer.hpp"
+#include <Log.hpp>
+#include <Unit.hpp>
+#include <UnitHTTP.hpp>
+#include <helpers.hpp>
+
+#include <Poco/Net/HTTPRequest.h>
+
+class UnitWopiHttpHeaders : public WopiTestServer
+{
+    enum class Phase
+    {
+        Load,
+        Polling
+    } _phase;
+
+protected:
+    void assertCheckFileInfoRequest(const Poco::Net::HTTPRequest& request) 
override
+    {
+        assertHeaders(request);
+    }
+
+    void assertGetFileRequest(const Poco::Net::HTTPRequest& request) override
+    {
+        assertHeaders(request);
+        exitTest(TestResult::Ok); //TODO: Remove when we add put/rename cases.
+    }
+
+    void assertPutFileRequest(const Poco::Net::HTTPRequest& request) override
+    {
+        assertHeaders(request);
+        exitTest(TestResult::Ok);
+    }
+
+    void assertPutRelativeFileRequest(const Poco::Net::HTTPRequest& request) 
override
+    {
+        assertHeaders(request);
+        exitTest(TestResult::Ok);
+    }
+
+    void assertRenameFileRequest(const Poco::Net::HTTPRequest& request) 
override
+    {
+        assertHeaders(request);
+        exitTest(TestResult::Ok);
+    }
+
+    void assertHeaders(const Poco::Net::HTTPRequest& request) const
+    {
+        static const std::map<std::string, std::string> Headers{
+            { "Authorization", "Bearer xyz123abc456vwc789z" },
+            { "X-Requested-With", "XMLHttpRequest" },
+        };
+
+        for (const auto& pair : Headers)
+        {
+            LOK_ASSERT_MESSAGE("Request must have [" + pair.first + "]", 
request.has(pair.first));
+            LOK_ASSERT_EQUAL(pair.second, request[pair.first]);
+        }
+    }
+
+public:
+    UnitWopiHttpHeaders()
+        : _phase(Phase::Load)
+    {
+    }
+
+    void invokeTest() override
+    {
+        constexpr char testName[] = "UnitWopiHttpHeaders";
+
+        switch (_phase)
+        {
+            case Phase::Load:
+            {
+                // Technically, having an empty line in the header
+                // is invalid (it signifies the end of headers), but
+                // this is to illustrate that we are able to overcome
+                // such issues and generate valid headers.
+                const std::string params
+                    = "access_header=Authorization%3A%2520Bearer%"
+                      
"2520xyz123abc456vwc789z%250D%250A%250D%250AX-Requested-With%"
+                      
"3A%2520XMLHttpRequest&reuse_cookies=language%3Den-us%3AK%3DGS1&permission="
+                      "edit";
+
+                initWebsocket("/wopi/files/0?" + params);
+
+                helpers::sendTextFrame(*getWs()->getLOOLWebSocket(), "load 
url=" + getWopiSrc(),
+                                       testName);
+                SocketPoll::wakeupWorld();
+
+                _phase = Phase::Polling;
+                break;
+            }
+            case Phase::Polling:
+            {
+                // Just wait for the results.
+                break;
+            }
+        }
+    }
+};
+
+UnitBase* unit_create_wsd(void) { return new UnitWopiHttpHeaders(); }
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/WopiTestServer.hpp b/test/WopiTestServer.hpp
index 6fac6c336..305348d39 100644
--- a/test/WopiTestServer.hpp
+++ b/test/WopiTestServer.hpp
@@ -112,7 +112,18 @@ protected:
         Poco::URI uriReq(request.getURI());
         Poco::RegularExpression regInfo("/wopi/files/[0-9]");
         Poco::RegularExpression regContent("/wopi/files/[0-9]/contents");
-        LOG_INF("Fake wopi host request: " << uriReq.toString());
+
+        Log::StreamLogger logger = Log::info();
+        if (logger.enabled())
+        {
+            logger << "Fake wopi host request URI [" << uriReq.toString() << 
"]:\n";
+            for (const auto& pair : request)
+            {
+                logger << '\t' << pair.first << ": " << pair.second << " / ";
+            }
+
+            LOG_END(logger, true);
+        }
 
         // CheckFileInfo
         if (request.getMethod() == "GET" && regInfo.match(uriReq.getPath()))
diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 3ee9e4142..fcf853173 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -668,6 +668,9 @@ void FileServerRequestHandler::preprocessFile(const 
HTTPRequest& request,
     LOG_DBG("Preprocessing file: " << relPath);
     std::string preprocess = *getUncompressedFile(relPath);
 
+    // We need to pass certain parameters from the loleaflet html GET URI
+    // to the embedded document URI. Here we extract those params
+    // from the GET URI and set them in the generated html (see 
loleaflet.html.m4).
     HTMLForm form(request, message);
     const std::string accessToken = form.get("access_token", "");
     const std::string accessTokenTtl = form.get("access_token_ttl", "");
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 6c5a14878..bcd428ce7 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -232,7 +232,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const 
Poco::URI& uri, const std
 
     if (UnitWSD::get().createStorage(uri, jailRoot, jailPath, storage))
     {
-        LOG_INF("Storage load hooked.");
+        LOG_INF("Storage create hooked.");
         if (storage)
         {
             return storage;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to