From 9f6a58334ace6fb9e39a9bcc63fb4de5d10d4534 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A9ven=20Car?= <meven.car@collabora.com>
Date: Thu, 19 Dec 2024 09:15:09 +0100
Subject: [PATCH] Clean up and comments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Méven Car <meven.car@collabora.com>
Change-Id: I9f57ea5db4e338b04f217098ddf1c9231c8a06c7
---
 net/HttpRequest.hpp             | 66 ++++++++++++++++-----------------
 net/Socket.hpp                  |  2 +-
 wsd/ClientRequestDispatcher.cpp |  7 ++--
 3 files changed, 38 insertions(+), 37 deletions(-)

diff --git a/net/HttpRequest.hpp b/net/HttpRequest.hpp
index eb0474025773e..098fd3cb5d4cd 100644
--- a/net/HttpRequest.hpp
+++ b/net/HttpRequest.hpp
@@ -1455,25 +1455,25 @@ class Session final : public ProtocolHandlerInterface
 
     void callOnFinished()
     {
-        if (_onFinished)
+        if (!_onFinished)
+            return;
+
+        LOG_TRC("onFinished calling client");
+        auto self = shared_from_this();
+        try
         {
-            LOG_TRC("onFinished calling client");
-            auto self = shared_from_this();
-            try
-            {
-                [[maybe_unused]] const auto references = self.use_count();
-                assert(references > 1 && "Expected more than 1 reference to http::Session.");
+            [[maybe_unused]] const auto references = self.use_count();
+            assert(references > 1 && "Expected more than 1 reference to http::Session.");
 
-                _onFinished(std::static_pointer_cast<Session>(self));
+            _onFinished(std::static_pointer_cast<Session>(self));
 
-                assert(self.use_count() > 1 &&
-                        "Erroneously onFinish reset 'this'. Use 'addCallback()' on the "
-                        "SocketPoll to reset on idle instead.");
-            }
-            catch (const std::exception& exc)
-            {
-                LOG_ERR("Error while invoking onFinished client callback: " << exc.what());
-            }
+            assert(self.use_count() > 1 &&
+                    "Erroneously onFinish reset 'this'. Use 'addCallback()' on the "
+                    "SocketPoll to reset on idle instead.");
+        }
+        catch (const std::exception& exc)
+        {
+            LOG_ERR("Error while invoking onFinished client callback: " << exc.what());
         }
     }
 
@@ -1628,23 +1628,24 @@ class Session final : public ProtocolHandlerInterface
 
     void callOnConnectFail()
     {
-        if (_onConnectFail) {
-            auto self = shared_from_this();
-            try
-            {
-                [[maybe_unused]] const auto references = self.use_count();
-                assert(references > 1 && "Expected more than 1 reference to http::Session.");
+        if (!_onConnectFail)
+            return;
 
-                _onConnectFail(std::static_pointer_cast<Session>(self));
+        auto self = shared_from_this();
+        try
+        {
+            [[maybe_unused]] const auto references = self.use_count();
+            assert(references > 1 && "Expected more than 1 reference to http::Session.");
 
-                assert(self.use_count() > 1 &&
-                        "Erroneously onConnectFail reset 'this'. Use 'addCallback()' on the "
-                        "SocketPoll to reset on idle instead.");
-            }
-            catch (const std::exception& exc)
-            {
-                LOG_ERR("Error while invoking onConnectFail client callback: " << exc.what());
-            }
+            _onConnectFail(std::static_pointer_cast<Session>(self));
+
+            assert(self.use_count() > 1 &&
+                    "Erroneously onConnectFail reset 'this'. Use 'addCallback()' on the "
+                    "SocketPoll to reset on idle instead.");
+        }
+        catch (const std::exception& exc)
+        {
+            LOG_ERR("Error while invoking onConnectFail client callback: " << exc.what());
         }
     }
 
@@ -1676,9 +1677,8 @@ class Session final : public ProtocolHandlerInterface
         }
 
         _connected = false;
-        if (_response) {
+        if (_response)
             _response->finish();
-        }
 
         _fd = -1; // No longer our socket fd.
     }
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 4bbc85103437d..eb1cf73661ed2 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -1572,7 +1572,7 @@ class StreamSocket : public Socket,
 
         if (closed)
         {
-            LOG_TRC("Closed. Firing onDisconnect." << "hanging up? " << (events & POLLHUP) );
+            LOG_TRC("Closed. Firing onDisconnect.");
             _socketHandler->onDisconnect();
             setClosed(disposition);
         }
diff --git a/wsd/ClientRequestDispatcher.cpp b/wsd/ClientRequestDispatcher.cpp
index a6a779a0d3ac2..0c72bb0e2933b 100644
--- a/wsd/ClientRequestDispatcher.cpp
+++ b/wsd/ClientRequestDispatcher.cpp
@@ -1234,7 +1234,8 @@ bool ClientRequestDispatcher::handleWopiAccessCheckRequest(const Poco::Net::HTTP
         sendResult(CheckStatus::NoScheme);
         return true;
     }
-    else if (protocol != http::Session::Protocol::HttpSsl)
+    // if the wopi hosts uses https, so must cool or it will have Mixed Content
+    else if (protocol == http::Session::Protocol::HttpSsl && !request.getURI().starts_with("https://"))
     {
         sendResult(CheckStatus::NotHttps);
         return true;
@@ -1247,11 +1248,11 @@ bool ClientRequestDispatcher::handleWopiAccessCheckRequest(const Poco::Net::HTTP
     bool wopiHostAllowed = false;
     if (Util::iequal(ConfigUtil::getString("storage.wopi.alias_groups[@mode]", "first"), "first"))
         // if first mode was selected and wopi Hosts are empty
-        // the domain is allowed, as it will be the "first"
+        // the domain is allowed, as it will be the effective "first" host
         wopiHostAllowed = HostUtil::isWopiHostsEmpty();
 
     if (!wopiHostAllowed) {
-        // port and scheme from wopi host config are currently ignored
+        // port and scheme from wopi host config are currently ignored by HostUtil
         LOG_TRC("Wopi Access Check, matching allowed wopi host for host " << host);
         wopiHostAllowed = HostUtil::allowedWopiHost(host);
     }