D24489: KAutosaveFile not respecting maximum filename length
Jean-Baptiste Mardelle
noreply at phabricator.kde.org
Fri Dec 6 12:37:54 GMT 2019
mardelle added a comment.
The unittest is in another diff because it's a different author I guess. I can process the comments but looking closer I found 2 other important issues in this class:
1. When using KAutoSaveFile::staleFiles(filename) to check if there is an existing stale file for a document, we use the function KAutoSaveFile::extractManagedFilePath
This function compares the orginal document path one reconstructed from the lock file name. However this logic seems flawed since the lock file name uses a trimmed path in some cases it is impossible to rebuild the original document's path from it.
Instead I would propose to replace extractManagedFilePath with a new function, comparing filename, then paths like:
bool staleMatchesManaged(const QString& staleFileName, const QString managedFile)
{
const QStringRef sep = staleFileName.rightRef(3);
int sepPos = staleFileName.indexOf(sep);
if (QFileInfo(managedFile).fileName() != staleFileName.leftRef(sepPos).toString()) {
// File names don't match
return false;
}
int pathPos = staleFileName.indexOf(QChar::fromLatin1('_'), sepPos);
const QByteArray encodedPath = staleFileName.midRef(pathPos+1, staleFileName.length()-pathPos-1-KAutoSaveFilePrivate::NamePadding).toLatin1();
const QString sourcePathStart = QUrl::fromPercentEncoding(encodedPath);
return QFileInfo(managedFile).absolutePath().startsWith(sourcePathStart);
}
2. The QLockFile class used in KAutoSaveFile creates a temporary file to manage locking:
> QLockFile rmlock(d->fileName + QLatin1String(".rmlock"));
https://github.com/qt/qtbase/blob/dev/src/corelib/io/qlockfile.cpp#L259
This means that in our situation where we have a filename that already has the maximum length, trying to lock will always fail, preventing usage of the autosave file.
Easier solution is probably to make maximum filename in KAutoSave File::tempFileName() a bit shorter.
Thoughts? Should I prepare separate patches ? But changing the fileName encoding as suggested:
> Simpler than toPercentEncoding: use QUrl::fileName(QUrl::FullyEncoded).
Might break more the path recovery method extractManagedFilePath..
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D24489
To: mardelle, #frameworks, dfaure, mpyne
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191206/bf33b69e/attachment.html>
More information about the Kde-frameworks-devel
mailing list