112333 - Leaking charset on fwd'ing inline

Status: VERIFIED FIXED

(MailNews Core :: MIME, defect)

Description

[Navin Gupta]

Distribution of leaked blocks
        Allocation location
        malloc         [dbgheap.c:129]
        PL_strdup      [strdup.c:46]
        nsCRT::strdup(char const*) [nsCRT.h:182]
        mime_decompose_file_init_fn(void *,MimeHeaders *) [mimedrft.cpp:1682]
        
          if (!nAttachments && !mdd->messageBody)
          {
     =>     // if we've been told to use an override charset then do 
so....otherwise use the charset
            // inside the message header...
            if (mdd->options && mdd->options->override_charset)
                mdd->mailcharset = nsCRT::strdup(mdd->options->default_charset);
        MimeMessage_add_child [mimemsg.cpp:603]
            int status = 0;
            status = parent->options->decompose_file_init_fn (
                                                      
parent->options->stream_closure,
     =>                                               
((MimeMessage*)parent)->hdrs );
                        if ( status < 0 ) return status;
                      }
                    #endif /* MIME_DRAFTS */
                MimeMessage_close_headers [mimemsg.cpp:442]
                    
                      PR_FREEIF(ct);
                      if (!body) return MIME_OUT_OF_MEMORY;
                 =>   status = ((MimeContainerClass *) obj->clazz)->add_child 
(obj, body);
                      if (status < 0)
                        {
                          mime_free(body);
                MimeMessage_parse_line [mimemsg.cpp:271]
                       */
                      if (*line == nsCRT::CR || *line == nsCRT::LF)
                        {
                 =>       status = MimeMessage_close_headers(obj);
                          if (status < 0) return status;
                        }
                    
                convert_and_send_buffer [mimebuf.cpp:168]
                        }
                    #endif
                    
                 =>   return (*per_line_fn)(buf, length, closure);
                    }
                    
                    extern "C" int
                mime_LineBuffer [mimebuf.cpp:255]
                    
                          status = convert_and_send_buffer(*bufferP, 
*buffer_fpP,
                                                               
convert_newlines_p,
                 =>                                            per_line_fn, 
closure);
                          if (status < 0)
                          return status;
                    
                MimeObject_parse_buffer [mimeobj.cpp:255]

Comment 1

[Navin Gupta]

Attachment: proposed fix

The fix is to free mdd->mailcharset in parse_stream_complete and in
parse_stream_abort

Comment 2

[Navin Gupta]

cc ducarroz and bienvenu for review.

Comment 3

[David :Bienvenu]

Comment on attachment 59484 [details] [diff] [review]
proposed fix

if it's ok with jf, it's ok with me (but please don't take this as an r/sr -
let JF look at it too)

Comment 4

[Jean-Francois Ducarroz]

Comment on attachment 59484 [details] [diff] [review]
proposed fix

I am ok with the fact we release the mailcharset, it's the right thing to do as
it's allocated and use only in this file. However, I looked at the code around
line 1549 and it seems wrong:

    mime_free_attachments( mdd->attachments, mdd->attachments_count );
  }

  PR_FREEIF(mdd);

  // Release the prefs service
  MimeObject *obj = (mdd ? mdd->obj : 0);  
  if ( (obj) && (obj->options) && (obj->options->prefs) )
    nsServiceManager::ReleaseService(kPrefCID, obj->options->prefs);

  PR_Free (mdd);

You should remove the first PR_FREEIF(mdd), it's bogus. But keep you line to
free the mailcharset.

Comment 5

[Navin Gupta]

Attachment: proposed fix

patch as per your suggestion

Comment 6

[Jean-Francois Ducarroz]

Comment on attachment 59596 [details] [diff] [review]
proposed fix

r=ducarroz

Comment 7

[Navin Gupta]

fixed

Comment 8

[[e-mail] (gone - use [e-mail] instead)]

Verified FIXED using the latest trunk on Windows 2000 with Purify.