MASTER mozilla-thunderbird crashed [@NS_InitXPCOM3_P] [@nsVoidArray] [@??]

[Neil]

Binary package hint: mozilla-thunderbird

... custom reply headers cause Thunderbird to crash

When changing "mailnews.reply_header_ondate" to "On %a, %d %b %Y %H:%M:%S %z" (which is the strftime format for RFC-822-style dates), Thunderbird crashes.

Tested on Thunderbird version 1.5.0.10 (20070306), on Ubuntu 6.10 (x86).

Suggestions:
1. If a bad format is given, Thunderbird should switch to default.
2. Give an option, as a 4th mode (currently 3), for RFC-822 dates.
3. In addition, Thunderbird should support strftime-style dating in the custom mode (currently mode 3).

[Adam Guthrie (ispiked)]

Any sort of stack for the crash here would be nice. http://kb.mozillazine.org/Getting_a_stacktrace_with_gdb

[Timeless-bemail (timeless-bemail)]

stack would relate to nsTextFormatter::smprintf

smprintf assumes trusted input. the input here is very untrusted. My approach is to write a js component that implements smprintf because it can be much smarter and handle sucky things like this. Simon should remember that bug.

The crash would be stack or heap corruption probably, so it's not the kind of thing you would usefully signature. Anyway. In short: "Don't do that!" we never said you could, and if it hurts when you shoot yourself in the foot, don't.

I know of no place where we said the preference was strftime, as it happens, it's smprintf.

[Neil (nnkx00)]

Yeah, well, I ranked it minor originally because the workaround is simply not to do it. Clearly strftime isn't supposed here, so I just went back to defaults.

Stacktrace is coming...

Quick question, is there any disadvantage to running the debug version of TB instead of the normal one for everyday use?

[Neil (nnkx00)]

Binary package hint: mozilla-thunderbird

When changing "mailnews.reply_header_ondate" to "On %a, %d %b %Y %H:%M:%S %z" (which is the strftime format for RFC-822-style dates), Thunderbird crashes.

Tested on Thunderbird version 1.5.0.10 (20070306), on Ubuntu 6.10 (x86).

Crash file will be attached.

Suggestions:
1. If a bad format is given, Thunderbird should switch to default.
2. Give an option, as a 4th mode (currently 3), for RFC-822 dates.
3. In addition, Thunderbird should support strftime-style dating in the custom mode (currently mode 3).

[Neil (nnkx00)]

Crash file attached.

[John Vivirito (gnomefreak)]

Taking for retrace.

[John Vivirito (gnomefreak)]

Where did you get 1.5.0.10 for edgy from? Package: mozilla-thunderbird 1.5.0.10-0ubuntu0.6.10 here is my apt-cache policy mozilla-thunderbird for edgy

(Edgy)gnomefreak@FeistyFawn:~$ policy mozilla-thunderbird
mozilla-thunderbird:
  Installed: 1.5.0.9-0ubuntu0.6.10
  Candidate: 1.5.0.9-0ubuntu0.6.10
  Version table:
 *** 1.5.0.9-0ubuntu0.6.10 0
        500 http://security.ubuntu.com edgy-security/main Packages
        100 /var/lib/dpkg/status
     1.5.0.7-0ubuntu1 0
        500 http://archive.ubuntu.com edgy/main Packages

[Timeless-bemail (timeless-bemail)]

it'll be slower, and on Windows you'll get lots of dialogs you can turn off w/ the registry. on Linux you'll get lots of beeps if you don't silence them in some way shape or form.

Otherwise, it should be fine. I used to do it w/ Mozilla long ago.

The characteristics of your mailbox are more relevant.

[Neil (nnkx00)]

I'm afraid I'm not able to be as much help here as I should be...
I actually got the upgrade notice for tbird just the other day via autoupdate. I didn't really think twice about it since it's just micro upgrade.

I assume this is what you did, and here's my output:
neil@ion:~$ apt-cache policy mozilla-thunderbird
mozilla-thunderbird:
  Installed: 1.5.0.10-0ubuntu0.6.10
  Candidate: 1.5.0.10-0ubuntu0.6.10
  Version table:
 *** 1.5.0.10-0ubuntu0.6.10 0
        500 http://security.ubuntu.com edgy-security/main Packages
        100 /var/lib/dpkg/status
     1.5.0.7-0ubuntu1 0
        500 http://us.archive.ubuntu.com edgy/main Packages

And what do you mean, "taking for retrace"?

[Neil (nnkx00)]

Nevermind, I think I figured out "taking for retrace"...

[John Vivirito (gnomefreak)]

Taking for retrace means i am assigning to me to retrace your crash report to find out what caused the crash. i have to wait a bit for the new -dbgsym packages to roll out. I found out why i couldnt upgrade to .10 :(

[Ajschult (ajschult)]

> I know of no place where we said the preference was strftime, as it
> happens, it's smprintf.

And beyond that, your string can contain only a single "%s" (and text).
The smprintf implementation has no way to tell how many parameters are passed (it only gets one) and happily walks off the end.

The date is formatted according to your locale.

[Neil (nnkx00)]

Yeah, I know no one said it was strftime. I was just playing around with it, and that's what came to mind first (I didn't know it was smprintf).

Is there a particular reason not to change it to strftime? It'd be a lot more flexible.

[Timeless-bemail (timeless-bemail)]

well, for one, it'd break anyone currently using the pref :). which means writing migration code.

beyond that, i don't own the code in question.

[Neil (nnkx00)]

