Conflicts: Add user name to conflict file name #6325

For the case of uploading conflict files only.

src/common/utility.cpp

@@ -553,7 +553,8 @@ QUrl Utility::concatUrlPath(const QUrl &url, const QString &concatPath,
     return tmpUrl;
 }
 
-QString Utility::makeConflictFileName(const QString &fn, const QDateTime &dt)
+QString Utility::makeConflictFileName(
+    const QString &fn, const QDateTime &dt, const QString &user)
 {
     QString conflictFileName(fn);
     // Add _conflict-XXXX  before the extension.
@@ -562,9 +563,15 @@ QString Utility::makeConflictFileName(const QString &fn, const QDateTime &dt)
     if (dotLocation <= conflictFileName.lastIndexOf('/') + 1) {
         dotLocation = conflictFileName.size();
     }
-    QString timeString = dt.toString("yyyyMMdd-hhmmss");
 
-    conflictFileName.insert(dotLocation, "_conflict-" + timeString);
+    QString conflictMarker = QStringLiteral("_conflict-");
+    if (!user.isEmpty()) {
+        conflictMarker.append(sanitizeForFileName(user));
+        conflictMarker.append('-');
+    }
+    conflictMarker.append(dt.toString("yyyyMMdd-hhmmss"));
+
+    conflictFileName.insert(dotLocation, conflictMarker);
     return conflictFileName;
 }
 
@@ -606,4 +613,19 @@ QByteArray Utility::conflictFileBaseName(const QByteArray &conflictName)
     return "";
 }
 
+QString Utility::sanitizeForFileName(const QString &name)
+{
+    const auto invalid = QStringLiteral("/?<>\\:*|\"");
+    QString result;
+    result.reserve(name.size());
+    for (const auto c : name) {
+        if (!invalid.contains(c)
+            && c.category() != QChar::Other_Control
+            && c.category() != QChar::Other_Format) {
+            result.append(c);
+        }
+    }
+    return result;
+}
+
 } // namespace OCC

src/common/utility.h

@@ -182,9 +182,24 @@ namespace Utility {
          with the given parent. If no parent is specified, the caller must destroy the settings */
     OCSYNC_EXPORT std::unique_ptr<QSettings> settingsWithGroup(const QString &group, QObject *parent = 0);
 
+    /** Sanitizes a string that shall become part of a filename.
+     *
+     * Filters out reserved characters like
+     * - unicode control and format characters
+     * - reserved characters: /, ?, <, >, \, :, *, |, and "
+     *
+     * Warning: This does not sanitize the whole resulting string, so
+     * - unix reserved filenames ('.', '..')
+     * - trailing periods and spaces
+     * - windows reserved filenames ('CON' etc)
+     * will pass unchanged.
+     */
+    OCSYNC_EXPORT QString sanitizeForFileName(const QString &name);
+
     /** Returns a file name based on \a fn that's suitable for a conflict.
      */
-    OCSYNC_EXPORT QString makeConflictFileName(const QString &fn, const QDateTime &dt);
+    OCSYNC_EXPORT QString makeConflictFileName(
+        const QString &fn, const QDateTime &dt, const QString &user);
 
     /** Returns whether a file name indicates a conflict file
      */

src/libsync/owncloudpropagator.cpp

@@ -732,8 +732,11 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item,
 
     QString renameError;
     auto conflictModTime = FileSystem::getModTime(fn);
+    QString conflictUserName;
+    if (account()->capabilities().uploadConflictFiles())
+        conflictUserName = account()->davDisplayName();
     QString conflictFileName = Utility::makeConflictFileName(
-        item->_file, Utility::qDateTimeFromTime_t(conflictModTime));
+        item->_file, Utility::qDateTimeFromTime_t(conflictModTime), conflictUserName);
     QString conflictFilePath = getFilePath(conflictFileName);
 
     emit touchedFile(fn);

test/testsyncconflict.cpp

@@ -80,6 +80,12 @@ private slots:
         fakeFolder.remoteModifier().appendByte("A/a2");
         fakeFolder.remoteModifier().appendByte("A/a2");
         QVERIFY(fakeFolder.syncOnce());
+
+        // Verify that the conflict names don't have the user name
+        for (const auto &name : findConflicts(fakeFolder.currentLocalState().children["A"])) {
+            QVERIFY(!name.contains(fakeFolder.syncEngine().account()->davUser()));
+        }
+
         QVERIFY(expectAndWipeConflict(fakeFolder.localModifier(), fakeFolder.currentLocalState(), "A/a1"));
         QVERIFY(expectAndWipeConflict(fakeFolder.localModifier(), fakeFolder.currentLocalState(), "A/a2"));
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
@@ -121,6 +127,9 @@ private slots:
         QCOMPARE(conflictMap.size(), 2);
         QCOMPARE(Utility::conflictFileBaseName(conflictMap[a1FileId].toUtf8()), QByteArray("A/a1"));
 
+        // Check that the conflict file contains the username
+        QVERIFY(conflictMap[a1FileId].contains(QString("-%1-").arg(fakeFolder.syncEngine().account()->davUser())));
+
         QCOMPARE(remote.find(conflictMap[a1FileId])->contentChar, 'L');
         QCOMPARE(remote.find("A/a1")->contentChar, 'R');
 

test/testutility.cpp

@@ -187,7 +187,28 @@ private slots:
         qunsetenv("OWNCLOUD_TEST_CASE_PRESERVING");
     }
 
+    void testSanitizeForFileName_data()
+    {
+        QTest::addColumn<QString>("input");
+        QTest::addColumn<QString>("output");
+
+        QTest::newRow("")
+            << "foobar"
+            << "foobar";
+        QTest::newRow("")
+            << "a/b?c<d>e\\f:g*h|i\"j"
+            << "abcdefghij";
+        QTest::newRow("")
+            << QString::fromLatin1("a\x01 b\x1f c\x80 d\x9f")
+            << "a b c d";
+    }
 
+    void testSanitizeForFileName()
+    {
+        QFETCH(QString, input);
+        QFETCH(QString, output);
+        QCOMPARE(sanitizeForFileName(input), output);
+    }
 };
 
 QTEST_GUILESS_MAIN(TestUtility)