Closed Bug 241128 Opened 20 years ago Closed 20 years ago

Thunderbird and Seamonkey Mail crash on some messages with JS enabled

Categories

(MailNews Core :: Backend, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7final

People

(Reporter: mscott, Assigned: dbaron)

References

Details

(Keywords: crash, fixed1.7, Whiteboard: [patch])

Attachments

(3 files, 1 obsolete file)

Talkback ID: tb26235H

 CNavDTD::HandleToken
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/htmlparser/src/CNavDTD.cpp,
line 853]
CNavDTD::BuildModel
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/htmlparser/src/CNavDTD.cpp,
line 520]
nsParser::BuildModel
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/htmlparser/src/nsParser.cpp,
line 1899]
nsParser::ResumeParse
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/htmlparser/src/nsParser.cpp,
line 1763]
nsParser::Parse
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/htmlparser/src/nsParser.cpp,
line 1659]
gkparser.dll + 0x29374 (0x604d9374)

The trick is to have JS enabled in the message pane. The messages that cause
this crash have JS in them that seem to be trying to do some interesting things.

I can't post some of the example messages that cause this but David and I can
give dbaron one of these examples.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.7final
Any chance you could send one of the examples?
the tokenizer has been deleted out from under us, it seems. I'll send you one of
the messages...

    [E] FMR: Free memory read in CNavDTD::BuildModel(nsIParser *,nsITokenizer
*,nsITokenObserver *,nsIContentSink *) {1 occurrence}
        Reading 4 bytes from 0x0c79f2f0 (4 bytes at 0x0c79f2f0 illegal)
        Address 0x0c79f2f0 is at the beginning of a 84 byte block
        Address 0x0c79f2f0 points to a C++ new block in heap 0x02420000
        Thread ID: 0x8ac
        Error location
        CNavDTD::BuildModel(nsIParser *,nsITokenizer *,nsITokenObserver
*,nsIContentSink *) [CNavDTD.cpp:509]
        nsParser::BuildModel(void) [nsParser.cpp:1894]
        nsParser::ResumeParse(int,int,int) [nsParser.cpp:1761]
        nsParser::Parse(nsAString const&,void *,nsACString
const&,int,int,nsDTDMode) [nsParser.cpp:1644]
        nsHTMLDocument::WriteCommon(nsAString const&,int) [nsHTMLDocument.cpp:2268]
        nsHTMLDocument::ScriptWriteCommon(int) [nsHTMLDocument.cpp:2354]
        nsHTMLDocument::Write(void) [nsHTMLDocument.cpp:2380]
        XPTC_InvokeByIndex [xptcinvoke.cpp:101]
        XPCWrappedNative::CallMethod(XPCCallContext&,CallMode::XPCWrappedNative)
[xpcwrappednative.cpp:2027]
        XPC_WN_CallMethod(JSContext *,JSObject *,UINT,long *,long *)
[xpcwrappednativejsops.cpp:1287]
        Allocation location
        new(UINT)      [new.cpp:23]
        NS_NewHTMLTokenizer(nsITokenizer * *,int,eParserDocType,eParserCommands)
[nsHTMLTokenizer.cpp:113]
        CParserContext::GetTokenizer(int,nsITokenizer *&) [CParserContext.cpp:177]
        nsParser::Tokenize(int) [nsParser.cpp:2541]
        nsParser::ResumeParse(int,int,int) [nsParser.cpp:1760]
        nsParser::Parse(nsAString const&,void *,nsACString
const&,int,int,nsDTDMode) [nsParser.cpp:1644]
        nsHTMLDocument::WriteCommon(nsAString const&,int) [nsHTMLDocument.cpp:2268]
        nsHTMLDocument::ScriptWriteCommon(int) [nsHTMLDocument.cpp:2354]
        nsHTMLDocument::Write(void) [nsHTMLDocument.cpp:2380]
        XPTC_InvokeByIndex [xptcinvoke.cpp:101]
        XPCWrappedNative::CallMethod(XPCCallContext&,CallMode::XPCWrappedNative)
[xpcwrappednative.cpp:2027]
        XPC_WN_CallMethod(JSContext *,JSObject *,UINT,long *,long *)
[xpcwrappednativejsops.cpp:1287]
        js_Invoke      [jsinterp.c:1281]
        js_Interpret   [jsinterp.c:3366]
        js_Execute     [jsinterp.c:1507]
        JS_EvaluateUCScriptForPrincipals [jsapi.c:3569]
        nsJSContext::EvaluateString(nsAString const&,void *,nsIPrincipal *,char
const*,UINT,char const*,nsAString&,int *) [nsJSEnvironment.cpp:942]
        nsScriptLoader::EvaluateScript(nsScriptLoadRequest *,nsString const&)
[nsScriptLoader.cpp:658]
    Free location
        delete(void *) [delop.cpp:6]
        nsHTMLTokenizer::`scalar deleting destructor'(UINT) [gkparser.dll]
        nsHTMLTokenizer::Release(void) [nsHTMLTokenizer.cpp:122]
        CParserContext::~CParserContext(void) [CParserContext.cpp:142]
        CParserContext::`scalar deleting destructor'(UINT) [gkparser.dll]
        nsParser::ResumeParse(int,int,int) [nsParser.cpp:1832]
        nsParser::ContinueParsing(void) [nsParser.cpp:1359]
        CSSLoaderImpl::SheetComplete(SheetLoadData *,int) [nsCSSLoader.cpp:1530]
        CSSLoaderImpl::LoadSheet(SheetLoadData *,StyleSheetState)