As you know, "mailnews.reply_header_type" is what determines which strings it uses when crafting the reply header. Currently:
1 - "[Author] wrote:"
2 - "On [date] [author] wrote:"
3 - User-defined reply header.
With 3 being used with custom "mailnews.reply_header_authorwrote" and "mailnews.reply_header_ondate" prefs.

I visualized just adding an option 4 to reply_header_type, which would use a new pref string, maybe, reply_full_custom, which would have the strftime options plus new letters for [author] and [author-email].

That way it wouldn't break anyone's prefs since no one is using an option 4 right now.

Do you think that's feasible?

[Timeless-bemail (timeless-bemail)]

kinda messy. not technically impossible. not something i'd personally want to implement. parsing once is imo bad enough.

that said, i'm not a module owner and wouldn't reject such a thing. although explaining such a feature to users worries me.

[Neil (nnkx00)]

Forgive me if this seems obvious to you; Thunderbird is well beyond the scope of any program I've ever written...

Why is it messy? And why is it parsing twice in this case?

[Timeless-bemail (timeless-bemail)]

because strftime and snprintf both do parsing, and reinventing either or both of them is not something we really want to do. to do what you want, we'd either have to roll our own (which means handling all the wonderful edge cases of both) or send it through to both (which means making sure that neither of them have any other amusing unintended consequences).

it's just moderately messy, again, not impossible, just messy.

I'm not opposed, just not volunteering.

FWIW, you wrote "As you know," and as it happens, I didn't. I deal in crashes, especially amusing crashes (and this counts). So I thank you for informing me about how the code-path works, because otherwise I'd have had to figure it out by reading the code (which is not something I enjoy).

Here are some fun pieces of input:
* you reply to someone whose real name is: "yibber%s yabber%t snippe%tt" and whose email address is <timeless%<email address hidden>>

So, perhaps I should restate the number of parsings as closer to 3 not 2. :)

[Hilario J. Montoliu (hjmf) (hmontoliu)]

Retrace done.

Extract from retraced stacktrace:
...
#3 <signal handler called>
#4 NS_InitXPCOM3_P (result=0x2, binDirectory=0xbf8b8936,
#5 nsVoidArray (this=0xbf8b8936) at nsVoidArray.cpp:284
#6 ?? ()
#7 ?? ()
#8 ?? ()
#9 nsVoidArray::IndexOf (this=0xffffffff, aPossibleElement=0x0)
#10 nsAutoVoidArray::Clear (this=0x98e63e8)
#11 nsStringArray (this=0x99bf2c0, aCount=-1081373764)
#12 nsSmallVoidArray::EnumerateBackwards (this=0x2,
...
Tagging as mt-confirm mt-needtestcase for further processing

[Hilario J. Montoliu (hjmf) (hmontoliu)]

Retraced Thread Stacktrace

[Hilario J. Montoliu (hjmf) (hmontoliu)]

Is this still an issue for you? We are trying to sort this issue and would like to know if this still happens.

Thank you in advance.

[Hilario J. Montoliu (hjmf) (hmontoliu)]

Neil, please, in case it is still reproducible, can you try to reproduce w/o extensions (e.g. a shown in apport retrace: «library /usr/lib/mozilla-thunderbird/extensions/{847b3a00-7ab1-11d4-8f02-006008948af5}/components/libenigmime.so not found in system packages -- ignoring» libenigmime.so seems to be involved in the crash).

Thank you,

H. Montoliu.

[Darkerer (darkerer)]

(In reply to comment #8)
> I visualized just adding an option 4 to reply_header_type

seems like it would open up some fun add-ons

[Rolf Leggewie (r0lf)]

Neil, is this still an issue for you?

[Rolf Leggewie (r0lf)]

We lack further information to work on this issue. Closing. Feel free to
reopen with the requested information if this is still a problem for
you.

[Neil (nnkx00)]

Sorry, somehow I missed the last comment on this.

No, it's not still an issue for me; and I think it was an upstream issue anyways. And they're redoing that section of code regardless, unless I'm mistaken.

Thanks.

[Steven Orr (steveo-syslang)]

thunderbird-2.0.0.21-1.fc10.i386.

As of today, I tried to set

"mailnews.reply_header_ondate" to "On %a, %d %b %Y %H:%M:%S %z"

When I set the value using the config editor, I got back 70+ instances of

Warning: more than one line!

From there, an attempt to reply to a message caused a segfault. Also, TB would not start

541 > /usr/lib/thunderbird-2.0.0.21/run-mozilla.sh: line 131: 24504 Segmentation fault "$prog" ${1+"$@"}
[1]+ Exit 139 thunderbird (wd: ~)

If the feature gets fixed I will use it. Please let me know if I can help provide more info.

[Vseerror (vseerror)]

I can't reproduce crash with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.5pre) Gecko/20091013 Shredder/3.0pre

[Neil (nnkx00)]

No longer crashes, but still displays some undesirable behavior.

Using the string I used previously[1] resulted in (what I suspect is) the %d showing 4 bytes of the date in numerical form, and then the later %s showing gibberish from memory.

But it did not crash, and replies were apparently still possible (I didn't actually try sending it with all that).

While I've sort of given up on having a 4th reply type option; I might still like to see some input validation. On the other hand, this /is/ about:config, there's already a disclaimer on the front that bad stuff can happen if you mess around in there, and that's probably sufficient. Protecting users from the stupid things they (I) do should only have to go so far.

Either way, I think we can close this bug, but I'll leave the final decision to people with more experience than me.

1. ie. changing "mailnews.reply_header_ondate" to "On %a, %d %b %Y %H:%M:%S %z".

[Neil (nnkx00)]

Oh, and last comment was using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3