[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: Serious flaw in ActionScheduler::moveMessage()
From:       Andreas Gungl <a.gungl () gmx ! de>
Date:       2005-12-06 20:34:34
Message-ID: 200512062134.41333 () gungl-dd ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Am Dienstag, 6. Dezember 2005 19:38 schrieb Ingo Klöcker:
> On Tuesday 06 December 2005 12:05, Andreas Gungl wrote:
> > Hi, when working on http://bugs.kde.org/show_bug.cgi?id=113730 I've
> > found some concerns with the following code:
> >
> > void ActionScheduler::moveMessage()
> > ...
> >   if (!orgMsg || !orgMsg->parent()) {
> >     // Original message is gone, no point filtering it anymore
> >     mSrcFolder->removeMsg( mSrcFolder->find( msg ) );
> >     mExecutingLock = false;
> >     processMessageTimer->start( 0, true );
> >   } else {
> >     if (!folder) // no filter folder specified leave in current place
> >       folder = orgMsg->parent();
> >   }
> >
> > Given that we cannot identify the original message (e.g. due to a
> > lost X-KMail-Filtered header after spamassassin having wrapped the
> > spam message, see the report for details) we try to remove the
> > message from the source folder.
> > Even if I didn't manage to really delete such a message (they stayed
> > in the folder), some lines later we get an invalid index which leads
> > to a crash:
> >
> >   mSrcFolder->take( mSrcFolder->find( msg ) );
> >   mSrcFolder->addMsg( msg );
>
> This code removes the old version of the message from the folder and
> adds the new (filtered) version.

In the meantime, I've found out the code is refreshing the message in the 
filter folder (.kde/share/apps/kmail/filter/*).

> > Further, during my tests I managed to get a (folder == 0) case which
> > crashes in:
> >
> >   if (msg && kmkernel->folderIsTrash( folder ))
> >     KMFilterAction::sendMDN( msg, KMime::MDN::Deleted );
> >
> > Okay, that one was easy to fix actually.
> > But what is the first code block intended to do? I feel like missing
> > at least a return statement before the line containing the else.
>
> Which "first" code block do you mean? The code in the if-branch? I agree
> that there seems to be missing a return.
>
> > It looks like many people suffer from such crashes even if the
> > actionscheduler should normally be disabled for non-IMAP accounts. So
> > it would be nice, if somebody could invest some time to at least let
> > me know what the code should do.
>
> The code in the if-branch stops processing the current message and
> starts a single-shot timer for processing the next message. But I don't
> see what the mSrcFolder->removeMsg() is supposed to do.

Okay, so we have at least the same opinion. I think, the  remove is there to 
take away messages which have been changed during the filter process, but 
that doesn't work with messages piped through an external filter.

Another observation is (actually was) the pipe action leaves the message in 
the filter folder untouched. Damn, that gave me headaches because I had spam 
messages already filtered by sa on the server. They had a score, but the 
local sa calculated another score which never occurred in the filter log.

Anyway, the async filtering looks pretty unfinished or at least untested to 
me. I've put this whole day into investigating the ideas behind the code. And 
I'm a bit upset to have found out only a few things which can be considered a 
step toward a solution.

Andreas

[Attachment #5 (application/pgp-signature)]

_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel


[prev in list] [next in list] [prev in thread] [next in thread]