522561 - crash replying to message that has attached eml [@ strlen | nsDateTimeFormatMac::FormatPRExplodedTime(nsILocale*, int, int, PRExplodedTime const*, nsAString_internal&)]

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Wayne Mery (:wsmwk)]

crash replying to message that has attached eml 
[@ strlen | nsDateTimeFormatMac::FormatPRExplodedTime(nsILocale*, int, int, PRExplodedTime const*, nsAString_internal&)]

very few 3.0b4 crashes

"replying to an email which was attached to another email" sounds like 
1. open email attached to message
2. reply 

but i can't reproduce with plain text attachment

bp-6c238359-8518-42a0-a559-1c4e82091012
replying to an email which was attached to another email (mailman admin message) this is the second time. I was able to fully reproduce it this time.
0	libSystem.B.dylib	strlen	
1	thunderbird-bin	nsDateTimeFormatMac::FormatPRExplodedTime	intl/locale/src/mac/nsDateTimeFormatMac.cpp:299
2	thunderbird-bin	nsDateTimeFormatMac::FormatPRTime	intl/locale/src/mac/nsDateTimeFormatMac.cpp:276
3	thunderbird-bin	nsMsgCompose::SendMsg	mailnews/compose/src/nsMsgCompose.cpp:2277
4	thunderbird-bin	nsMsgCompose::QuoteOriginalMessage	mailnews/compose/src/nsMsgCompose.cpp:2983
5	thunderbird-bin	nsMsgCompose::BuildQuotedMessageAndSignature	mailnews/compose/src/nsMsgCompose.cpp:3933
6	thunderbird-bin	nsMsgCompose::InitEditor	mailnews/compose/src/nsMsgCompose.cpp:1489
7	libxpcom_core.dylib	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179
8	thunderbird-bin	XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2454
9	thunderbird-bin	XPC_WN_CallMethod	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1590
10	libmozjs.dylib	js_Invoke	js/src/jsinterp.cpp:1386
11	libmozjs.dylib	js_Interpret	js/src/jsinterp.cpp:5179
12	libmozjs.dylib	js_Invoke	js/src/jsinterp.cpp:1394 

a couple others - perhaps the same person:
* replying to an attached .eml file
* replying to a message which was attached to another (mailmain admin message)

Comment 1

[Brian Warner [:warner :bwarner]]

FWIW, I hit a similar crash, consistently, when I hit the "Random Comic" link on any page on http://xkcd.com/ . bp-c5cf3ec5-84a7-4500-8559-907142091202 and also bp-2727608c-d7cf-40c1-a826-880022091202 have details. This is on FF3.5.5, OSX-10.6.2 .

Comment 2

[Wayne Mery (:wsmwk)]

a couple of the FF crashes (only 33 in one month) cite "viewing cookies" like bp-7c270717-048d-4e00-80ee-4f4532091124

all the FF crashes are also Mac-only.  Ditto Seamonkey whose comments all mention managing cookies

Comment 3

[Tony Mechelynck [:tonymec]]

Hm, Wayne, I see you added me to the CC; but don't expect me to reproduce a Mac-only crash on my Linux-only system ;-).

Comment 4

[Sidney Markowitz]

I get a consistent crash but only in one profile - not if I make a new profile - even though I disable all extensions and plugins on that profile, going to one specific URL. This is also on a Mac. The crash report referred me to this bug, I think because of the function that the crash happens in looks the same.

Crash signature is bp-bcf6d464-4ac8-40f8-91bf-7a0112100106
URL that causes this is http://www.wnj.com/cross_appeal_revisited_jjb_article/

Comment 5

[Jonathan Louie]

Should product be changed to Core, since this occurs in Firefox as well?  I also triggered this crash at xkcd.

Comment 6

[timeless]

Attachment: don't try to crash as a fallback

Comment 7

[Simon Montagu :smontagu]

Comment on attachment 422105 [details] [diff] [review]
don't try to crash as a fallback

Why is this expected to prevent the crash?

Comment 8

[timeless]

if asctime returns null, nsDependentCString gets a null pointer which it tries to measure with strlen which doesn't consider null pointers to be valid input.

Comment 9

[Simon Montagu :smontagu]

OK, but do we have any reason to believe that that is what is happening here? Other than OOM, why should asctime ever return null?

Comment 10

[timeless]

my assumption is that the structure is unhappy such that it insists on returning null. however i can't find the source for asctime (i searched).

anyway, the code is totally unnecessary, and i'd like to let crash-stats prove me right. it fits, as there aren't any other paths that i can see to strlen from this code.

Comment 11

[Simon Montagu :smontagu]

I'm not so convinced that the code is totally unnecessary. Can't you do something like

char *defaultString = asctime(tmTime);
if (defaultString)
 ...

