QFileSystemModel on Windows: consistently trim spaces and dots

On Windows, file- and directory-names that end with a space (or a dot)
are invalid, even though it's possible to create them. Same for names
that start with a space (but starting with dots is ok). Even Explorer
chokes when trying to then delete or rename such a file or directory,
and often the only way out of the mess is to operate on the 8.3 path.

In Qt, we only sometimes fixed such paths by removing trailing spaces
and dots. This still made it possible to create a "bad " directory in a
non-native file dialog, which could then not be deleted. The node tree
even ended up with both "bad " and "bad", neither of which was handled
correctly.

Fix this by always chopping trailing space and dots when adding or
modifying a node, amending 3693d3d2a1e76f6e6d41db340505e00c6ddaeda1.

Fortunately, this can be reproduced with QFileSystemModel, so add a test
that exercises it in the same way as QFileDialog does, e.g. both by
creating the misnamed directory directly, and by renaming a directory to
an invalid name.

Given that this is very broken on Windows Explorer as well, don't
cherry-pick this back into branches under strict change management.

Fixes: QTBUG-129028
Change-Id: Iec1edf9fd2548dda4567134a9b5cf3e298589c19
Reviewed-by: Wladimir Leuschner <wladimir.leuschner@qt.io>
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
(cherry picked from commit 53df22b42d73dbb48bc25ad05500993df56b928c)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/gui/itemmodels/qfilesystemmodel.cpp

@@ -310,6 +310,24 @@ static QString qt_GetLongPathName(const QString &strShortPath)
         return QDir::fromNativeSeparators(strShortPath);
     }
 }
