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

diff --git a/src/common/remotepermissions.cpp b/src/common/remotepermissions.cpp
index ce39460b512b46daf97b692fa6f2cc2979d076f2..ad777b36e103c27c4fdf4e5ae3b1afe08a854d83 100644 (file)
--- 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 f0cceb3db6ce6a26ce0e469c38eef2308e5d5983..4f62cca3cc07ab1e5b8182626129b8f3c83f2225 100644 (file)
--- a/src/common/remotepermissions.h
+++ b/src/common/remotepermissions.h
@@ -58,11 +58,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 e61ef0970b67faa4ad5efeea7a4ff9079819b5fc..ce6cc234b18564436fd2bc72ebb1a384e431b3c5 100644 (file)
--- 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 93efdd0c827a2c1dccd97e8899f85b5b2a6de9bb..d7aa729a99e1d314484f3ace01c28f4401eab24f 100644 (file)
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -239,7 +239,7 @@ static void propertyMapToFileStat(const QMap<QString, QString> &map, RemoteInfo
         } else if (property == "dDC") {
             result.directDownloadCookies = value;
         } else if (property == "permissions") {
-            result.remotePerm = RemotePermissions(value);
+            result.remotePerm = RemotePermissions::fromServerString(value);
         } else if (property == "checksums") {
             result.checksumHeader = findBestChecksum(value.toUtf8());
         } else if (property == "share-types" && !value.isEmpty()) {
@@ -263,7 +263,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 fdcef6860590b72a90d3ed54ee2536cbd23eaf26..696fd3db66231181b460daf30d8663d8559c0d79 100644 (file)
--- 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 61f00fffd4f863d9fa9ff975dfef3b3d4c2af894..de3a415bc325c8c0772af73b5705349d288a5be1 100644 (file)
--- 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));