Closed
Bug 479206
Opened 15 years ago
Closed 15 years ago
Crash [@ construct_addresslist] when entering an invalid email address
Categories
(MailNews Core :: Composition, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 3.0b2
People
(Reporter: standard8, Assigned: standard8)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(2 files)
1.60 KB,
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
992 bytes,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
STR: 1) Compose an email 2) Set the address to: test test@email> 3) Try to send the mail -> crashes Stack: #0 0x96d8fe70 in strlen () #1 0x96db2067 in strdup () #2 0x114f3196 in construct_addresslist (s=0x6135016 ">") at /Users/moztest/comm/main/src/mailnews/mime/src/comi18n.cpp:554 #3 0x114f4825 in apply_rfc2047_encoding (_src=0x6136050 "\"\"\"Moztest moztest\"@standard8.plus.com>\"", structured=1, charset=0x610ce60 "UTF-8", cursor=4, foldlen=72) at /Users/moztest/comm/main/src/mailnews/mime/src/comi18n.cpp:629 #4 0x114f4e35 in MIME_EncodeMimePartIIStr (header=0x6136050 "\"\"\"Moztest moztest\"@standard8.plus.com>\"", structured=1, mailCharset=0x610ce60 "UTF-8", fieldNameLen=4, encodedWordSize=72) at /Users/moztest/comm/main/src/mailnews/mime/src/comi18n.cpp:769 #5 0x114cbb9a in nsMimeConverter::EncodeMimePartIIStr_UTF8 (this=0x1d450c10, header=@0xbfffd714, structured=1, mailCharset=0x610ce60 "UTF-8", fieldnamelen=4, encodedWordSize=72, encodedString=0xbfffd674) at /Users/moztest/comm/main/src/mailnews/mime/src/nsMimeConverter.cpp:133 #6 0x1117768f in nsMsgI18NEncodeMimePartIIStr (header=0x6136050 "\"\"\"Moztest moztest\"@standard8.plus.com>\"", structured=1, charset=0x610ce60 "UTF-8", fieldnamelen=4, usemime=1) at /Users/moztest/comm/main/src/mailnews/base/util/nsMsgI18N.cpp:249 #7 0x112f90bf in mime_generate_headers (fields=0x610c900, charset=0x610ce60 "UTF-8", deliver_mode=0, aPrompt=0x4a87da0, status=0xbfffdce4) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgCompUtils.cpp:606 #8 0x112d6ab4 in nsMsgComposeAndSend::GatherMimeAttachments (this=0x132a02d0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgSend.cpp:993 #9 0x11300832 in nsMsgAttachmentHandler::UrlExit (this=0x132a1cd0, status=0, aMsg=0x0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgAttachmentHandler.cpp:1350 #10 0x11300a38 in FetcherURLDoneCallback (aStatus=0, aContentType=@0x6116bfc, aCharset=@0x6116c08, totalSize=216, aMsg=0x0, tagData=0x132a1cd0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgAttachmentHandler.cpp:535 #11 0x113311e7 in nsURLFetcher::OnStopRequest (this=0x6116bb0, request=0x61376b0, ctxt=0x0, aStatus=0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsURLFetcher.cpp:327 #12 0x14b4a824 in nsDocumentOpenInfo::OnStopRequest (this=0x612b490, request=0x61376b0, aCtxt=0x0, aStatus=0) at /Users/moztest/comm/main/src/mozilla/uriloader/base/nsURILoader.cpp:323 #13 0x11c2f69b in nsBaseChannel::OnStopRequest (this=0x6137680, request=0x612a750, ctxt=0x0, status=0) at /Users/moztest/comm/main/src/mozilla/netwerk/base/src/nsBaseChannel.cpp:680 #14 0x11c42804 in nsInputStreamPump::OnStateStop (this=0x612a750) at /Users/moztest/comm/main/src/mozilla/netwerk/base/src/nsInputStreamPump.cpp:576 #15 0x11c42924 in nsInputStreamPump::OnInputStreamReady (this=0x612a750, stream=0x610069c) at /Users/moztest/comm/main/src/mozilla/netwerk/base/src/nsInputStreamPump.cpp:401 #16 0x004df092 in nsInputStreamReadyEvent::Run (this=0x6128820) at /Users/moztest/comm/main/src/mozilla/xpcom/io/nsStreamUtils.cpp:111 #17 0x00511948 in nsThread::ProcessNextEvent (this=0x713bf0, mayWait=0, result=0xbfffe254) at /Users/moztest/comm/main/src/mozilla/xpcom/threads/nsThread.cpp:510 #18 0x0049ae0e in NS_ProcessPendingEvents_P (thread=0x713bf0, timeout=20) at nsThreadUtils.cpp:180 #19 0x120793a3 in nsBaseAppShell::NativeEventCallback (this=0x740710) at /Users/moztest/comm/main/src/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:121 #20 0x120308f0 in nsAppShell::ProcessGeckoEvents (aInfo=0x740710) at /Users/moztest/comm/main/src/mozilla/widget/src/cocoa/nsAppShell.mm:374 #21 0x906235f5 in CFRunLoopRunSpecific () #22 0x90623cd8 in CFRunLoopRunInMode () #23 0x92d682c0 in RunCurrentEventLoopInMode () #24 0x92d68012 in ReceiveNextEventCommon () #25 0x92d67f4d in BlockUntilNextEventMatchingListInMode () #26 0x95e27d7d in _DPSNextEvent () #27 0x95e27630 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #28 0x95e2066b in -[NSApplication run] () #29 0x1202efea in nsAppShell::Run (this=0x740710) at /Users/moztest/comm/main/src/mozilla/widget/src/cocoa/nsAppShell.mm:693 #30 0x12e923b2 in nsAppStartup::Run (this=0x758e50) at /Users/moztest/comm/main/src/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:192 #31 0x000b3d04 in XRE_main (argc=1, argv=0xbffff7d8, aAppData=0x70df70) at /Users/moztest/comm/main/src/mozilla/toolkit/xre/nsAppRunner.cpp:3279 #32 0x0000285f in main (argc=1, argv=0xbffff7d8) at /Users/moztest/comm/main/src/mail/app/nsMailApp.cpp:103 Not sure when it regressed, may be useful to know if it happens in b1.
Flags: blocking-thunderbird3?
Flags: blocking-seamonkey2.0a3?
Comment 1•15 years ago
|
||
My server rejects the address as invalid "5.1.3 User address required". So dependent on server response?
Severity: normal → critical
Keywords: regressionwindow-wanted
Assignee | ||
Comment 2•15 years ago
|
||
(In reply to comment #1) > My server rejects the address as invalid "5.1.3 User address required". So > dependent on server response? I got something similar on Windows. Mac seems to be the only one crashing at the moment...
Comment 3•15 years ago
|
||
I actually tried on a self-built SeaMonkey from a few days ago (Gecko/20090216) on Linux and it "successfully" crashed.
Assignee | ||
Comment 4•15 years ago
|
||
FWIW I just reproduced this with beta 1.
Comment 5•15 years ago
|
||
This goes wayy back. I crash with both TB3 alpha 1 and alpha 2 using the steps in comment #0. Time for zzz - else someone should test with TB 2.0.0.x...
Comment 6•15 years ago
|
||
(In reply to comment #5) > Time for zzz - else someone should test with TB 2.0.0.x... This does not crash with TB 2.0.0.19 -- official release mirrors download quickly but not for Mozilla FTP, another reason for me to go to bed. The current regression window range is now (when 1.9 separated from 1.8.1.x) and TB3 alpha 1 release date. Zzz..
Comment 7•15 years ago
|
||
Whoopie, regression window found. :) does not crash in 20070611 Mac nightly. crashes in 20070612 Mac nightly. bonsai for regression range: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fmail%2F+mozilla%2Fmailnews&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-06-11+02%3A00%3A00&maxdate=2007-06-12+04%3A00%3A00&cvsroot=%2Fcvsroot Suspect: bug 379070 (Remove nsXPIDLCString / nsXPIDLString from mailnews) since it's the only one during that time period that touches compose, and anything containing the word "address" doesn't exist during that time period. Probably https://bugzilla.mozilla.org/show_bug.cgi?id=379070#c197 . bienvenu, seems to be your sr on a patch by mscott. (I hope my analysis is correct, feel free to set Depends on or Blocking as per needed)
Keywords: regressionwindow-wanted
Comment 8•15 years ago
|
||
Archived off a conversation in #maildev... [19:55] nth10sd> NeilAway: i hope i am right in my analysis though [20:01] NeilAway> nth10sd: well, 379070 could well be suspicious [20:01] nth10sd> true [20:01] NeilAway> nth10sd: since mime is on the stack... [20:02] NeilAway> nth10sd: chances are it's trying to strdup a null pointer [20:03] NeilAway> nth10sd: yeah, what happens is this [20:03] NeilAway> nth10sd: construct_addresslist looks for the "<" at the beginning of the address, and the ">" at the end [20:04] NeilAway> nth10sd: when it finds the ">" it tries to make a copy of the substring [20:04] NeilAway> nth10sd: this doesn't work out well when it never found a "<" [20:04] NeilAway> nth10sd: probably the old nsCRT::strdup (?) was null-safe If MIME is part of the culprit, https://bugzilla.mozilla.org/show_bug.cgi?id=379070#c196 might be the real cause.. (I see nsCRT::strdup being removed and strdup somewhere being added in that patch) I guess the QA work here will suffice for the moment. :)
Assignee | ||
Comment 9•15 years ago
|
||
The problem (at least on mac) is that these aren't being null-checked before strdup, therefore add checks, and deal appropriately. This fixes the problem for me on mac. I'll see if I can come up with a unit test later.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #363568 -
Flags: superreview?(neil)
Attachment #363568 -
Flags: review?(neil)
Comment 10•15 years ago
|
||
This same pattern (i.e, assuming that the string is non-null) is happening in other spots, see attachment 267283 [details] [diff] [review] from bug 379070. Is there value in preemptively doing something similar for other cases of *strdup in another bug?
Comment 11•15 years ago
|
||
(In reply to comment #10) > This same pattern (i.e, assuming that the string is non-null) is happening in > other spots, see attachment 267283 [details] [diff] [review] from bug 379070. Is there value in > preemptively doing something similar for other cases of *strdup in another bug? Or would making strdup null-safe be a better approach?
Updated•15 years ago
|
Attachment #363568 -
Flags: superreview?(neil)
Attachment #363568 -
Flags: superreview+
Attachment #363568 -
Flags: review?(neil)
Attachment #363568 -
Flags: review+
Assignee | ||
Comment 12•15 years ago
|
||
Attachment #363607 -
Flags: review?(neil)
Comment 13•15 years ago
|
||
(In reply to comment #11) > > Or would making strdup null-safe be a better approach? No, strdup comes from the compiler/os c-library. One of the points to having a wrapper class or functions like nsCRT and PL_strdup, etc, was the ability to add null checking so part of the price of getting rid of those is losing that protection.
Assignee | ||
Updated•15 years ago
|
Attachment #363568 -
Attachment description: The fix → [checked in] The fix
Updated•15 years ago
|
Attachment #363607 -
Flags: review?(neil) → review+
Assignee | ||
Updated•15 years ago
|
Attachment #363607 -
Attachment description: Testcase → [checked in] Testcase
Assignee | ||
Comment 15•15 years ago
|
||
Currently fixed for b3, do we want to fix for b2 as well?
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b3
Comment 16•15 years ago
|
||
Both SeaMonkey and Thunderbird have apparently shipped the previous milestones with that crash and didn't really have complaints, so I guess we can ship another one with it. That said, if we are doing a respin for something else, this might be a nice ride-along.
Comment 17•15 years ago
|
||
Landed on release branch: changeset: 2048:3cd3c8d17db6 branch: COMM_20090224_RELBRANCH parent: 2044:7db1d7c5d187 user: Mark Banner <bugzilla@standard8.plus.com> date: Mon Feb 23 14:31:11 2009 +0000 summary: Testcase for Bug 479206 Crash [@ construct_addresslist] when entering an invalid email address. r=Neil changeset: 2049:2617cb2e3af3 branch: COMM_20090224_RELBRANCH tag: tip user: Mark Banner <bugzilla@standard8.plus.com> date: Sun Feb 22 22:21:26 2009 +0000 summary: Bug 479206 Crash [@ construct_addresslist] when entering an invalid email address. r/sr=Neil
Assignee | ||
Updated•15 years ago
|
Flags: blocking-thunderbird3?
Flags: blocking-seamonkey2.0a3?
Target Milestone: Thunderbird 3.0b3 → Thunderbird 3.0b2
Updated•13 years ago
|
Crash Signature: [@ construct_addresslist]
You need to log in
before you can comment on or make changes to this bug.
Description
•