Private links: improve legacy fileid derivation #6745

Author: ckamm

src/common/syncjournalfilerecord.cpp

@@ -29,9 +29,20 @@ SyncJournalFileRecord::SyncJournalFileRecord()
 {
 }
 
-QByteArray SyncJournalFileRecord::numericFileId() const
+QByteArray SyncJournalFileRecord::legacyDeriveNumericFileId() const
 {
-    // Use the id up until the first non-numeric character
+    // The id property which is stored in _fileId is
+    //   leftpad_with_zero(fileid, 8) + instanceid
+    // so if it starts with a 0 we know the first 8 bytes
+    // can be taken.
+    if (_fileId.startsWith('0')) {
+        return _fileId.left(8);
+    }
+
+    // Otherwise we don't know exactly how long it is,
+    // use every digit until the first letter. The instanceid of
+    // oc >= 6 starts with "oc". This will break for older instances
+    // that have a digit as the first character of the instance id.
     for (int i = 0; i < _fileId.size(); ++i) {
         if (_fileId[i] < '0' || _fileId[i] > '9') {
             return _fileId.left(i);

src/common/syncjournalfilerecord.h

@@ -45,13 +45,18 @@ class OCSYNC_EXPORT SyncJournalFileRecord
         return !_path.isEmpty();
     }
 
-    /** Returns the numeric part of the full id in _fileId.
+    /** Returns a guess for the numeric part of the full id in _fileId.
      *
-     * On the server this is sometimes known as the internal file id.
+     * On the server this is sometimes known as the internal file id, the "fileid" DAV property.
      *
-     * It is used in the construction of private links.
+     * It is used in the fallback construction of private links.
+     *
+     * New code should not use this function: Request the fileid property from the server.
+     * In the future we might store the fileid instead of the id, but the migration is complex
+     * if we don't want to store both.
      */
-    QByteArray numericFileId() const;
+    QByteArray legacyDeriveNumericFileId() const;
+
     QDateTime modDateTime() const { return Utility::qDateTimeFromTime_t(_modtime); }
 
     QByteArray _path;

src/gui/owncloudgui.cpp

@@ -1125,7 +1125,7 @@ void ownCloudGui::slotShowShareDialog(const QString &sharePath, const QString &l
         w = _shareDialogs[localPath];
     } else {
         qCInfo(lcApplication) << "Opening share dialog" << sharePath << localPath << maxSharingPermissions;
-        w = new ShareDialog(accountState, sharePath, localPath, maxSharingPermissions, fileRecord.numericFileId(), startPage);
+        w = new ShareDialog(accountState, sharePath, localPath, maxSharingPermissions, fileRecord.legacyDeriveNumericFileId(), startPage);
         w->setAttribute(Qt::WA_DeleteOnClose, true);
 
         _shareDialogs[localPath] = w;

src/gui/protocolwidget.cpp

@@ -141,7 +141,7 @@ void ProtocolItem::openContextMenu(QPoint globalPos, QTreeWidgetItem *item, QWid
         // "Open in Browser" action
         auto openInBrowser = menu->addAction(ProtocolWidget::tr("Open in browser"));
         QObject::connect(openInBrowser, &QAction::triggered, parent, [parent, account, rec]() {
-            fetchPrivateLinkUrl(account, rec._path, rec.numericFileId(), parent,
+            fetchPrivateLinkUrl(account, rec._path, rec.legacyDeriveNumericFileId(), parent,
                 [parent](const QString &url) {
                     Utility::openBrowser(url, parent);
                 });

src/gui/socketapi.cpp

@@ -580,7 +580,7 @@ void SocketApi::fetchPrivateLinkUrlHelper(const QString &localFile, const std::f
     fetchPrivateLinkUrl(
         fileData.folder->accountState()->account(),
         fileData.serverRelativePath,
-        record.numericFileId(),
+        record.legacyDeriveNumericFileId(),
         this,
         targetFun);
 }

test/testsyncjournaldb.cpp

@@ -176,11 +176,15 @@ private slots:
 
         // Typical 8-digit padded id
         record._fileId = "00000001abcd";
-        QCOMPARE(record.numericFileId(), QByteArray("00000001"));
+        QCOMPARE(record.legacyDeriveNumericFileId(), QByteArray("00000001"));
+
+        // Typical 8-digit padded id with instanceid that starts with a digit
+        record._fileId = "00000001999";
+        QCOMPARE(record.legacyDeriveNumericFileId(), QByteArray("00000001"));
 
         // When the numeric id overflows the 8-digit boundary
         record._fileId = "123456789ocidblaabcd";
-        QCOMPARE(record.numericFileId(), QByteArray("123456789"));
+        QCOMPARE(record.legacyDeriveNumericFileId(), QByteArray("123456789"));
     }
 
     void testConflictRecord()