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

@@ -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<int> 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<int> 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;
 }
 

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

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;

templateparser.cpp

@@ -27,6 +27,8 @@
 #include "customtemplates_kfg.h"
 #include "globalsettings_base.h"
 #include "kmkernel.h"
+#include "partNode.h"
+#include "attachmentcollector.h"
 
 #include <mimelib/bodypart.h>
 
@@ -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 );

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 &quote ) const;
 };