Skip to content

Commit

Permalink
Fix bugs and add support for UNC paths on Windows.
Browse files Browse the repository at this point in the history 
- Added code to detect network share paths on Windows and
  treat make them behave like drives.

- Fixed a bug where paths typed in with lower case drive
  letters would cause an upper case drive AND a lower
  case drive entry to appear in the "My Computer" view.
  A similar thing would happen for network share hostnames.
  Added logic to upper case the drive letter and hostnames
  as part of the "path cleanup" logic. This causes the
  paths to map to single entries in the "My Computer" view.

- Fixed an infinite loop during directory restore when
  a non-existant network share was specified in a previous
  Open File dialog instance.

- Fixed "up" button so that it behaves more consistently
  and properly handles network share paths.

- Fixed invalid memory access for files that start with a '.'

- Fixed a few places where code was using a URL path instead
  of converting the URL to a local path string like the
  vast majority of the code. This made it possible to properly
  view the root directory of network shares.

- Updated unit tests to reflect changes in path handling
  behaviour.
  • Loading branch information
@acolwell
acolwell committed Sep 11, 2023
1 parent 3b4d607 commit cc07dc6
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 80 deletions.
175 changes: 139 additions & 36 deletions Engine/FileSystemModel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,43 +61,118 @@ CLANG_DIAG_ON(uninitialized)

NATRON_NAMESPACE_ENTER

static int
getDriveNameSize(const QString& name)
{
#ifdef __NATRON_WIN32__
if (name.size() < 3) {
return 0;
}

if (name.at(0).isLetter() && name.at(1) == QLatin1Char(':') && ( name.at(2) == QLatin1Char('/') || name.at(2) == QLatin1Char('\\') ) ) {
// Drive path. (e.g. C:/ or C:\)
return 3;
}

if (((name.at(0) == QLatin1Char('/') && name.at(1) == QLatin1Char('/')) ||
(name.at(0) == QLatin1Char('\\') && name.at(1) == QLatin1Char('\\'))) && name.at(2).isLetterOrNumber()) {
// Possible Network share. (e.g. //SomeHost/ShareName)

// Search for end of hostname.
int idx = name.indexOf(QLatin1Char('/'), 2);
if (idx == -1) {
idx = name.indexOf(QLatin1Char('\\'), 2);
}

if (idx == -1) {
// Hostname only without a trailing slash. (e.g. //SomeHost).
// Do not consider this valid, just like "C:" is not considered a valid drive.
return 0;
}

++idx; // Move beyond slash.
return idx;
}
#else
if (name.startsWith(QDir::rootPath())) {
return QDir::rootPath().size();
}
#endif

return 0;
}

#ifdef __NATRON_WIN32__
// Ensures that drive names and network share hostnames are always upper case.
static QString
ensureCanonicalDriveAndShareNames(const QString& path)
{
if (path.isEmpty() || !FileSystemModel::startsWithDriveName(path)) {
return path;
}

if (path.size() < 3) {
return path;
}

// Assume that any path that reaches this point has already had backslashes converted
// to forward slashes.
assert(path.indexOf(QLatin1Char('\\')) == -1);

QString ret = path;
if (path[0] == QLatin1Char('/') && path[1] == QLatin1Char('/') && path[2].isLetterOrNumber()) {
// Network share name.

int idx = path.indexOf(QLatin1Char('/'), 2);
ret.truncate(2); // keep starting slashes.
if (idx == -1) {
// No trailing slash. Hostname only case.
// Change the hostname to upper case and add the trailing slash.
ret.append(path.mid(2).toUpper());
ret.append(QLatin1Char('/')); // Add a trailing slash.
} else {
// Hostname with slash and possible path components.
const QString hostname = path.mid(2, idx - 2);

// Change hostname to upper case and append any path components that may remain.
ret.append(hostname.toUpper());
ret.append(path.mid(idx));
}
} else if (path[0].isLetter() && path[1] == QLatin1Char(':') && path[2] == QLatin1Char('/')) {
// Drive name.
// Force drive letter to be upper case.
ret[0] = ret[0].toUpper();
}

return ret;
}
#endif // __NATRON_WIN32__

