commit 3c748fde9f70eeedaf7a1c1b2f22dc3552769c41 Author: Patrick McManus Date: Wed Jan 16 16:47:31 2008 -0500 for http://bugs.kde.org/show_bug.cgi?id=127696 The issue here is error handling when receiving a blank uid prortion of a UIDL response.. for instance: 1 1200413730.1[CRLF] 2 [CRLF] 3 1200414111.3[CRLF] . [CRLF] will trigger it. John's debug data from the original bug description shows this nicely. it seems some pop server will generate a bogus uidl response like the above based on the email containing an invalid message-id header. I only tested with Dovecot, and could not reproduce that behavior, but forcing the protocol trace with netcat using the above sequence did create a reproduction. If any of the reporters could identify the pop server that is doing that, we could potentially file a bug against the server which is really the root cause here too. In any event, without a valid UIDL identifier the message cannot be persisted on the server - otherwise, everytime a LIST operation occurred there would be no way to know whether or not there was a local copy of this message and we would keep downloading it over and over again. The attached patch * separates error handling for LIST and UIDL operations (removing the confusing error dialog here that casts doubt on LIST un-necessarily) * takes UIDL lines with an ID but without a valid UID portion and forces a download of those messages and a delete from the server * logs a debug message when it is doing this diff --git a/kmail/popaccount.cpp b/kmail/popaccount.cpp index 2f0b26c..689113b 100644 --- a/kmail/popaccount.cpp +++ b/kmail/popaccount.cpp @@ -389,6 +389,8 @@ void PopAccount::startJob() idsOfMsgs.clear(); mUidForIdMap.clear(); idsOfMsgsToDelete.clear(); + idsOfForcedDeletes.clear(); + //delete any headers if there are some this have to be done because of check again headersOnServer.clear(); headers = false; @@ -735,6 +737,14 @@ void PopAccount::slotJobFinished() { idsOfMsgsToDelete.remove( it.key().second ); } } + + if (!idsOfForcedDeletes.isEmpty()) + { + idsOfMsgsToDelete += idsOfForcedDeletes; + idsOfForcedDeletes.clear(); + } + + // If there are messages to delete then delete them if ( !idsOfMsgsToDelete.isEmpty() ) { stage = Dele; @@ -923,8 +933,9 @@ void PopAccount::slotData( KIO::Job* job, const QByteArray &data) QString qdata = data; qdata = qdata.simplifyWhiteSpace(); // Workaround for Maillennium POP3/UNIBOX int spc = qdata.find( ' ' ); - if (spc > 0) { - if (stage == List) { + + if (stage == List) { + if (spc > 0) { QString length = qdata.mid(spc+1); if (length.find(' ') != -1) length.truncate(length.find(' ')); int len = length.toInt(); @@ -933,46 +944,85 @@ void PopAccount::slotData( KIO::Job* job, const QByteArray &data) idsOfMsgs.append( id ); mMsgsPendingDownload.insert( id, len ); } - else { // stage == Uidl - const QString id = qdata.left(spc); - const QString uid = qdata.mid(spc + 1); - int *size = new int; //malloc(size_of(int)); - *size = mMsgsPendingDownload[id]; - mSizeOfNextSeenMsgsDict.insert( uid, size ); - if ( mUidsOfSeenMsgsDict.find( uid ) != 0 ) { - - if ( mMsgsPendingDownload.contains( id ) ) { - mMsgsPendingDownload.remove( id ); - } - else - kdDebug(5006) << "PopAccount::slotData synchronization failure." << endl; - idsOfMsgsToDelete.append( id ); - mUidsOfNextSeenMsgsDict.insert( uid, (const int *)1 ); - if ( mTimeOfSeenMsgsVector.empty() ) { - mTimeOfNextSeenMsgsMap.insert( uid, time(0) ); - } - else { - // cast the int* with a long to can convert it to a int, BTW - // works with g++-4.0 and amd64 - mTimeOfNextSeenMsgsMap.insert( uid, - mTimeOfSeenMsgsVector[(int)( long )mUidsOfSeenMsgsDict[uid] - 1] ); - } - } - mUidForIdMap.insert( id, uid ); + else { + stage = Idle; + if (job) job->kill(); + job = 0; + mSlave = 0; + + KMessageBox::error(0, i18n( "Unable to complete LIST operation." ), + i18n("Invalid Response From Server")); + return; } } - else { - stage = Idle; - if (job) job->kill(); - job = 0; - mSlave = 0; - KMessageBox::error(0, i18n( "Unable to complete LIST operation." ), - i18n("Invalid Response From Server")); - return; + else { // stage == Uidl + + Q_ASSERT ( stage == Uidl); + + QString id; + QString uid; + + if (spc <=0) + { + // an invalid uidl line.. we might just need to skip it, but + // some servers generate invalid uids with valid ids.. in that + // case we will just make up a uid - which will cause us to + // not cache the document, but we will be able to interoperate + + int testid = atoi (qdata.ascii()); + + if (testid < 1) + { + // we'll just have to skip this + kdDebug(5006) << "PopAccount::slotData skipping UIDL entry due to parse error " << endl << qdata.ascii() << endl; + return; + } + + id.setNum (testid, 10); + + QString datestring, serialstring; + + serialstring.setNum (++dataCounter,10); + datestring.setNum (time(NULL),10); + uid = QString("uidlgen") + datestring + QString(".") + serialstring; + + kdDebug(5006) << "PopAccount::slotData message " << id.ascii() << "%d has bad UIDL, cannot keep a copy on server" << endl; + idsOfForcedDeletes.append( id ); + } + else + { + id = qdata.left(spc); + uid = qdata.mid(spc + 1); + } + + int *size = new int; //malloc(size_of(int)); + *size = mMsgsPendingDownload[id]; + mSizeOfNextSeenMsgsDict.insert( uid, size ); + if ( mUidsOfSeenMsgsDict.find( uid ) != 0 ) { + + if ( mMsgsPendingDownload.contains( id ) ) { + mMsgsPendingDownload.remove( id ); + } + else + kdDebug(5006) << "PopAccount::slotData synchronization failure." << endl; + idsOfMsgsToDelete.append( id ); + mUidsOfNextSeenMsgsDict.insert( uid, (const int *)1 ); + if ( mTimeOfSeenMsgsVector.empty() ) { + mTimeOfNextSeenMsgsMap.insert( uid, time(0) ); + } + else { + // cast the int* with a long to can convert it to a int, BTW + // works with g++-4.0 and amd64 + mTimeOfNextSeenMsgsMap.insert( uid, + mTimeOfSeenMsgsVector[(int)( long )mUidsOfSeenMsgsDict[uid] - 1] ); + } + } + mUidForIdMap.insert( id, uid ); } } + //----------------------------------------------------------------------------- void PopAccount::slotResult( KIO::Job* ) { diff --git a/kmail/popaccount.h b/kmail/popaccount.h index 887436a..718ac31 100644 --- a/kmail/popaccount.h +++ b/kmail/popaccount.h @@ -150,6 +150,7 @@ protected: QMap mTimeOfNextSeenMsgsMap; // map of uid to times of seen messages QDict mSizeOfNextSeenMsgsDict; QStringList idsOfMsgsToDelete; + QStringList idsOfForcedDeletes; int indexOfCurrentMsg; QValueList msgsAwaitingProcessing;