[nsCSSLoader.cpp:1367]
        CSSLoaderImpl::LoadStyleLink(nsIContent *,nsIURI *,nsAString
const&,nsAString const&,nsIParser *,int&,nsICSSLoaderObserver *)
[nsCSSLoader.cpp:1719]
        nsStyleLinkElement::UpdateStyleSheet(nsIDocument *,nsICSSLoaderObserver
*) [nsStyleLinkElement.cpp:311]
        HTMLContentSink::ProcessLINKTag(nsIParserNode const&)
[nsHTMLContentSink.cpp:4016]
        HTMLContentSink::AddLeaf(nsIParserNode const&) [nsHTMLContentSink.cpp:3194]
        HTMLContentSink::AddHeadContent(nsIParserNode const&)
[nsHTMLContentSink.cpp:3162]
        CNavDTD::AddHeadLeaf(nsIParserNode *) [CNavDTD.cpp:3839]
        CNavDTD::HandleStartToken(CToken *) [CNavDTD.cpp:1832]
        CNavDTD::HandleToken(CToken *,nsIParser *) [CNavDTD.cpp:1019]
        CNavDTD::BuildModel(nsIParser *,nsITokenizer *,nsITokenObserver
*,nsIContentSink *) [CNavDTD.cpp:511]
looks like the js is doing a document.write, and we've already deleting the
tokenizer...
Any chance you could attach longer versions of the stacks in comment 2? 
(Preferably the whole stack, if purify has it.  And probably in an attachment
rather than pasted in the bug...)
(In reply to comment #2)
>         CSSLoaderImpl::SheetComplete(SheetLoadData *,int) [nsCSSLoader.cpp:1530]
>         CSSLoaderImpl::LoadSheet(SheetLoadData *,StyleSheetState)
> [nsCSSLoader.cpp:1367]

I'm guessing you have revision 3.184.  Why did NS_NewChannel fail?
It could have something to do with

WARNING: unknown protocol in nsScriptSecurityManager::CheckLoadURI, file
/builds/trunk/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1286

as well.  I should also be able to figure out why NS_NewChannel failed myself...
> WARNING: unknown protocol in nsScriptSecurityManager::CheckLoadURI, file
> /builds/trunk/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1286

If it's some random bogus protocol (eg <link rel="stylesheet" href="foo:bar">)
that could give this warning and cause the channel creation to fail.

Marking dependent on bug 84582, but we may need to put in an interim hack like
we did for bug 197015....
Depends on: 84582
Attached file complete stack of FMR
So, actually, I suspect that what's going wrong here is that a relative URL in
an HTML file forwarded in an email isn't working correctly, despite the presence
of a <BASE HREF="...">, and if we fix that, we might avoid the crash.  Or
something like that...
Are those URIs with a spec of "" I'm seeing in the log?

I assume there's some HTML to that effect or that some protocol handler impl is
royally screwing up?
Allow me to correct that to "some protocol handler impl IS royally screwing up"
(quite apart from whether the HTML has "" as URIs).
note that this happens in both imap and local mailboxes - is it mime that's
messing up, generating incorrect urls?
I think part of the problem is that in HTMLContentSink::ProcessBaseHref , mBody
is non-null, even though the document begins <BASE ...><html><head>....
This fixes the crash, but there are at least two other underlying problems here
(see comment 7 and comment 14).
This one is more consistent about where to add the new function calls.
Attachment #146637 - Attachment is obsolete: true
Attachment #146638 - Flags: superreview?(jst)
Attachment #146638 - Flags: review?(jst)
(Actually, comment 14 isn't a bug; it's the result of the way mail displays HTML
inline.)
Comment on attachment 146638 [details] [diff] [review]
patch to set _baseHref on LINK, META, STYLE, and SCRIPT

r+sr=jst
Attachment #146638 - Flags: superreview?(jst)
Attachment #146638 - Flags: superreview+
Attachment #146638 - Flags: review?(jst)
Attachment #146638 - Flags: review+
Assignee: mscott → dbaron
Severity: normal → critical
Status: ASSIGNED → NEW
OS: Windows 2000 → All
Priority: -- → P1
Hardware: PC → All
Whiteboard: [patch]
Comment on attachment 146638 [details] [diff] [review]
patch to set _baseHref on LINK, META, STYLE, and SCRIPT

This is a long-standing bug that can cause some rather serious problems with
pages with multiple BASE elements or with HTML attached to email.
Attachment #146638 - Flags: approval1.7?
fix checked into m4 branch - this fixes the problem on that branch.
Attachment #146638 - Flags: approval1.7? → approval1.7+
Fun stuff.  So remaining issues are, as I see them:

1)  What produced bogus URIs instead of throwing an exception on newURI()?  This
    should be fixed.
2)  Why are we actually crashing?  That is, could we reproduce this outside
    mailnews by using URIs that are valid uris with unknown schemes but fail to
    open the channel?

We can probably spin off separate bugs for those if people want....
Fix checked in to trunk, 2004-04-20 16:41:56 -0700.
Fix checked in to MOZILLA_1_7_BRANCH, 2004-04-20 19:40:15 -0700.
Status: NEW → RESOLVED
Closed: 20 years ago
Keywords: fixed1.7
Resolution: --- → FIXED
Keywords: crash
we have a similar e-mail that still crashes, so we'll either need to spin off a
new bug, or re-open this one...I've sent it to dbaron. In that message, the
baseHREF is empty, which might explain why the attached fix didn't fix this
problem...
Separate bug.  I filed bug 241254 (and attached a simple testcase).
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: