From e6749e22c14aa28574e4b34e067c8990656fc5d5 Mon Sep 17 00:00:00 2001 From: Thomas McGuire Date: Wed, 18 Mar 2009 13:02:40 +0000 Subject: [PATCH] Merge the forwarding fixes. Tested and seems to still work. -- Merged revisions 940772,940775,940777,940816 via svnmerge from svn+ssh://tmcguire@svn.kde.org/home/kde/branches/KDE/4.2/kdepim ........ r940772 | tmcguire | 2009-03-18 10:32:23 +0100 (Wed, 18 Mar 2009) | 20 lines Backport r940441 by tmcguire from trunk to the 4.2 branch: Don't produce garbage when forwarding a mail with a filter. For this, I had to remove the code duplication in KMFilterActionForward::process() and use KMMessage::createForward() instead. This uncovered a bug in createForward (content type boundary was not saved) and a small bug in the template parser. Also, I added a KMMessage::dump() function to help with debugging mimelib related troubles. Not perfect yet, the way the template parser copies the message needs to be improved, right now, when forwarding a signed message, the signed text gets replaced with the template text, resulting in an invalid signature ... CCBUG: 174942 ........ r940775 | tmcguire | 2009-03-18 10:34:02 +0100 (Wed, 18 Mar 2009) | 11 lines Backport r940516 by tmcguire from trunk to the 4.2 branch: When replacing the text of a message after processing the template, don't try to just replace the first text/plain part, this will cause trouble with signatures and possibly many other situations. Instead, delete the content of the old message, and set the processed template text as the first body part. If the old message had attachments, re-add them to the new message. ........ r940777 | tmcguire | 2009-03-18 10:36:48 +0100 (Wed, 18 Mar 2009) | 9 lines Backport r940547 by tmcguire from trunk to the 4.2 branch: Make valgrind happy. This didn't find the cause of the problem I was searching for, though (inline forwarding runs out of memory because allocating insanley huge amounts of memory) ........ r940816 | tmcguire | 2009-03-18 11:52:19 +0100 (Wed, 18 Mar 2009) | 8 lines Backport r940565 by tmcguire from trunk to the 4.2 branch: Don't run out of memory when forwarding a mail with more than one attachment. The mimelib behavior here is very irritating... ........ svn path=/branches/kdepim/enterprise4/kdepim/; revision=940873 --- kmfilteraction.cpp | 84 ++++++---------------------------------------- kmmessage.cpp | 52 +++++++++++++++++++++++----- kmmessage.h | 23 ++++++++++++- templateparser.cpp | 69 +++++++++++++++++++++++++++++++------ templateparser.h | 16 +++++++-- 5 files changed, 150 insertions(+), 94 deletions(-) diff --git a/kmfilteraction.cpp b/kmfilteraction.cpp index dfc7a2c76..97b8695ec 100644 --- a/kmfilteraction.cpp +++ b/kmfilteraction.cpp @@ -1462,92 +1462,30 @@ KMFilterActionForward::KMFilterActionForward() { } -KMFilterAction::ReturnCode KMFilterActionForward::process( KMMessage *aMsg ) const +KMFilterAction::ReturnCode KMFilterActionForward::process( KMMessage *msg ) const { if ( mParameter.isEmpty() ) return ErrorButGoOn; // avoid endless loops when this action is used in a filter // which applies to sent messages - if ( KMMessage::addressIsInAddressList( mParameter, QStringList( aMsg->to() ) ) ) + if ( KMMessage::addressIsInAddressList( mParameter, QStringList( msg->to() ) ) ) { + kWarning() << "Attempt to forward to receipient of original message, ignoring."; return ErrorButGoOn; + } - // Create the forwarded message by hand to make forwarding of messages with - // attachments work. - // Note: This duplicates a lot of code from KMMessage::createForward() and - // KMComposeWin::applyChanges(). - // ### FIXME: Remove the code duplication again. - - KMMessage* msg = new KMMessage; - - msg->initFromMessage( aMsg ); - - // QString st = QString::fromUtf8( aMsg->createForwardBody() ); - TemplateParser parser( msg, TemplateParser::Forward, - aMsg->body(), false, false, false); - parser.process( aMsg ); - - QByteArray - encoding = KMMsgBase::autoDetectCharset( aMsg->charset(), - KMMessage::preferredCharsets(), - msg->body() ); - if( encoding.isEmpty() ) - encoding = "utf-8"; - QByteArray str = KMMsgBase::codecForName( encoding )->fromUnicode( msg->body() ); - - msg->setCharset( encoding ); - msg->setTo( mParameter ); - msg->setSubject( "Fwd: " + aMsg->subject() ); - - bool isQP = kmkernel->msgSender()->sendQuotedPrintable(); + KMMessage *fwdMsg = msg->createForward(); + fwdMsg->setTo( mParameter ); - if( aMsg->numBodyParts() == 0 ) - { - msg->setAutomaticFields( true ); - msg->setHeaderField( "Content-Type", "text/plain" ); - // msg->setCteStr( isQP ? "quoted-printable": "8bit" ); - QList dummy; - msg->setBodyAndGuessCte(str, dummy, !isQP); - msg->setCharset( encoding ); - if( isQP ) - msg->setBodyEncoded( str ); - else - msg->setBody( str ); + if ( !kmkernel->msgSender()->send( fwdMsg, KMail::MessageSender::SendLater ) ) { + kWarning() << "KMFilterAction: could not forward message (sending failed)"; + return ErrorButGoOn; // error: couldn't send } else - { - KMMessagePart bodyPart, msgPart; - - msg->removeHeaderField( "Content-Type" ); - msg->removeHeaderField( "Content-Transfer-Encoding" ); - msg->setAutomaticFields( true ); - msg->setBody( "This message is in MIME format.\n\n" ); - - bodyPart.setTypeStr( "text" ); - bodyPart.setSubtypeStr( "plain" ); - // bodyPart.setCteStr( isQP ? "quoted-printable": "8bit" ); - QList dummy; - bodyPart.setBodyAndGuessCte(str, dummy, !isQP); - bodyPart.setCharset( encoding ); - bodyPart.setBodyEncoded( str ); - msg->addBodyPart( &bodyPart ); - - for( int i = 0; i < aMsg->numBodyParts(); i++ ) - { - aMsg->bodyPart( i, &msgPart ); - if( i > 0 || qstricmp( msgPart.typeStr(), "text" ) != 0 ) - msg->addBodyPart( &msgPart ); - } - } - msg->cleanupHeader(); - msg->link( aMsg, MessageStatus::statusForwarded() ); + sendMDN( msg, KMime::MDN::Dispatched ); - sendMDN( aMsg, KMime::MDN::Dispatched ); + // (the msgSender takes ownership of the message, so don't delete it here) - if ( !kmkernel->msgSender()->send( msg, KMail::MessageSender::SendDefault ) ) { - kDebug(5006) <<"KMFilterAction: could not forward message (sending failed)"; - return ErrorButGoOn; // error: couldn't send - } return GoOn; } diff --git a/kmmessage.cpp b/kmmessage.cpp index 4a49b112a..a3454147d 100644 --- a/kmmessage.cpp +++ b/kmmessage.cpp @@ -1216,7 +1216,6 @@ void KMMessage::sanitizeHeaders( const QStringList& whiteList ) KMMessage* KMMessage::createForward( const QString &tmpl /* = QString() */ ) { KMMessage* msg = new KMMessage(); - QString id; // If this is a multipart mail or if the main part is only the text part, // Make an identical copy of the mail, minus headers, so attachments are @@ -1227,8 +1226,7 @@ KMMessage* KMMessage::createForward( const QString &tmpl /* = QString() */ ) msg->fromDwString( this->asDwString() ); // remember the type and subtype, initFromMessage sets the contents type to // text/plain, via initHeader, for unclear reasons - const int type = msg->type(); - const int subtype = msg->subtype(); + DwMediaType oldContentType = msg->mMsg->Headers().ContentType(); msg->sanitizeHeaders(); @@ -1245,12 +1243,14 @@ KMMessage* KMMessage::createForward( const QString &tmpl /* = QString() */ ) } } msg->mMsg->Assemble(); - msg->initFromMessage( this ); + //restore type - msg->setType( type ); - msg->setSubtype( subtype ); - } else if( type() == DwMime::kTypeText && subtype() == DwMime::kSubtypeHtml ) { + msg->mMsg->Headers().ContentType().FromString( oldContentType.AsString() ); + msg->mMsg->Headers().ContentType().Parse(); + msg->mMsg->Assemble(); + } + else if( type() == DwMime::kTypeText && subtype() == DwMime::kSubtypeHtml ) { // This is non-multipart html mail. Let`s make it text/plain and allow // template parser do the hard job. msg->initFromMessage( this ); @@ -1258,7 +1258,8 @@ KMMessage* KMMessage::createForward( const QString &tmpl /* = QString() */ ) msg->setSubtype( DwMime::kSubtypeHtml ); msg->mNeedsAssembly = true; msg->cleanupHeader(); - } else { + } + else { // This is a non-multipart, non-text mail (e.g. text/calendar). Construct // a multipart/mixed mail and add the original body as an attachment. msg->initFromMessage( this ); @@ -2561,6 +2562,16 @@ void KMMessage::setNeedsAssembly() mNeedsAssembly = true; } +//----------------------------------------------------------------------------- +void KMMessage::assembleIfNeeded() +{ + Q_ASSERT( mMsg ); + + if ( mNeedsAssembly ) { + mMsg->Assemble(); + mNeedsAssembly = false; + } +} //----------------------------------------------------------------------------- QByteArray KMMessage::body() const @@ -2719,6 +2730,8 @@ void KMMessage::setBodyEncodedBinary( const QByteArray& bodyStr, DwEntity *entit } entity->Body().FromString( dwResult ); + entity->Body().Parse(); + mNeedsAssembly = true; } @@ -4342,3 +4355,26 @@ void KMMessage::deleteWhenUnused() { s->pendingDeletes << this; } + +#ifndef NDEBUG +void KMMessage::dump( DwEntity *entity, int level ) +{ + if ( !entity ) + entity = mMsg; + + QString spaces; + for ( int i = 1; i <= level; i++ ) + spaces += " "; + + kDebug() << QString( spaces + "Headers of entity " + entity->partId() + ":" ); + kDebug() << QString( spaces + entity->Headers().AsString().c_str() ); + kDebug() << QString( spaces + "Body of entity " + entity->partId() + ":" ); + kDebug() << QString( spaces + entity->Body().AsString().c_str() ); + + DwBodyPart *current = entity->Body().FirstBodyPart(); + while ( current ) { + dump( current, level + 1 ); + current = current->Next(); + } +} +#endif diff --git a/kmmessage.h b/kmmessage.h index 75454b9cd..ca316b40e 100644 --- a/kmmessage.h +++ b/kmmessage.h @@ -490,6 +490,12 @@ public: the header via headers() function) */ void setNeedsAssembly(); + /** + * Assemble the internal message. This is done automatically in most + * cases, but sometimes still necessary to call this manually. + */ + void assembleIfNeeded(); + /** * Get or set the 'Content-Transfer-Encoding' header field * The member functions that involve enumerated types (ints) @@ -890,6 +896,21 @@ public: /** Delete this message as soon as it no longer in use. */ void deleteWhenUnused(); +#ifndef NDEBUG + + /** + * Dump the internal mimelib message structure to kDebug(). + * This is useful if there are inconsistencies because of a missing + * Parse() or Assemble(). + * + * This function is recursive, pass 0 as level when calling this with + * the root entity. + * + * If entity is 0, the root will be dumped. + */ + void dump( DwEntity *entity = 0, int level = 0 ); +#endif + private: /** Initialization shared by the ctors. */ @@ -900,7 +921,7 @@ private: QString mDrafts; QString mTemplates; mutable DwMessage* mMsg; - mutable bool mNeedsAssembly :1; + mutable bool mNeedsAssembly :1; bool mDecodeHTML :1; bool mReadyToShow :1; bool mComplete :1; diff --git a/templateparser.cpp b/templateparser.cpp index cfaa28680..75ef78eab 100644 --- a/templateparser.cpp +++ b/templateparser.cpp @@ -27,6 +27,8 @@ #include "customtemplates_kfg.h" #include "globalsettings_base.h" #include "kmkernel.h" +#include "partNode.h" +#include "attachmentcollector.h" #include @@ -851,25 +853,72 @@ void TemplateParser::processWithTemplate( const QString &tmpl ) } } - // kDebug(5006) << "Message body:" << body; + addProcessedBodyToMessage( body ); +} + +void TemplateParser::addProcessedBodyToMessage( const QString &body ) +{ if ( mAppend ) { + + // ### What happens here if the body is multipart or in some way encoded? QByteArray msg_body = mMsg->body(); msg_body.append( body.toUtf8() ); mMsg->setBody( msg_body ); - } else { - DwEntity *entityToChange = 0; - if ( mMsg->typeStr().toLower() == "multipart" ) { - entityToChange = mMsg->findDwBodyPart( "text", "plain" ); - if ( !entityToChange ) - kWarning() << "No text/plain part found in this multipart message, " - "template parser can not set the text!"; + } + else { + + // Get the attachments of the original mail + partNode *root = partNode::fromMessage( mMsg ); + KMail::AttachmentCollector ac; + ac.setDiveIntoEncryptions( true ); + ac.setDiveIntoSignatures( true ); + ac.setDiveIntoMessages( false ); + ac.collectAttachmentsFrom( root ); + + // Now, delete the old content and set the new content, which + // is either only the new text or the new text with some attachments. + mMsg->deleteBodyParts(); + + // If we have no attachment, simply create a text/plain part and + // set the processed template text as the body + if ( ac.attachments().empty() ) { + mMsg->headers().ContentType().FromString( DwString() ); // to get rid of old boundary + mMsg->headers().ContentType().Parse(); + mMsg->headers().ContentType().SetType( DwMime::kTypeText ); + mMsg->headers().ContentType().SetSubtype( DwMime::kSubtypePlain ); + mMsg->headers().Assemble(); + mMsg->setBodyFromUnicode( body ); + mMsg->assembleIfNeeded(); } - mMsg->setBodyFromUnicode( body, entityToChange ); + // If we have some attachments, create a multipart/mixed mail and + // add the normal body as well as the attachments + else + { + mMsg->headers().ContentType().SetType( DwMime::kTypeMultipart ); + mMsg->headers().ContentType().SetSubtype( DwMime::kSubtypeMixed ); + mMsg->headers().ContentType().CreateBoundary( 0 ); + + KMMessagePart textPart; + textPart.setBodyFromUnicode( body ); + mMsg->addDwBodyPart( mMsg->createDWBodyPart( &textPart ) ); + mMsg->assembleIfNeeded(); + + foreach( const partNode *attachment, ac.attachments() ) { + + // When adding this body part, make sure to _not_ add the next bodypart + // as well, which mimelib would do, therefore creating a mail with many + // duplicate attachments (so many that KMail runs out of memory, in fact). + // Body::AddBodyPart is very misleading here... + attachment->dwPart()->SetNext( 0 ); + + mMsg->addDwBodyPart( attachment->dwPart() ); + mMsg->assembleIfNeeded(); + } + } } } - QString TemplateParser::findCustomTemplate( const QString &tmplName ) { CTemplates t( tmplName ); diff --git a/templateparser.h b/templateparser.h index 1f0aaa3ef..4a26c0a40 100644 --- a/templateparser.h +++ b/templateparser.h @@ -47,10 +47,11 @@ class TemplateParser : public QObject bool aSmartQuote, bool aallowDecryption, bool aselectionIsBody ); - virtual void process( KMMessage *aorig_msg, KMFolder *afolder = NULL, bool append = false ); + virtual void process( KMMessage *aorig_msg, KMFolder *afolder = 0, bool append = false ); virtual void process( const QString &tmplName, KMMessage *aorig_msg, - KMFolder *afolder = NULL, bool append = false ); + KMFolder *afolder = 0, bool append = false ); virtual void processWithTemplate( const QString &tmpl ); + virtual QString findTemplate(); virtual QString findCustomTemplate( const QString &tmpl ); virtual QString pipe( const QString &cmd, const QString &buf ); @@ -72,6 +73,17 @@ class TemplateParser : public QObject QString mQuoteString; bool mAppend; + /** + * Called by processWithTemplate(). This adds the completely processed body to + * the message. + * + * In append mode, this will simply append the text to the body. + * + * Otherwise, the content of the old message is deleted and replaced with @p body. + * Attachments of the original message are also added back to the new message. + */ + void addProcessedBodyToMessage( const QString &body ); + int parseQuotes( const QString &prefix, const QString &str, QString "e ) const; };