+
+static inline void chopSpaceAndDot(QString &element)
+{
+    if (element == "."_L1 || element == ".."_L1)
+        return;
+    // On Windows, "filename    " and "filename" are equivalent and
+    // "filename  .  " and "filename" are equivalent
+    // "filename......." and "filename" are equivalent Task #133928
+    // whereas "filename  .txt" is still "filename  .txt"
+    while (element.endsWith(u'.') || element.endsWith(u' '))
+        element.chop(1);
+
+    // If a file is saved as ' Foo.txt', where the leading character(s)
+    // is an ASCII Space (0x20), it will be saved to the file system as 'Foo.txt'.
+    while (element.startsWith(u' '))
+        element.remove(0, 1);
+}
+
 #endif
 
 /*!
@@ -403,15 +421,10 @@ QFileSystemModelPrivate::QFileSystemNode *QFileSystemModelPrivate::node(const QS
         if (i == pathElements.size() - 1)
             elementPath.append(trailingSeparator);
 #ifdef Q_OS_WIN
-        // On Windows, "filename    " and "filename" are equivalent and
-        // "filename  .  " and "filename" are equivalent
-        // "filename......." and "filename" are equivalent Task #133928
-        // whereas "filename  .txt" is still "filename  .txt"
         // If after stripping the characters there is nothing left then we
         // just return the parent directory as it is assumed that the path
-        // is referring to the parent
-        while (element.endsWith(u'.') || element.endsWith(u' '))
-            element.chop(1);
+        // is referring to the parent.
+        chopSpaceAndDot(element);
         // Only filenames that can't possibly exist will be end up being empty
         if (element.isEmpty())
             return parent;
@@ -866,6 +879,12 @@ bool QFileSystemModel::setData(const QModelIndex &idx, const QVariant &value, in
     }
 
     QString newName = value.toString();
+#ifdef Q_OS_WIN
+    chopSpaceAndDot(newName);
+    if (newName.isEmpty())
+        return false;
+#endif
+
     QString oldName = idx.data().toString();
     if (newName == oldName)
         return true;
@@ -1428,17 +1447,24 @@ QModelIndex QFileSystemModel::mkdir(const QModelIndex &parent, const QString &na
     if (!parent.isValid())
         return parent;
 
+    QString fileName = name;
+#ifdef Q_OS_WIN
+    chopSpaceAndDot(fileName);
+    if (fileName.isEmpty())
+        return QModelIndex();
+#endif
+
     QDir dir(filePath(parent));
-    if (!dir.mkdir(name))
+    if (!dir.mkdir(fileName))
         return QModelIndex();
     QFileSystemModelPrivate::QFileSystemNode *parentNode = d->node(parent);
-    d->addNode(parentNode, name, QFileInfo());
-    Q_ASSERT(parentNode->children.contains(name));
-    QFileSystemModelPrivate::QFileSystemNode *node = parentNode->children[name];
+    d->addNode(parentNode, fileName, QFileInfo());
+    Q_ASSERT(parentNode->children.contains(fileName));
+    QFileSystemModelPrivate::QFileSystemNode *node = parentNode->children[fileName];
 #if QT_CONFIG(filesystemwatcher)
-    node->populate(d->fileInfoGatherer->getInfo(QFileInfo(dir.absolutePath() + QDir::separator() + name)));
+    node->populate(d->fileInfoGatherer->getInfo(QFileInfo(dir.absolutePath() + QDir::separator() + fileName)));
 #endif
-    d->addVisibleFiles(parentNode, QStringList(name));
+    d->addVisibleFiles(parentNode, QStringList(fileName));
     return d->index(node);
 }
 
@@ -1932,6 +1958,11 @@ void QFileSystemModelPrivate::fileSystemChanged(const QString &path,
         QExtendedInformation info = fileInfoGatherer->getInfo(update.second);
         bool previouslyHere = parentNode->children.contains(fileName);
         if (!previouslyHere) {
+#ifdef Q_OS_WIN
+            chopSpaceAndDot(fileName);
+            if (fileName.isEmpty())
+                continue;
+#endif
             addNode(parentNode, fileName, info.fileInfo());
         }
         QFileSystemModelPrivate::QFileSystemNode * node = parentNode->children.value(fileName);

tests/auto/gui/itemmodels/qfilesystemmodel/tst_qfilesystemmodel.cpp

@@ -114,6 +114,9 @@ private slots:
 
     void fileInfo();
 
+    void pathWithTrailingSpace_data();
+    void pathWithTrailingSpace();
+
 protected:
     bool createFiles(QFileSystemModel *model, const QString &test_path,
                      const QStringList &initial_files, int existingFileCount = 0,
@@ -1296,6 +1299,78 @@ void tst_QFileSystemModel::fileInfo()
     QCOMPARE(model.fileInfo(idx), QFileInfo(dirPath));
 }
 
+void tst_QFileSystemModel::pathWithTrailingSpace_data()
+{
+    QTest::addColumn<QString>("input");
+    QTest::addColumn<QString>("foundAs");
+
+    static constexpr bool windows =
+#ifdef Q_OS_WIN
+        true;
+#else
+        false;
+#endif
+    // Cover the various cases as documented at
+    // https://learn.microsoft.com/en-us/troubleshoot/windows-client/shell-experience/file-folder-name-whitespace-characters
+
+    QTest::addRow("trailing space") << "space "
+        << (windows ? "space" : "space ");
+    QTest::addRow("leading space") << " leadingspace"
+        << (windows ? "leadingspace" : " leadingspace");
+    QTest::addRow("trailing dot") << "dot."
+        << (windows ? "dot" : "dot.");
+    QTest::addRow("leading dot") << ".dot" << ".dot";
+
+    QTest::addRow("spacedot .") << "spacedot ."
+        << (windows ? "spacedot" : "spacedot .");
+    QTest::addRow("dotspace. ") << "dotspace. "
+        << (windows ? "dotspace" : "dotspace. ");
+    QTest::addRow("invisible") << "      "
+        << (windows ? "" : "      ");
+
+    QTest::addRow("leading 0x3000") << "\u3000-x3000" << "\u3000-x3000";
+    QTest::addRow("trailing 0x3000") << "x3000-\u3000" << "x3000-\u3000";
+}
+
+void tst_QFileSystemModel::pathWithTrailingSpace()
+{
+    QFETCH(const QString, input);
+    QFETCH(const QString, foundAs);
+
+    const bool validInput = !foundAs.isEmpty();
+
+    QTemporaryDir temp;
+
+    QFileSystemModel model;
+    model.setReadOnly(false);
+    const QModelIndex rootIndex = model.setRootPath(temp.path());
+    const QDir rootDir = model.rootDirectory();
+    const QString absoluteInput = rootDir.absoluteFilePath(input);
+    const QString absoluteFoundAs = validInput ? rootDir.absoluteFilePath(foundAs)
+                                               : rootDir.absolutePath();
+
+    const QModelIndex createdIndex = model.mkdir(rootIndex, input);
+    QCOMPARE(createdIndex.isValid(), validInput);
+    if (createdIndex.isValid())
+        QCOMPARE(model.filePath(createdIndex), absoluteFoundAs);
+    const QModelIndex foundIndex = model.index(absoluteInput);
+    QVERIFY(foundIndex.isValid());
+    QCOMPARE(model.filePath(foundIndex), absoluteFoundAs);
+
+    if (createdIndex.isValid())
+        QVERIFY(model.rmdir(createdIndex));
+
+    const QPersistentModelIndex newFolder = model.mkdir(rootIndex, "New Folder");
+    QVERIFY(newFolder.isValid());
+    QVERIFY(model.flags(newFolder) & Qt::ItemIsEditable);
+    QCOMPARE(model.setData(newFolder, input, Qt::EditRole), validInput);
+    if (validInput) {
+        const QModelIndex renamedIndex = model.index(absoluteInput);
+        QVERIFY(renamedIndex.isValid());
+    }
+}
+
+
 QTEST_MAIN(tst_QFileSystemModel)
 #include "tst_qfilesystemmodel.moc"