Fix resolving NTFS symbolic links that point to UNC shares

Without this change, the target of a symbolic link that points to a UNC
share would include UNC in the target path, and not be correctly made
absolute.

Add a relevant test case, and use the opportunity to factor out the
helper code that creates NTFS symlinks into a function that takes
care of error handling.

The file created with the new test case only gets cleaned up correctly
when passing the file path into QDir::rmdir, which is either way the
right thing to do.

[ChangeLog][QtCore][QFileInfo] Fixed resolving of symbolic links to UNC
shares on NTFS file systems.

Change-Id: I9ba75d627aedf7c4cc289f0cb71088d795d30d8a
Fixes: QTBUG-73688
Task-number: QTBUG-63970
Task-number: QTBUG-30401
Task-number: QTBUG-20791
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/io/qfilesystemengine_win.cpp

@@ -310,9 +310,18 @@ static QString readSymLink(const QFileSystemEntry &link)
                 const wchar_t* PathBuffer = &rdb->SymbolicLinkReparseBuffer.PathBuffer[offset];
                 result = QString::fromWCharArray(PathBuffer, length);
             }
-            // cut-off "//?/" and "/??/"
+            // cut-off "\\?\" and "\??\"
-            if (result.size() > 4 && result.at(0) == QLatin1Char('\\') && result.at(2) == QLatin1Char('?') && result.at(3) == QLatin1Char('\\'))
+            if (result.size() > 4
+                    && result.at(0) == QLatin1Char('\\')
+                    && result.at(2) == QLatin1Char('?')
+                    && result.at(3) == QLatin1Char('\\')) {
                 result = result.mid(4);
+                // cut off UNC in addition when the link points at a UNC share
+                // in which case we need to prepend another backslash to get \\server\share
+                if (result.leftRef(3) == QLatin1String("UNC")) {
+                    result.replace(0, 3, QLatin1Char('\\'));
+                }
+            }
         }
         free(rdb);
         CloseHandle(handle);

tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp

@@ -95,6 +95,33 @@ inline bool qIsLikelyToBeNfs(const QString &path)
 #endif
 }
 