QStringList
FileSystemModel::getSplitPath(const QString& path)
{
if ( path.isEmpty() ) {
return QStringList();
}
QString pathCpy = path;
bool isdriveOrRoot;
#ifdef __NATRON_WIN32__
QString startPath = pathCpy.mid(0, 3);
isdriveOrRoot = FileSystemModel::isDriveName(startPath);
if (isdriveOrRoot) {
pathCpy = pathCpy.remove(0, 3);
}
if ( (pathCpy.size() > 0) && ( pathCpy[pathCpy.size() - 1] == QChar::fromLatin1('/') ) ) {
pathCpy = pathCpy.mid(0, pathCpy.size() - 1);
}
QStringList splitPath = pathCpy.split( QChar::fromLatin1('/') );
if (isdriveOrRoot) {
splitPath.prepend( startPath.mid(0, 3) );
}

#else
isdriveOrRoot = pathCpy.startsWith( QChar::fromLatin1('/') );
if (isdriveOrRoot) {
pathCpy = pathCpy.remove(0, 1);
int driveNameSize = getDriveNameSize(path);
if (driveNameSize > 0) {
// Remove the drive name from pathCpy.
pathCpy = pathCpy.remove(0, driveNameSize);
}

// Remove trailing slash.
if ( (pathCpy.size() > 0) && ( pathCpy[pathCpy.size() - 1] == QChar::fromLatin1('/') ) ) {
pathCpy = pathCpy.mid(0, pathCpy.size() - 1);
pathCpy = pathCpy.mid(0, pathCpy.size() - 1);
}

QStringList splitPath = pathCpy.split( QChar::fromLatin1('/') );
if (isdriveOrRoot) {
splitPath.prepend( QChar::fromLatin1('/') );
if (driveNameSize > 0) {
// Put the drive name at the beginning of the list.
splitPath.prepend(path.mid(0, driveNameSize));
}

#endif

return splitPath;
}

Expand Down Expand Up @@ -613,29 +688,57 @@ FileSystemModel::getSharedItemPtr(FileSystemItem* item) const
bool
FileSystemModel::isDriveName(const QString& name)
{
#ifdef __NATRON_WIN32__

return name.size() == 3 && name.at(0).isLetter() && name.at(1) == QLatin1Char(':') && ( name.at(2) == QLatin1Char('/') || name.at(2) == QLatin1Char('\\') );
#else

return name == QDir::rootPath();
#endif
return !name.isEmpty() && getDriveNameSize(name) == name.size();
}

bool
FileSystemModel::startsWithDriveName(const QString& name)
{
return getDriveNameSize(name) > 0;
}

QString FileSystemModel::cleanPath(const QString& path) {
if (path.isEmpty()) {
return path;
}

QString newPath = path;

#ifdef __NATRON_WIN32__
// Replace backslashes with slashes.
newPath.replace( QLatin1Char('\\'), QLatin1Char('/') );
#endif

return name.size() >= 3 && name.at(0).isLetter() && name.at(1) == QLatin1Char(':') && ( name.at(2) == QLatin1Char('/') || name.at(2) == QLatin1Char('\\') );
#else
bool startedWithDriveName = startsWithDriveName(newPath);

return name.startsWith( QDir::rootPath() );
// Resolve and remove '.' and "..".
newPath = QDir::cleanPath(newPath);

if (newPath.isEmpty()) {
return newPath;
}

// QDir::cleanPath() strips the trailing slash for network shares
// (e.g. //SomeHost/) , but not for drives (e.g. C:/). The trailing
// slash is required for network shares to behave the same way as
// drives elsewhere in the code so we need to put back the trailing
// slash.
if (startedWithDriveName && !startsWithDriveName(newPath)) {
// The path no longer starts with a drive name after cleaning.
// See if appending a trailing slash fixes it.
const QString newPathWithTrailingSlash = newPath + QLatin1Char('/');
if (startsWithDriveName(newPathWithTrailingSlash)) {
newPath = newPathWithTrailingSlash;
}
}

#ifdef __NATRON_WIN32__
// Make sure drive letters and share hostnames are upper case so they get properly
// collapsed together in the UI and other path matching.
newPath = ensureCanonicalDriveAndShareNames(newPath);
#endif
}

QString FileSystemModel::cleanPath(const QString& path) {
return QDir::cleanPath(path);
return newPath;
}

QVariant
Expand Down
69 changes: 42 additions & 27 deletions Gui/SequenceFileDialog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,10 +540,12 @@ SequenceFileDialog::SequenceFileDialog( QWidget* parent, // necessary to transmi
}

if (!hasRestoredDir) {
// try to go up until we find a directory that exists.
// try to go up until we find a directory that exists or we have reached a drive name.
// Stopping at the drive name is important because cleanPath() does not guarantee that
// applying a "/.." to all drive names will return an empty string. (e.g. network shares //somehost/)
QString dir = QString::fromUtf8( directoryArgs.c_str() );
//qDebug() << "dir=" << dir;
while (!dir.isEmpty() && !hasRestoredDir) {
while (!dir.isEmpty() && !FileSystemModel::isDriveName(dir) && !hasRestoredDir) {
dir = FileSystemModel::cleanPath(dir + QString::fromUtf8("/.."));
if ( isDirectory(dir) ) {
hasRestoredDir = setDirectory(dir);
Expand Down Expand Up @@ -737,7 +739,10 @@ SequenceFileDialog::setFileExtensionOnLineEdit(const QString & ext)
return;
} else {
int pos = str.lastIndexOf( QLatin1Char('.') );
if (pos != -1) {
if (pos == 0) {
// Do not modify filenames that start with a period.
return;
} else if (pos != -1) {
if ( str.at(pos - 1) == QLatin1Char('#') ) {
--pos;
}
Expand Down Expand Up @@ -1079,7 +1084,7 @@ SequenceDialogView::dropEvent(QDropEvent* e)
QList<QUrl> urls = e->mimeData()->urls();
QString path;
if (urls.size() > 0) {
path = QtCompat::toLocalFileUrlFixed( urls.at(0) ).path();
path = urlToPathString( urls.at(0) );
}
if ( !path.isEmpty() ) {
_fd->setDirectory(path);
Expand Down Expand Up @@ -1282,26 +1287,19 @@ SequenceFileDialog::parentFolder()
QDir dir(rootPath);
dir.cdUp();
newDir = dir.absolutePath();
}


#ifdef __NATRON_WIN32__
if ( FileSystemModel::isDriveName(newDir) ) {
_upButton->setEnabled(false);
} else {
_upButton->setEnabled(true);
}
#else
if ( FileSystemModel::isDriveName(newDir) || newDir.isEmpty() ) {
_upButton->setEnabled(false);
if (!FileSystemModel::startsWithDriveName(newDir)) {
// QDir always provides a trailing slash to drives, but
// leaves it off for network shares. If the new
// directory does not start with a drive name anymore
// assume it just needs a trailing slash appended.
StrUtils::ensureLastPathSeparator(newDir);
}

return;
} else {
_upButton->setEnabled(true);
assert(FileSystemModel::startsWithDriveName(newDir));
}

#endif

updateUpButton(newDir);

setDirectory(newDir);
}
Expand Down Expand Up @@ -1617,11 +1615,7 @@ SequenceFileDialog::getEnvironmentVariable(const QString &string)
void
SequenceFileDialog::pathChanged(const QString &newPath)
{
if ( newPath.isEmpty() ) {
_upButton->setEnabled(false);
} else {
_upButton->setEnabled(true);
}
updateUpButton(newPath);

_favoriteView->selectUrl( QUrl::fromLocalFile(newPath) );
setHistory( _lookInCombobox->history() );
Expand All @@ -1638,6 +1632,27 @@ SequenceFileDialog::pathChanged(const QString &newPath)
_previousButton->setEnabled(_currentHistoryLocation > 0);
}

void
SequenceFileDialog::updateUpButton(const QString &newPath) {
if (newPath.isEmpty()) {
_upButton->setEnabled(false);
return;
}

#ifndef __NATRON_WIN32__
// On non-Windows platforms the up button should be disabled for the rootPath (i.e. "/").
// On Windows we don't want to stop here because the rootPath is typically "C:/" and
// we want to be able to "go up one more" so that the drives and network shares can
// be seen.
if (newPath == QDir::rootPath()) {
_upButton->setEnabled(false);
return;
}
#endif

_upButton->setEnabled(true);
}

void
SequenceFileDialog::setHistory(const QStringList &paths)
{
Expand Down Expand Up @@ -1910,7 +1925,7 @@ UrlModel::setUrl(const QModelIndex &index,
const QModelIndex &dirIndex)
{
setData(index, url, UrlRole);
if ( url.path().isEmpty() ) {
if ( urlToPathString(url).isEmpty() ) {
const char* myComputer = "Computer";
setData(index, tr(myComputer));
setData(index, tr(myComputer), Qt::DecorationRole);
Expand Down Expand Up @@ -2130,7 +2145,7 @@ UrlModel::changed(const QString &path)
void
UrlModel::removeRowIndex(const QModelIndex& index)
{
QString urlPath = index.data(UrlRole).toUrl().path();
QString urlPath = urlToPathString(index.data(UrlRole).toUrl());

if ( !urlPath.isEmpty() ) {
removeRow( index.row() );
Expand Down
1 change: 1 addition & 0 deletions Gui/SequenceFileDialog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ public Q_SLOTS:
void getSequenceFromFilesForFole(const QString & file, SequenceParsing::SequenceFromFiles* sequence) const;

void updateFileExtensionCombo(const QString& extension);
void updateUpButton(const QString &newPath);

private:
// FIXME: PIMPL
Expand Down
Loading

0 comments on commit cc07dc6

Please sign in to comment.