KAutoSaveFile: add a unit test to check max. filename length
ClosedPublic

Authored by ahmadsamir on Dec 5 2019, 4:59 PM.

Details

Summary

In KAutoSaveFilePrivate::tempFile() the name of the kautosavefile that's
going to be created is generated using the filename and the absolute path
and some other bits (e.g. a string generated using KRandom). The problem
with that approach is that sometimes really long filenames can get created.
This unit test ensures that the checks in tempFile() work correctly and
generate filenames that don't exceed NAME_MAX (255 bytes/chars for Linux),
otherwise the files can't be opened.

Depends on D24489

References:
/usr/include/linux/limits.h
https://www.gnu.org/software/libc/manual/html_node/Limits-for-Files.html
https://www.kernel.org/doc/Documentation/filesystems/path-lookup.rst

Test Plan
  • Running ctest -R autosavefiletest fails with current master
  • Apply the patch from D24489, and the test should pass

Diff Detail

Repository
R244 KCoreAddons
Branch
l-kautosave-unittest (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19522
Build 19540: arc lint + arc unit
ahmadsamir created this revision.Dec 5 2019, 4:59 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 5 2019, 4:59 PM
ahmadsamir requested review of this revision.Dec 5 2019, 4:59 PM
dfaure accepted this revision.Dec 5 2019, 10:49 PM
dfaure added inline comments.
autotests/kautosavefiletest.cpp
84

typo: concatenated

95

fill modifies the string, no need for s =

98

Let's hope none of the supported OSes have a smaller PATH_MAX :)
FreeBSD, Windows, macOS, who knows what limits they have.

108

merge with previous line using the QFile(QString) constructor

110

Doesn't belong in unittests

112

Why the QFileInfo::absoluteFilePath call? It's already an absolute path.

This revision is now accepted and ready to land.Dec 5 2019, 10:49 PM
dhaumann added inline comments.
autotests/kautosavefiletest.cpp
98

What I wonder is: PATH_MAX etc are #defines, right? Since on Windows there is a registry flag to enable much longer paths/file names. My intention is not to block this series of patches, but we might end up with restrictions in our code that do not exist on the filesystem at hand.

ahmadsamir updated this revision to Diff 71047.Dec 7 2019, 8:26 AM

Address comments

ahmadsamir added inline comments.Dec 7 2019, 8:35 AM
autotests/kautosavefiletest.cpp
98

On windows the extended-length path is 32,767[1] characters, which is a bit unreasonable. I only skimmed those docs though.

[1] https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file

FTR, according to lxr.kde.org KAutoSaveFile is used by kdenlive, lokalize and libkeduvocdocument.

This revision was automatically updated to reflect the committed changes.