D25414: xattr: fix crash on dangling symlinks
This revision was automatically updated to reflect the committed changes. Closed by commit R286:4bb4195a6fc6: xattr: fix crash on dangling symlinks (authored by iasensio). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25414?vs=70035=71397 REVISION DETAIL https://phabricator.kde.org/D25414 AFFECTED FILES autotests/usermetadatawritertest.cpp autotests/usermetadatawritertest.h src/xattr_p.h To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH fix_symlink REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio added a comment. Ping? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio added inline comments. INLINE COMMENTS > bruns wrote in usermetadatawritertest.cpp:44 > You can just use the static overload and provide an invalid target name. > https://doc.qt.io/qt-5/qfile.html#link-1 You are right. I tried that with an empty string and it didn't work, but it does with a non-existent filename. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio updated this revision to Diff 70035. iasensio marked an inline comment as done. iasensio added a comment. - Simplify symlink creation REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25414?vs=70031=70035 BRANCH fix_symlink REVISION DETAIL https://phabricator.kde.org/D25414 AFFECTED FILES autotests/usermetadatawritertest.cpp autotests/usermetadatawritertest.h src/xattr_p.h To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
bruns added inline comments.
INLINE COMMENTS
> usermetadatawritertest.cpp:44
> +QFile tempTarget(testFilePath("temporal_target"));
> +tempTarget.link(testFilePath(TEST_SYMLINK));
> +tempTarget.remove();
You can just use the static overload and provide an invalid target name.
https://doc.qt.io/qt-5/qfile.html#link-1
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D25414
To: iasensio, astippich, bruns
Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n,
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio edited the test plan for this revision. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio updated this revision to Diff 70031. iasensio added a comment. - Use temporal symlink REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25414?vs=70029=70031 BRANCH fix_symlink REVISION DETAIL https://phabricator.kde.org/D25414 AFFECTED FILES autotests/usermetadatawritertest.cpp autotests/usermetadatawritertest.h src/xattr_p.h To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio updated this revision to Diff 70029. iasensio marked 2 inline comments as done. iasensio added a comment. - Protect from size < 0 - Add test case for dangling symlink REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25414?vs=70019=70029 BRANCH fix_symlink REVISION DETAIL https://phabricator.kde.org/D25414 AFFECTED FILES autotests/samplefiles/dangling_symlink autotests/usermetadatawritertest.cpp autotests/usermetadatawritertest.h src/xattr_p.h To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
bruns added a comment. Also add a testcase covering this, autotests/usermetadatawritertest.cpp REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio edited the summary of this revision. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
bruns added a comment. wrap the summary at <80 characters REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
bruns requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich, bruns Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
bruns added inline comments.
INLINE COMMENTS
> xattr_p.h:228
>
> +if (size == -1 && errno == ENOENT) {
> +return UserMetaData::Attribute::None;
if (size < 0) {
if (errno == E2BIG) {
return UserMetaData::Attribute::All;
}
return UserMetaData::Attribute::None;
}
> xattr_p.h:240
>
> if (size > 0 && attributes == UserMetaData::Attribute::Any) {
> return UserMetaData::Attribute::All;
The `size > 0` can be removed here then.
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D25414
To: iasensio, astippich
Cc: bruns, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n,
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio edited the summary of this revision. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D25414 To: iasensio, astippich Cc: kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D25414: xattr: fix crash on dangling symlinks
iasensio created this revision. iasensio added a reviewer: astippich. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. iasensio requested review of this revision. REVISION SUMMARY When requesting metadata on a dangling symlink, the framestack ends up calling `k_queryAttributes` with the symlink path. There, the `listxattr` syscall returns `size=-1` and `errno=ENOENT` (2 No such file or directory), which was not covered before, and provoking a segfault on `QByteArray`. Full traceback on: https://bugs.kde.org/show_bug.cgi?id=414227 It might be also a good idea to protect the function in any other cases where `size=-1` BUG: 414227 TEST PLAN On dolphin, with panel information open, hover over a dangling symlink REPOSITORY R286 KFileMetaData BRANCH fix_symlink REVISION DETAIL https://phabricator.kde.org/D25414 AFFECTED FILES src/xattr_p.h To: iasensio, astippich Cc: kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
