Shorten sockets file path
ClosedPublic

Authored by fazevedo on Jun 7 2019, 2:45 PM.

Details

Summary

Use Qt dedicated 'runtime' standard location for sockets.
This fix an issue on macos where temporary directories are so long
that a user having a long nick would bypass the socket file path length
of 104chars.

Diff Detail

Repository
R165 Akonadi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
fazevedo created this revision.Jun 7 2019, 2:45 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptJun 7 2019, 2:45 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
fazevedo requested review of this revision.Jun 7 2019, 2:45 PM
dfaure requested changes to this revision.Jun 8 2019, 8:46 AM

Thanks!

src/server/utils.cpp
109–110

This line is no longer necessary.

This revision now requires changes to proceed.Jun 8 2019, 8:46 AM
dvratil requested changes to this revision.Jun 8 2019, 11:46 AM

Good idea, this generally opens a new range of possibilities to simplify the code a bit, since the socket will now exist in user-specific directory (unlike until now it existed in /tmp on Linux, which could possibly clash with other users if it were called only "akonadi", hence the username and random string) so the name can be deterministic without random strings.

src/server/utils.cpp
108–109

Since on Linux you are now creating the directory in /var/run/user/<uid>/, I'd very much prefer to keep akonadi in the name...

Actually, since you cannot have two same instances of Akonadi running at the same time, it should be perfectly fine to call the folder akonadi for the default instance and akonadi-<instancename> when Akonadi::Instance::hasIdentifier() is true, no more random strings.

This would make the directory deterministic for each instance, simplifying the code.

150–151

Once you change to the deterministic format, mkdtemp can be switched to QDir::mkpath()

fazevedo marked 3 inline comments as done.Jun 11 2019, 7:51 AM
fazevedo added inline comments.
src/server/utils.cpp
108–109

createSocketDirectory is calling saveDir("runtime"), which already append "akonadi" tho the runtime path, ie "/path/to/runtime/akonadi", which result to path like "path/tp/runtime/akonadi/TMPL".

I could then just use fixed (no) template name like 'default' and 'identifier Name' in this folder then ?

fazevedo marked an inline comment as done.Jun 11 2019, 7:54 AM
fazevedo updated this revision to Diff 59579.Jun 11 2019, 7:55 AM

Removed the tmp name approach

fazevedo updated this revision to Diff 59584.Jun 11 2019, 8:34 AM

Also check the socket file path length now

dfaure requested changes to this revision.Jun 11 2019, 8:40 AM
dfaure added inline comments.
src/server/utils.cpp
72

should be >=, you need room for the trailing NUL character as well.

74

Let's hope that one does fit in the length limit :-)

110

Don't use QDir::separator(). That's '\' on Windows, while all this code uses '/' everywhere.

Just make it "/socket..." (the existing code is the result of automated conversion)

149

%1/%2, no need for QDir::separator, this uses Qt APIs, not native APIs.

This revision now requires changes to proceed.Jun 11 2019, 8:40 AM
fazevedo marked 4 inline comments as done.Jun 11 2019, 9:47 AM
fazevedo updated this revision to Diff 59587.Jun 11 2019, 9:47 AM

Better length checking.

dfaure accepted this revision.Jun 11 2019, 10:57 AM

Looks good to me, wait for Dan's approval too.

dvratil accepted this revision.Jun 12 2019, 8:36 AM

Thanks!

This revision is now accepted and ready to land.Jun 12 2019, 8:36 AM
This revision was automatically updated to reflect the committed changes.