+#if defined(Q_OS_WIN) && !defined(Q_OS_WINRT)
+enum NtfsTargetType {
+    NtfsTargetFile = 0x0,
+    NtfsTargetDir = 0x1
+};
+
+static bool createNtfsSymLinkHelper(const QString &path, const QString &target, NtfsTargetType targetType)
+{
+    DWORD dwFlags = targetType;
+    DWORD err = ERROR_SUCCESS;
+
+    SetLastError(0);
+    const bool result = CreateSymbolicLink(reinterpret_cast<const wchar_t*>(path.utf16()),
+                                           reinterpret_cast<const wchar_t*>(target.utf16()), dwFlags);
+    err = GetLastError();
+
+    // CreateSymbolicLink can return TRUE & still fail to create the link,
+    // the error code in that case might be ERROR_PRIVILEGE_NOT_HELD (1314)
+    if (!result || err != ERROR_SUCCESS) {
+        qWarning() << "Error creating NTFS-symlink from:" << path << "to" << target << ":" << qt_error_string(err);
+
+        return false;
+    }
+    return true;
+}
+#endif
+
 static QString seedAndTemplate()
 {
     QString base;
@@ -704,18 +731,12 @@ void tst_QFileInfo::canonicalFilePath()
 
 #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT)
     {
-        // CreateSymbolicLink can return TRUE & still fail to create the link,
-        // the error code in that case is ERROR_PRIVILEGE_NOT_HELD (1314)
-        SetLastError(0);
         const QString linkTarget = QStringLiteral("res");
-        BOOL ret = CreateSymbolicLink((wchar_t*)linkTarget.utf16(), (wchar_t*)m_resourcesDir.utf16(), 1);
+        BOOL ret = createNtfsSymLinkHelper(linkTarget, m_resourcesDir, NtfsTargetDir);
-        DWORD dwErr = GetLastError();
         if (!ret)
             QSKIP("Symbolic links aren't supported by FS");
         QString currentPath = QDir::currentPath();
         bool is_res_Current = QDir::setCurrent(linkTarget);
-        if (!is_res_Current && dwErr == 1314)
-            QSKIP("Not enough privilages to create Symbolic links");
         QCOMPARE(is_res_Current, true);
         const QString actualCanonicalPath = QFileInfo("file1").canonicalFilePath();
         QVERIFY(QDir::setCurrent(currentPath));
@@ -1482,19 +1503,12 @@ void tst_QFileInfo::ntfsJunctionPointsAndSymlinks_data()
         file.open(QIODevice::ReadWrite);
         file.close();
 
-        DWORD err = ERROR_SUCCESS ;
+        bool result = true;
         if (!pwd.exists("abs_symlink"))
-            if (!CreateSymbolicLink((wchar_t*)absSymlink.utf16(),(wchar_t*)absTarget.utf16(),0x1))
+            result = createNtfsSymLinkHelper(absSymlink, absTarget, NtfsTargetDir);
-                err = GetLastError();
+        if (result && !pwd.exists(relSymlink))
-        if (err == ERROR_SUCCESS && !pwd.exists(relSymlink))
+            result = createNtfsSymLinkHelper(relSymlink, relTarget, NtfsTargetDir);
-            if (!CreateSymbolicLink((wchar_t*)relSymlink.utf16(),(wchar_t*)relTarget.utf16(),0x1))
+        if (!result) {
-                err = GetLastError();
-        if (err != ERROR_SUCCESS) {
-            wchar_t errstr[0x100];
-            DWORD count = FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM,
-                0, err, 0, errstr, 0x100, 0);
-            QString error(QString::fromWCharArray(errstr, count));
-            qWarning() << error;
             //we need at least one data set for the test not to assert fail when skipping _data function
             QDir target("target");
             QTest::newRow("dummy") << target.path() << false << "" << target.canonicalPath();
@@ -1517,13 +1531,21 @@ void tst_QFileInfo::ntfsJunctionPointsAndSymlinks_data()
         QString relSymlink = "rel_symlink.cpp";
         QString relToRelTarget = QDir::toNativeSeparators(relativeDir.relativeFilePath(target.absoluteFilePath()));
         QString relToRelSymlink = "relative/rel_symlink";
-        QVERIFY(pwd.exists("abs_symlink.cpp") || CreateSymbolicLink((wchar_t*)absSymlink.utf16(),(wchar_t*)absTarget.utf16(),0x0));
+        QVERIFY(pwd.exists("abs_symlink.cpp") || createNtfsSymLinkHelper(absSymlink, absTarget, NtfsTargetFile));
-        QVERIFY(pwd.exists(relSymlink) || CreateSymbolicLink((wchar_t*)relSymlink.utf16(),(wchar_t*)relTarget.utf16(),0x0));
+        QVERIFY(pwd.exists(relSymlink) || createNtfsSymLinkHelper(relSymlink, relTarget, NtfsTargetFile));
-        QVERIFY(pwd.exists(relToRelSymlink) || CreateSymbolicLink((wchar_t*)relToRelSymlink.utf16(), (wchar_t*)relToRelTarget.utf16(),0x0));
+        QVERIFY(pwd.exists(relToRelSymlink) || createNtfsSymLinkHelper(relToRelSymlink, relToRelTarget, NtfsTargetFile));
         QTest::newRow("absolute file symlink") << absSymlink << true << QDir::fromNativeSeparators(absTarget) << target.canonicalFilePath();
         QTest::newRow("relative file symlink") << relSymlink << true << QDir::fromNativeSeparators(absTarget) << target.canonicalFilePath();
         QTest::newRow("relative to relative file symlink") << relToRelSymlink << true << QDir::fromNativeSeparators(absTarget) << target.canonicalFilePath();
     }
+    {
+        // Symlink to UNC share
+        pwd.mkdir("unc");
+        QString uncTarget = QStringLiteral("//") + QtNetworkSettings::winServerName() + "/testshare";
+        QString uncSymlink = QDir::toNativeSeparators(pwd.absolutePath().append("\\unc\\link_to_unc"));
+        QVERIFY(pwd.exists("link_to_unc") || createNtfsSymLinkHelper(uncSymlink, uncTarget, NtfsTargetDir));
+        QTest::newRow("UNC symlink") << uncSymlink << true << uncTarget << uncTarget;
+    }
 
     //Junctions
     QString target = "target";
@@ -1570,7 +1592,7 @@ void tst_QFileInfo::ntfsJunctionPointsAndSymlinks()
     // Ensure that junctions, mountpoints are removed. If this fails, do not remove
     // temporary directory to prevent it from trashing the system.
     if (fi.isDir()) {
-        if (!QDir().rmdir(fi.fileName())) {
+        if (!QDir().rmdir(fi.filePath())) {
             qWarning("Unable to remove NTFS junction '%s'', keeping '%s'.",
                      qPrintable(fi.fileName()), qPrintable(QDir::toNativeSeparators(m_dir.path())));
             m_dir.setAutoRemove(false);