Let QDir::absoluteFilePath() use isAbsolutePath() for resource paths

Using QFileSystemEntry::isAbsolute() broke handling of resource paths.
Extended QDir::absoluteFilePath() tests to cover absolute resource path
and some UNC variants also resolved in the same fix.
Amend existing filePath tests to use drives where needed.

Task-number: QTBUG-68337
Change-Id: I4f02cf67828ad93e562857118f8442037f18bab7
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Edward Welbourne 2018-05-22 16:50:38 +02:00
parent b616a8a8cd
commit 27f1f84c1c
2 changed files with 89 additions and 51 deletions

View File

@ -715,6 +715,37 @@ QString QDir::dirName() const
return d->dirEntry.fileName();
}
#ifdef Q_OS_WIN
static int drivePrefixLength(const QString &path)
{
// Used to extract path's drive for use as prefix for an "absolute except for drive" path
const int size = path.length();
int drive = 2; // length of drive prefix
if (size > 1 && path.at(1).unicode() == ':') {
if (Q_UNLIKELY(!path.at(0).isLetter()))
return 0;
} else if (path.startsWith(QLatin1String("//"))) {
// UNC path; use its //server/share part as "drive" - it's as sane a
// thing as we can do.
for (int i = 2; i-- > 0; ) { // Scan two "path fragments":
while (drive < size && path.at(drive).unicode() == '/')
drive++;
if (drive >= size) {
qWarning("Base directory starts with neither a drive nor a UNC share: %s",
qUtf8Printable(QDir::toNativeSeparators(path)));
return 0;
}
while (drive < size && path.at(drive).unicode() != '/')
drive++;
}
} else {
return 0;
}
return drive;
}
#endif // Q_OS_WIN
/*!
Returns the path name of a file in the directory. Does \e not
check if the file actually exists in the directory; but see
@ -727,16 +758,27 @@ QString QDir::dirName() const
QString QDir::filePath(const QString &fileName) const
{
const QDirPrivate* d = d_ptr.constData();
if (isAbsolutePath(fileName))
// Mistrust our own isAbsolutePath() for real files; Q_OS_WIN needs a drive.
if (fileName.startsWith(QLatin1Char(':')) // i.e. resource path
? isAbsolutePath(fileName) : QFileSystemEntry(fileName).isAbsolute()) {
return fileName;
}
QString ret = d->dirEntry.filePath();
if (!fileName.isEmpty()) {
if (!ret.isEmpty() && ret[(int)ret.length()-1] != QLatin1Char('/') && fileName[0] != QLatin1Char('/'))
ret += QLatin1Char('/');
ret += fileName;
if (fileName.isEmpty())
return ret;
#ifdef Q_OS_WIN
if (fileName.startsWith(QLatin1Char('/')) || fileName.startsWith(QLatin1Char('\\'))) {
// Handle the "absolute except for drive" case (i.e. \blah not c:\blah):
const int drive = drivePrefixLength(ret);
return drive > 0 ? ret.leftRef(drive) % fileName : fileName;
}
return ret;
#endif // Q_OS_WIN
if (ret.isEmpty() || ret.endsWith(QLatin1Char('/')))
return ret % fileName;
return ret % QLatin1Char('/') % fileName;
}
/*!
@ -750,9 +792,11 @@ QString QDir::filePath(const QString &fileName) const
QString QDir::absoluteFilePath(const QString &fileName) const
{
const QDirPrivate* d = d_ptr.constData();
// Don't trust our own isAbsolutePath(); Q_OS_WIN needs a drive.
if (QFileSystemEntry(fileName).isAbsolute())
// Mistrust our own isAbsolutePath() for real files; Q_OS_WIN needs a drive.
if (fileName.startsWith(QLatin1Char(':')) // i.e. resource path
? isAbsolutePath(fileName) : QFileSystemEntry(fileName).isAbsolute()) {
return fileName;
}
d->resolveAbsoluteEntry();
const QString absoluteDirPath = d->absoluteDirEntry.filePath();
@ -760,35 +804,15 @@ QString QDir::absoluteFilePath(const QString &fileName) const
return absoluteDirPath;
#ifdef Q_OS_WIN
// Handle the "absolute except for drive" case (i.e. \blah not c:\blah):
int size = absoluteDirPath.length();
if ((fileName.startsWith(QLatin1Char('/'))
|| fileName.startsWith(QLatin1Char('\\')))
&& size > 1) {
if (fileName.startsWith(QLatin1Char('/')) || fileName.startsWith(QLatin1Char('\\'))) {
// Combine absoluteDirPath's drive with fileName
int drive = 2; // length of drive prefix
if (Q_UNLIKELY(absoluteDirPath.at(1).unicode() != ':')) {
// Presumably, absoluteDirPath is an UNC path; use its //server/share
// part as "drive" - it's as sane a thing as we can do.
for (int i = 2; i-- > 0; ) { // Scan two "path fragments":
while (drive < size && absoluteDirPath.at(drive).unicode() == '/')
drive++;
if (drive >= size) {
qWarning("Base directory starts with neither a drive nor a UNC share: %s",
qPrintable(QDir::toNativeSeparators(absoluteDirPath)));
return QString();
}
while (drive < size && absoluteDirPath.at(drive).unicode() != '/')
drive++;
}
// We'll append fileName, which starts with a slash; so omit trailing slash:
if (absoluteDirPath.at(drive).unicode() == '/')
drive--;
} else if (!absoluteDirPath.at(0).isLetter()) {
qWarning("Base directory's drive is not a letter: %s",
qPrintable(QDir::toNativeSeparators(absoluteDirPath)));
return QString();
}
return absoluteDirPath.leftRef(drive) % fileName;
const int drive = drivePrefixLength(absoluteDirPath);
if (Q_LIKELY(drive))
return absoluteDirPath.leftRef(drive) % fileName;
qWarning("Base directory's drive is not a letter: %s",
qUtf8Printable(QDir::toNativeSeparators(absoluteDirPath)));
return QString();
}
#endif // Q_OS_WIN
if (!absoluteDirPath.endsWith(QLatin1Char('/')))

View File

@ -56,6 +56,12 @@
#define Q_NO_SYMLINKS
#endif
#ifdef Q_OS_WIN
#define DRIVE "Q:"
#else
#define DRIVE
#endif
#ifdef QT_BUILD_INTERNAL
QT_BEGIN_NAMESPACE
@ -1385,14 +1391,12 @@ void tst_QDir::absoluteFilePath_data()
QTest::addColumn<QString>("expectedFilePath");
#if defined(Q_OS_WIN) && !defined(Q_OS_WINRT)
QTest::newRow("UNC") << "//machine" << "share" << "//machine/share";
QTest::newRow("Drive") << "c:/side/town" << "/my/way/home" << "c:/my/way/home";
#endif
#ifdef Q_OS_WIN
#define DRIVE "Q:"
#else
#define DRIVE
QTest::newRow("UNC-rel") << "//machine/share" << "dir" << "//machine/share/dir";
QTest::newRow("UNC-abs") << "//machine/share/path/to/blah" << "/dir" << "//machine/share/dir";
QTest::newRow("UNC-UNC") << "//machine/share/path/to/blah" << "//host/share/path" << "//host/share/path";
QTest::newRow("Drive-UNC") << "c:/side/town" << "//host/share/path" << "//host/share/path";
QTest::newRow("Drive-LTUNC") << "c:/side/town" << "\\/leaning\\toothpick/path" << "\\/leaning\\toothpick/path";
QTest::newRow("Drive-abs") << "c:/side/town" << "/my/way/home" << "c:/my/way/home";
#endif
QTest::newRow("0") << DRIVE "/etc" << "/passwd" << DRIVE "/passwd";
@ -1401,8 +1405,10 @@ void tst_QDir::absoluteFilePath_data()
QTest::newRow("3") << "relative" << "path" << QDir::currentPath() + "/relative/path";
QTest::newRow("4") << "" << "" << QDir::currentPath();
QTest::newRow("resource") << ":/prefix" << "foo.bar" << ":/prefix/foo.bar";
#undef DRIVE
// Resource paths are absolute:
QTest::newRow("resource-rel") << ":/prefix" << "foo.bar" << ":/prefix/foo.bar";
QTest::newRow("abs-res-res") << ":/prefix" << ":/abc.txt" << ":/abc.txt";
QTest::newRow("abs-res-path") << DRIVE "/etc" << ":/abc.txt" << ":/abc.txt";
}
void tst_QDir::absoluteFilePath()
@ -1517,12 +1523,17 @@ void tst_QDir::filePath_data()
QTest::addColumn<QString>("fileName");
QTest::addColumn<QString>("expectedFilePath");
QTest::newRow("0") << "/etc" << "/passwd" << "/passwd";
QTest::newRow("1") << "/etc" << "passwd" << "/etc/passwd";
QTest::newRow("2") << "/" << "passwd" << "/passwd";
QTest::newRow("3") << "relative" << "path" << "relative/path";
QTest::newRow("4") << "" << "" << ".";
QTest::newRow("abs-abs") << DRIVE "/etc" << DRIVE "/passwd" << DRIVE "/passwd";
QTest::newRow("abs-rel") << DRIVE "/etc" << "passwd" << DRIVE "/etc/passwd";
QTest::newRow("root-rel") << DRIVE "/" << "passwd" << DRIVE "/passwd";
QTest::newRow("rel-rel") << "relative" << "path" << "relative/path";
QTest::newRow("empty-empty") << "" << "" << ".";
QTest::newRow("resource") << ":/prefix" << "foo.bar" << ":/prefix/foo.bar";
#ifdef Q_OS_WIN
QTest::newRow("abs-LTUNC") << "Q:/path" << "\\/leaning\\tooth/pick" << "\\/leaning\\tooth/pick";
QTest::newRow("LTUNC-slash") << "\\/leaning\\tooth/pick" << "/path" << "//leaning/tooth/path";
QTest::newRow("LTUNC-abs") << "\\/leaning\\tooth/pick" << "Q:/path" << "Q:/path";
#endif
}
void tst_QDir::filePath()
@ -1588,6 +1599,9 @@ void tst_QDir::exists2_data()
QTest::newRow("2") << "" << false;
QTest::newRow("3") << "testData" << true;
QTest::newRow("4") << "/testData" << false;
#ifdef Q_OS_WIN
QTest::newRow("abs") << "Q:/testData" << false;
#endif
QTest::newRow("5") << "tst_qdir.cpp" << true;
QTest::newRow("6") << "/resources.cpp" << false;
QTest::newRow("resource0") << ":/prefix/foo.bar" << false;