NodeHelper::fromAsString: make it work with extra content nodes
ClosedPublic

Authored by dvratil on Nov 16 2016, 2:33 PM.

Details

Summary

Currently NodeHelper::fromAsString() can only return the sender only if the passed KMime::Content is directly part of the original KMime::Message node tree. This patch makes NodeHelper::fromAsString() non-static, so that it can also perform lookups for extra KMime::Content parts.

Typical usecase is to look up sender from a signed part encapsulated inside an originally encrypted KMime::Content. The signed part is not part of the original KMime::Message node tree, thus NodeHelper::fromAsString() would return an empty string. With this patch we can lookup the original KMime::Content that this part is attached to (in this case it would be the original encrypted part) for which we can perform the sender lookup.

Test Plan

Added a unit-test

Diff Detail

Repository
R94 PIM: Message Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dvratil updated this revision to Diff 8223.Nov 16 2016, 2:33 PM
dvratil retitled this revision from to NodeHelper::fromAsString: make it work with extra content nodes.
dvratil updated this object.
dvratil edited the test plan for this revision. (Show Details)
dvratil added a reviewer: knauss.
dvratil set the repository for this revision to R94 PIM: Message Library.
dvratil added a subscriber: KDE PIM.
Restricted Application added a project: KDE PIM. · View Herald TranscriptNov 16 2016, 2:33 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
dvratil updated this revision to Diff 8226.Nov 16 2016, 2:34 PM
knauss added inline comments.Nov 16 2016, 5:45 PM
mimetreeparser/autotests/nodehelpertest.cpp
241

add helper.fromAsString(node, sender) to be complete.

244

I also would like to have a more complex test case aka:

toplevel (toplvel@)
-> sub -> toplevel@
->an attached rfc822 mail with own sender -> second@
   -> subsub -> second@

and for all 4 parts make sure that th correct from address is returned and for all extra parts, that must return the correct value.

dvratil marked an inline comment as done.Nov 18 2016, 10:51 AM
dvratil added inline comments.
mimetreeparser/autotests/nodehelpertest.cpp
244

That actually would not be compatible with the current behavior and is beyond the scope of this change (IMO). NodeHelper::fromAsString() returns sender of the top-level message, which always is a KMime::Content with a null parent. Even for an encapsulated message this function always returns a sender of the top-level message.

I can add X_FAIL test cases so we know this should be fixed/solved, but the correct detection of nested sub-message may be non-trivial and might break some other code that might rely on the current behaviour.

dvratil updated this revision to Diff 8300.Nov 18 2016, 10:54 AM

Add XFAIL testcases for encapsulated messages.

knauss accepted this revision.Nov 18 2016, 8:18 PM
knauss edited edge metadata.

I thouth this was acutailly implemented.

Yes to fix this it is out-of scope for this patch.

But this methods is only used for signed parts, I it would not change behaviour in a non desired way. We only have mimetreeparser -> messageviewer -> defaultrenderer, as only occurence for this method inside kdepim.

This revision is now accepted and ready to land.Nov 18 2016, 8:18 PM
This revision was automatically updated to reflect the committed changes.