D24489: KAutosaveFile not respecting maximum filename length

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Why is this not in the same commit as the related unittest, as is common practice?

INLINE COMMENTS

> kautosavefile.cpp:70
>      const QString protocol(managedFile.scheme());
> -    const QString path(managedFile.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash).path());
> -    QString name(managedFile.fileName());
> +    const QString path(QString::fromLatin1(QUrl::toPercentEncoding(managedFile.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash).path()).constData()));
> +    QString name(QString::fromLatin1(QUrl::toPercentEncoding(managedFile.fileName()).constData()));

That's one long line, hard to read. Extract a `const QByteArray encodedPath = QUrl::toPercentEncoding(....)` ?

[pre-existing: I would personally call this `QByteArray encodedDirectory` and `QString directory`, to my eyes path is a full path including filename. At least it's ambiguous, unlike directory]

Also check if you can remove the .constData(). `QString::fromLatin1(const QByteArray &str)` was added in Qt 5.0, this code is probably older than that.

> kautosavefile.cpp:71
> +    const QString path(QString::fromLatin1(QUrl::toPercentEncoding(managedFile.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash).path()).constData()));
> +    QString name(QString::fromLatin1(QUrl::toPercentEncoding(managedFile.fileName()).constData()));
>  

and a `const QByteArray encodedFileName = ...`, for symmetry.

Simpler than toPercentEncoding: use `QUrl::fileName(QUrl::FullyEncoded)`.
However this wouldn't work for `encodedDirectory` above, because it wouldn't encode '/'.

> kautosavefile.cpp:76
>      // Subtract 1 for the _ char, 3 for the padding separator, 5 is for the .lock
> -    int pathLengthLimit = FILENAME_MAX - NamePadding - name.size() - protocol.size() - 9;
> +    int pathLengthLimit = NAME_MAX - NamePadding - name.size() - protocol.size() - 9;
>  

(while changing this line: prepend `const`)

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/20191205/4b888e2c/attachment-0001.html>