RemotePermissions: Fix empty vs null

Fixes two bugs that appeared since the introduction of the struct:
- when reading permissions from the journal, null ("") was read as
  empty-not-null
- when reading permissinos from the server, empty ("") was read as null

Addresses #4608

6 files changed, 44 insertions, 22 deletions

diff --git a/src/common/remotepermissions.cpp b/src/common/remotepermissions.cpp
index ce39460b5..ad777b36e 100644
--- a/src/common/remotepermissions.cpp
+++ b/src/common/remotepermissions.cpp
@@ -27,7 +27,7 @@ static const char letters[] = " WDNVCKRSMm";
 template <typename Char>
 void RemotePermissions::fromArray(const Char *p)
 {
-    _value = p ? notNullMask : 0;
+    _value = notNullMask;
     if (!p)
         return;
     while (*p) {
@@ -37,17 +37,7 @@ void RemotePermissions::fromArray(const Char *p)
     }
 }
 
-RemotePermissions::RemotePermissions(const char *p)
-{
-    fromArray(p);
-}
-
-RemotePermissions::RemotePermissions(const QString &s)
-{
-    fromArray(s.isEmpty() ? nullptr : s.utf16());
-}
-
-QByteArray RemotePermissions::toString() const
+QByteArray RemotePermissions::toDbValue() const
 {
     QByteArray result;
     if (isNull())
@@ -64,4 +54,25 @@ QByteArray RemotePermissions::toString() const
     return result;
 }
 
+QByteArray RemotePermissions::toString() const
+{
+    return toDbValue();
+}
+
+RemotePermissions RemotePermissions::fromDbValue(const QByteArray &value)
+{
+    if (value.isEmpty())
+        return RemotePermissions();
+    RemotePermissions perm;
+    perm.fromArray(value.constData());
+    return perm;
+}
+
+RemotePermissions RemotePermissions::fromServerString(const QString &value)
+{
+    RemotePermissions perm;
+    perm.fromArray(value.utf16());
+    return perm;
+}
+
 } // namespace OCC
diff --git a/src/common/remotepermissions.h b/src/common/remotepermissions.h
index 2b34dcbf0..19f5a4e5f 100644
--- a/src/common/remotepermissions.h
+++ b/src/common/remotepermissions.h
@@ -57,11 +57,22 @@ public:
         // (by setting forceRemoteDiscovery in SyncJournalDb::checkConnect)
         PermissionsCount = IsMountedSub
     };
+
+    /// null permissions
     RemotePermissions() = default;
-    explicit RemotePermissions(const char *);
-    explicit RemotePermissions(const QString &);
 
+    /// array with one character per permission, "" is null, " " is non-null but empty
+    QByteArray toDbValue() const;
+
+    /// output for display purposes, no defined format (same as toDbValue in practice)
     QByteArray toString() const;
+
+    /// read value that was written with toDbValue()
+    static RemotePermissions fromDbValue(const QByteArray &);
+
+    /// read a permissions string received from the server, never null
+    static RemotePermissions fromServerString(const QString &);
+
     bool hasPermission(Permissions p) const
     {
         return _value & (1 << static_cast<int>(p));
diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp
index 9054951f7..cf497ef9c 100644
--- a/src/common/syncjournaldb.cpp
+++ b/src/common/syncjournaldb.cpp
@@ -58,7 +58,7 @@ static void fillFileRecordFromGetQuery(SyncJournalFileRecord &rec, SqlQuery &que
     rec._type = static_cast<ItemType>(query.intValue(3));
     rec._etag = query.baValue(4);
     rec._fileId = query.baValue(5);
-    rec._remotePerm = RemotePermissions(query.baValue(6).constData());
+    rec._remotePerm = RemotePermissions::fromDbValue(query.baValue(6));
     rec._fileSize = query.int64Value(7);
     rec._serverHasIgnoredFiles = (query.intValue(8) > 0);
     rec._checksumHeader = query.baValue(9);
diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index 4198d3a9c..dcfb8eadc 100644
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -331,7 +331,7 @@ static void propertyMapToFileStat(const QMap<QString, QString> &map, csync_file_
         } else if (property == "dDC") {
             file_stat->directDownloadCookies = value.toUtf8();
         } else if (property == "permissions") {
-            file_stat->remotePerm = RemotePermissions(value);
+            file_stat->remotePerm = RemotePermissions::fromServerString(value);
         } else if (property == "checksums") {
             file_stat->checksumHeader = findBestChecksum(value.toUtf8());
         } else if (property == "share-types" && !value.isEmpty()) {
@@ -355,7 +355,7 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file, con
         // The first entry is for the folder itself, we should process it differently.
         _ignoredFirst = true;
         if (map.contains("permissions")) {
-            RemotePermissions perm(map.value("permissions"));
+            auto perm = RemotePermissions::fromServerString(map.value("permissions"));
             emit firstDirectoryPermissions(perm);
             _isExternalStorage = perm.hasPermission(RemotePermissions::IsMounted);
         }
diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp
index a5a3470b2..57aac6eaf 100644
--- a/test/testpermissions.cpp
+++ b/test/testpermissions.cpp
@@ -16,7 +16,7 @@ static void applyPermissionsFromName(FileInfo &info) {
     static QRegularExpression rx("_PERM_([^_]*)_[^/]*$");
     auto m = rx.match(info.name);
     if (m.hasMatch()) {
-        info.permissions = RemotePermissions(m.captured(1));
+        info.permissions = RemotePermissions::fromServerString(m.captured(1));
     }
 
     for (FileInfo &sub : info.children)
diff --git a/test/testsyncjournaldb.cpp b/test/testsyncjournaldb.cpp
index 237b3b75a..6abf47761 100644
--- a/test/testsyncjournaldb.cpp
+++ b/test/testsyncjournaldb.cpp
@@ -57,7 +57,7 @@ private slots:
         record._type = ItemTypeDirectory;
         record._etag = "789789";
         record._fileId = "abcd";
-        record._remotePerm = RemotePermissions("RW");
+        record._remotePerm = RemotePermissions::fromDbValue("RW");
         record._fileSize = 213089055;
         record._checksumHeader = "MD5:mychecksum";
         QVERIFY(_db.setFileRecord(record));
@@ -79,7 +79,7 @@ private slots:
         record._type = ItemTypeFile;
         record._etag = "789FFF";
         record._fileId = "efg";
-        record._remotePerm = RemotePermissions("NV");
+        record._remotePerm = RemotePermissions::fromDbValue("NV");
         record._fileSize = 289055;
         _db.setFileRecordMetadata(record);
         QVERIFY(_db.getFileRecord(QByteArrayLiteral("foo"), &storedRecord));
@@ -96,7 +96,7 @@ private slots:
         {
             SyncJournalFileRecord record;
             record._path = "foo-checksum";
-            record._remotePerm = RemotePermissions("RW");
+            record._remotePerm = RemotePermissions::fromDbValue(" ");
             record._checksumHeader = "MD5:mychecksum";
             record._modtime = Utility::qDateTimeToTime_t(QDateTime::currentDateTimeUtc());
             QVERIFY(_db.setFileRecord(record));
@@ -117,7 +117,7 @@ private slots:
         {
             SyncJournalFileRecord record;
             record._path = "foo-nochecksum";
-            record._remotePerm = RemotePermissions("RWN");
+            record._remotePerm = RemotePermissions();
             record._modtime = Utility::qDateTimeToTime_t(QDateTime::currentDateTimeUtc());
 
             QVERIFY(_db.setFileRecord(record));