Leaking charset on fwd'ing inline

VERIFIED FIXED

Status

MailNews Core
MIME
VERIFIED FIXED
16 years ago
10 years ago

People

(Reporter: Navin Gupta, Assigned: Navin Gupta)

Tracking

({mlk})

Trunk
x86
Windows NT

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

792 bytes, patch
Jean-Francois Ducarroz
: review+
Details | Diff | Splinter Review
(Assignee)

Description

16 years ago
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]
(Assignee)

Comment 1

16 years ago
Created attachment 59484 [details] [diff] [review]
proposed fix

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

Comment 2

16 years ago
cc ducarroz and bienvenu for review.
QA Contact: esther → stephend

Comment 3

16 years ago
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)
Attachment #59484 - Flags: superreview+
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.
Attachment #59484 - Flags: needs-work+
(Assignee)

Comment 5

16 years ago
Created attachment 59596 [details] [diff] [review]
proposed fix

patch as per your suggestion
Attachment #59484 - Attachment is obsolete: true
Comment on attachment 59596 [details] [diff] [review]
proposed fix

r=ducarroz
Attachment #59596 - Flags: review+
(Assignee)

Comment 7

16 years ago
fixed
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Verified FIXED using the latest trunk on Windows 2000 with Purify.
Status: RESOLVED → VERIFIED
Keywords: mlk
Summary: MLK:leaking charset on fwd'ing inline → Leaking charset on fwd'ing inline
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.