From 29cfaa7682ad5bbd4940d8c15132d33c2dbb9242 Mon Sep 17 00:00:00 2001
From: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Date: Wed, 27 Dec 2023 06:37:01 -0500
Subject: [PATCH] wsd: quarantine: absolute-path and better logging

The quarantine path should be an absolute path.

Unfortunately, because we had relative=true in
the path config, we couldn't detect empty configs.
This is because with relative=true the getter
would create a path based on the current directory
and the config value, which would default to
"quarantine" when empty.
This would result in /opt/cool/quarantine or
/usr/bin/quarantine when in fact the path is
really empty.

Now, the config has relative=false and
there is no default. In addition, we
warn if the path is no absolute.

Change-Id: I1b3eb5e76866a12d214b47e6556942715a210e54
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
---
 coolwsd.xml.in         | 2 +-
 wsd/COOLWSD.cpp        | 9 +++++++++
 wsd/QuarantineUtil.cpp | 2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/coolwsd.xml.in b/coolwsd.xml.in
index bb6c388d28..bb98979a52 100644
--- a/coolwsd.xml.in
+++ b/coolwsd.xml.in
@@ -284,7 +284,7 @@
     <quarantine_files desc="Files are stored here to be examined later in cases of crashes or similar situation." default="false" enable="false">
         <limit_dir_size_mb desc="Maximum directory size. On exceeding the specified limit, older files will be deleted." default="250" type="uint"></limit_dir_size_mb>
         <max_versions_to_maintain desc="How many versions of the same file to keep." default="2" type="uint"></max_versions_to_maintain>
-        <path desc="Path to directory under which quarantined files will be stored" type="path" relative="true" default="quarantine"></path>
+        <path desc="Absolute path of the directory under which quarantined files will be stored. Do not use a relative path." type="path" relative="false"></path>
         <expiry_min desc="Time in mins after quarantined files will be deleted." type="int" default="30"></expiry_min>
     </quarantine_files>
 
diff --git a/wsd/COOLWSD.cpp b/wsd/COOLWSD.cpp
index 5c61173853..cea8f9d4c6 100644
--- a/wsd/COOLWSD.cpp
+++ b/wsd/COOLWSD.cpp
@@ -2698,6 +2698,7 @@ void COOLWSD::innerInitialize(Application& self)
     if (getConfigValue<bool>(conf, "quarantine_files[@enable]", false))
     {
         std::string path = Util::trimmed(getPathFromConfig("quarantine_files.path"));
+        LOG_INF("Quarantine path is set to [" << path << "] in config");
         if (path.empty())
         {
             LOG_WRN("Quarantining is enabled via quarantine_files config, but no path is set in "
@@ -2708,6 +2709,10 @@ void COOLWSD::innerInitialize(Application& self)
             if (path[path.size() - 1] != '/')
                 path += '/';
 
+            if (path[0] != '/')
+                LOG_WRN("Quarantine path is relative. Please use an absolute path for better "
+                        "reliability");
+
             Poco::File p(path);
             try
             {
@@ -2729,6 +2734,10 @@ void COOLWSD::innerInitialize(Application& self)
             }
         }
     }
+    else
+    {
+        LOG_INF("Quarantine is disabled in config");
+    }
 
     NumPreSpawnedChildren = getConfigValue<int>(conf, "num_prespawn_children", 1);
     if (NumPreSpawnedChildren < 1)
diff --git a/wsd/QuarantineUtil.cpp b/wsd/QuarantineUtil.cpp
index 4f358e9dc1..b5f460e5c1 100644
--- a/wsd/QuarantineUtil.cpp
+++ b/wsd/QuarantineUtil.cpp
@@ -79,7 +79,7 @@ void Quarantine::initialize(const std::string& path)
               });
 
     std::vector<StringToken> tokens;
-    for (const auto& file : files)
+    for (const std::string& file : files)
     {
         StringVector::tokenize(file.c_str(), file.size(), Delimiter, tokens);
         if (tokens.size() >= 3)