Comment 12

[Wayne Mery (:wsmwk)]

(In reply to comment #10)
> i'd like to let crash-stats prove
> me right. it fits, as there aren't any other paths that i can see to strlen
> from this code.

We have a couple testcase users. And I've pinged some thunderbird users to see if we can't get a test message or two.

Comment 13

[timeless]

121 nsresult nsDateTimeFormatMac::FormatTMTime(nsILocale* locale, 
126 {
127   nsresult res = NS_OK;
134     stringOut.Truncate();
135     return NS_OK;
^ we obviously don't bail here

139   CopyASCIItoUTF16(nsDependentCString(asctime(tmTime)), stringOut);
^ i believe we are here

160   switch (dateFormatSelector) {
174     default:
175       NS_ERROR("Unknown nsDateFormatSelector");
176       res = NS_ERROR_FAILURE;
^ we could do something stupid here, in which case we'll later violate xpcom

182   switch (timeFormatSelector) {
194     default:
195       NS_ERROR("Unknown nsTimeFormatSelector");
196       res = NS_ERROR_FAILURE;
^ we could do something stupid here, in which case we'll later violate xpcom

254   nsAutoTArray<UniChar, 256> stringBuffer;
255   if (stringBuffer.SetLength(stringLen + 1)) {
^ if the string really doesn't fit, then we should just fail
256     CFStringGetCharacters(formattedDate, CFRangeMake(0, stringLen), stringBuffer.Elements());
^ this is where we normally fill in the string
257     stringOut.Assign(stringBuffer.Elements(), stringLen);
^ here we stomp unconditionally (well, unless we ran out of memory trying to reduce the width of the buffer, which shouldn't happen)
258   }

263   return res;
264 }

Comment 14

[Fred Wenzel [:wenzel]]

Attachment: Test Case

Thanks for emailing me, Wayne, here's a test case. I removed some of the headers, but kept the user agent intact. I can consistently reproduce the problem with this email:

- right click it in finder, open with Thunderbird
- in the email showing up, double-click the forwarded email in the attachment
- in this email, finally click on reply to sender.

After a few seconds, Thunderbird crashes.

Comment 15

[Hayo Baan]

Attachment: Test case e-mail

Test case (see file):

1. Send yourself an HTML formatted mail.
1a. Wait till you receive it back in your inbox.

2. Send yourself another mail but now with the above mail as received as attachment.
2a. Wait till you receive it back in your inbox (this is the .eml file I attached).

3. Open the mail.
3a. Open the attachment message in the mail.
3b. In the message window that now opens, click on reply and Thunderbird crashes.

This is on a Mac with OS X 10.6.2 and Thunderbird 3.0.1

Hope this makes it reproducible at your end too.

Thanks,
    Hayo Baan

Comment 16

[Fred Wenzel [:wenzel]]

(In reply to comment #15)
> 1. Send yourself an HTML formatted mail.

FWIW, HTML format doesn't seem to play a role in this bug.

Comment 17

[Simon Montagu :smontagu]

I can't reproduce the crash with the STR in comment 14. However, it's interesting to note that the reply opens:

On 7/22/28164 11:59 AM, Frederic Wenzel wrote:

Comment 18

[Simon Montagu :smontagu]

I'm on to something here: dynamic.xkcd.com/comic/random sets a cookie with "Max-Age=604800000000; http://www.meh.es/" (which also appears in crash-data comments) sets a cookie with "expires=Fri, 31-Dec-9999 23:59:59 GMT"

So I'm guessing that we have two bugs here:

1) Forwarding an attached message as in the STR is corrupting the date somehow
2) OS X 10.6 (which is where all the crashes seem to be happening) crashes when formatting dates in the remote future. 

Question arising from this: are any of the people who haven't been able to reproduce the crash on 10.6? If not, what is the system locale of those who can reproduce and those who can't?

Comment 19

[Sidney Markowitz]

You are on to something with the cookie. I started up with the empty profile that did not crash on the URL I mentioned in comment #4 and tried to look for a cookie from the wnj.com domain. I used Preferences, Privacy, delete selected cookies, then I entered wnj.com in the search box. That resulted in a new example of this crash, same date related routine:
 crash report bp-3cd9b206-1161-4bf8-898c-e05e32100128

I exported the cookies, and sure enough it has a too large time stamp field:

www.wnj.com     FALSE   /       FALSE   253402300801    Localization    TimeZone=0&UsesDaylightSavings=False&TimeZoneAbbrev=IDLW&Persists=True

How do I answer your question about system locale? I'm set for New Zealand and display dates in dd/mm/yyyy format, if that's what you're looking for.

Comment 20

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I can reproduce the crash using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.7) Gecko/20100111 Thunderbird/3.0.1 on comment #14 testcase.

Comment 21

[Sidney Markowitz]

Attachment: Test code that demonstrates asctime() platform dependent behavior

I compiled and ran the attached file (asctimetest.c) on my Mac OS 10.6.2 system that demonstrates the bug and on a linux machine. Test this using

gcc -o asctimetest asctimetest.c
./asctimetest

On the Mac it output "atime was NULL"
On the linux box it output "atime was Sat Jan 30 06:35:46 21899"

This shows that a very large year in the tm struct passed to asctime() will return NULL on Mac OS X and not on linux, answering the questions posed in comment #9 and comment #10

Comment 22

[Simon Montagu :smontagu]

Comment on attachment 422105 [details] [diff] [review]
don't try to crash as a fallback

Looks like my questions have been answered

Comment 23

[Ludovic Hirlimann [:Usul]]

(In reply to comment #22)
> (From update of attachment 422105 [details] [diff] [review])
> Looks like my questions have been answered

Does this needs sr ?

Comment 24

[Simon Montagu :smontagu]

No

Comment 25

[Brian Warner [:warner :bwarner]]

FYI, the xkcd.com "random comic" link sets a cookie with a remarkably large
max-age:

 Set-Cookie: seen_comics="[219]";Max-Age=604800000000;Path=/;Version="1"

On my linux box, testing that in python with:

 time.asctime(time.localtime(time.time()[phone]))

reports that as being in the year 21175. On my OS-X box (which crashes on
that xkcd page), I get a segfault, so it looks like python is experiencing
the same error as javascript.

Comment 26

[timeless]

thanks. the confirmation from python helps.

I think this is the key point (from my OS X 10.5 man page for 'asctime', if the one you have for 10.6 is more specific, please do provide the relevant excerpt):

     The ctime() function adjusts the time value for the current time zone, in
     the same manner as localtime().  It returns a pointer to a 26-character
     string of the form:

           Thu Nov 24 18:22:48 1986\n\0

     All of the fields have constant width.

     The ctime_r() function provides the same functionality as ctime(), except
     that the caller must provide the output buffer buf (which must be at
     least 26 characters long) to store the result.  The localtime_r() and
     gmtime_r() functions provide the same functionality as localtime() and
     gmtime(), respectively, except the caller must provide the output buffer
     result.

     The asctime() function converts the broken-out time in the structure tm
     (pointed at by *timeptr) to the form shown in the example above.

a five digit year violates the fixed width string. My guess is that Apple is enforcing this. Of late, the CRT vendors have been adding validation and fatal asserts for things like this, so it doesn't really shock me.

Comment 27

[Brian Warner [:warner :bwarner]]

The 10.6 man page says the same thing. (it'd be nice if they'd update it to mention the NULL return, but the date on this page is from 1999, so I'm not holding my breath). Oh, and I should have mentioned that my python test was done on a 10.6 box, but I expect that 10.5 behaved the same way.

Comment 28

[Daniel Holbert [:dholbert]]

Landed on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/b713e3aa3955

Comment 29

[Wayne Mery (:wsmwk)]

Comment on attachment 422105 [details] [diff] [review]
don't try to crash as a fallback

#30 crash for 3.0.1, and so after some baking it seems this should be driven in to thunderbird 3.1, and 3.0 if possible.  one line patch seems minor

Comment 30

[Simon Montagu :smontagu]

I spun off the issue with the corrupt date in the reply to an attached email into bug 544359

Comment 32

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 422105 [details] [diff] [review]
don't try to crash as a fallback

a=beltzner for 1.9.2.2 and 1.9.1.9

Comment 33

[timeless]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5d8e5cbfb206

http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/intl/locale/src/mac/nsDateTimeFormatMac.cpp&mark=1.54

Comment 34

[timeless]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b6663144e617

http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/intl/locale/src/mac/nsDateTimeFormatMac.cpp&mark=1.55

sorry, i confused branches

Comment 35

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.9) Gecko/20100322 Shredder/3.0.5pre.

The attached e-mail still crashes in 1.9.2 though with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.2pre) Gecko/20100322 Lanikai/3.1b2pre so this isn't fixed there.

Comment 36

[Al Billings [:abillings - ex-MoCo]]

Here is the crash from crash-stats: http://crash-stats.mozilla.com/report/index/286714be-d6cb-4e7f-8ad9-963352100322

Comment 37

[timeless]

http://hg.mozilla.org/releases/mozilla-1.9.2/log?rev=5d8e5cbfb206 this landed roughly march 4

COMM192_20100302_RELBRANCH branched 2 days earlier.

Comment 38

[Al Billings [:abillings - ex-MoCo]]

Standard8 made a build for me with the fix and I've verified that it fixes the issue on 1.9.2.