| Summary: | Crash in KHTML on www.tweakers.net | ||
|---|---|---|---|
| Product: | [Applications] konqueror | Reporter: | Jonathan Brugge <jonathan_brugge> |
| Component: | khtml | Assignee: | Konqueror Developers <konq-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | kollix, odborg, spam |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
how IE and mozilla handle the invalid HTML
patch that fixes the problem |
||
|
Description
Jonathan Brugge
2004-01-21 09:30:17 UTC
bug 73052 same page, other backtrace... tons of others are resolved (?) I can confirm this, Konqueror did not crash very often on tweakers.net when I used Beta 2, but in the two days I have RC1 I crashed konqueror six times already. *** Bug 73052 has been marked as a duplicate of this bug. *** I could reproduce yesterday, but not today. I don't think my khtml has changed in the mean time though. Subject: Re: Crash in KHTML on www.tweakers.net > I could reproduce yesterday, but not today. I don't think my khtml has > changed in the mean time though. I have posted the KDebug info of a short Konqueror session here (duplicate bug, sorry about that btw): http://bugs.kde.org/show_bug.cgi?id=73052 Maybe this is useful? best, Jeroen Can now reproduce with MSIE useragent for adserver.webads.nl kio_httprc: [webads.nl] UserAgent=Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0) No reproduction possible currently, because WebAds and Tweakers.net decided not to show the offending banner to Konqueror/Safari...to test it, it should be possible to change your useragent string to Mozilla/IE. Bugzilla just tells me that Waldo said the same a minute ago, but now you know the reason. By the way: Waldo, welcome at GoT, the forum of Tweakers.net :). (http://gathering.tweakers.net/forum/list_message/19791809#19791809) Testcase: <div id='a'></div> <div id='b'> <center><a href=\"http://www.kde.org/\"> </div> <script language='javascript'> var html = document.getElementById('b').innerHTML; document.getElementById('a').innerHTML = html; document.getElementById('b').innerHTML=""; </script> Valgrind: ==13185== Invalid read of size 4 ==13185== at 0x40360D96: khtml::TreeShared<DOM::NodeImpl>::ref() (shared.h:34) ==13185== by 0x403C9A88: khtml::KHTMLParser::freeBlock() (htmlparser.cpp:1238) ==13185== by 0x403C7357: khtml::KHTMLParser::~KHTMLParser() (htmlparser.cpp:158) ==13185== by 0x403CFDE8: khtml::HTMLTokenizer::~HTMLTokenizer() (htmltokenizer.cpp:1596) ==13185== by 0x403A8223: DOM::DocumentImpl::close() (khtmlview.h:110) ==13185== by 0x403D831B: DOM::HTMLDocumentImpl::close() (html_documentimpl.cpp:293) ==13185== by 0x4036EA82: KHTMLPart::checkEmitLoadEvent() (khtml_part.cpp:2025) ==13185== by 0x4036DB69: KHTMLPart::slotFinishedParsing() (khtml_part.cpp:1792) ==13185== Address 0x4692A788 is 4 bytes inside a block of size 60 free'd ==13185== at 0x4002A0B3: __builtin_delete (vg_replace_malloc.c:244) ==13185== by 0x4002A0D1: operator delete(void*) (vg_replace_malloc.c:253) ==13185== by 0x403D5A1F: DOM::HTMLGenericElementImpl::~HTMLGenericElementImpl() (html_elementimpl.cpp:631) ==13185== by 0x403B30D0: DOM::NodeBaseImpl::removeChildren() (dom_nodeimpl.cpp:1286) ==13185== by 0x403D56AE: DOM::HTMLElementImpl::setInnerHTML(DOM::DOMString const&) (html_elementimpl.cpp:547) ==13185== by 0x404FB0AE: DOM::HTMLElement::setInnerHTML(DOM::DOMString const&) (html_element.cpp:144) ==13185== by 0x4049EAC9: KJS::HTMLElement::putValueProperty(KJS::ExecState*, int, KJS::Value const&, int) (kjs_html.cpp:3030) ==13185== by 0x404A9CED: void KJS::DOMObjectLookupPut<KJS::HTMLElement, KJS::DOMElement>(KJS::ExecState*, KJS::Identifier const&, KJS::Value const ==13185== by 0x4049DEDA: KJS::HTMLElement::tryPut(KJS::ExecState*, KJS::Identifier const&, KJS::Value const&, int) (kjs_html.cpp:2364) ==13185== by 0x40475BD0: KJS::DOMObject::put(KJS::ExecState*, KJS::Identifier const&, KJS::Value const&, int) (kjs_binding.cpp:70) ==13185== by 0x4063C6C6: KJS::Reference::putValue(KJS::ExecState*, KJS::Value const&) (reference.cpp:165) ==13185== by 0x4060929F: KJS::AssignNode::evaluate(KJS::ExecState*) const (nodes.cpp:1574) ==13185== by 0x4060AC8A: KJS::ExprStatementNode::execute(KJS::ExecState*) (nodes.cpp:1916) ==13185== by 0x40610DBA: KJS::SourceElementsNode::execute(KJS::ExecState*) (nodes.cpp:3035) ==13185== by 0x4060AAB2: KJS::BlockNode::execute(KJS::ExecState*) (nodes.cpp:1878) ==13185== by 0x406102BB: KJS::FunctionBodyNode::execute(KJS::ExecState*) (nodes.cpp:2881) ==13185== by 0x40602484: KJS::InterpreterImp::evaluate(KJS::UString const&, KJS::Value const&) (internal.cpp:873) ==13185== by 0x40637238: KJS::Interpreter::evaluate(KJS::UString const&, KJS::Value const&) (interpreter.cpp:166) ==13185== by 0x404C32AC: KJSProxyImpl::evaluate(QString, int, QString const&, DOM::Node const&, KJS::Completion*) (kjs_proxy.cpp:148) ==13185== by 0x4036940A: KHTMLPart::executeScript(QString const&, int, DOM::Node const&, QString const&) (khtml_part.cpp:965) ==13185== by 0x403CB9E8: khtml::HTMLTokenizer::scriptExecution(QString const&, QString const&, int) (khtmlview.h:110) ==13185== by 0x403CB5D6: khtml::HTMLTokenizer::scriptHandler() (htmltokenizer.cpp:406) ==13185== by 0x403CB3A3: khtml::HTMLTokenizer::parseSpecial(khtml::DOMStringIt&) (htmltokenizer.cpp:324) ==13185== by 0x403CE2A2: khtml::HTMLTokenizer::parseTag(khtml::DOMStringIt&) (htmltokenizer.cpp:1138) ==13185== by 0x403CEE36: khtml::HTMLTokenizer::write(QString const&, bool) (htmltokenizer.cpp:1272) ==13185== by 0x4036D6DA: KHTMLPart::write(char const*, int) (khtml_part.cpp:1719) ==13185== by 0x4036AC86: KHTMLPart::slotData(KIO::Job*, QMemArray<char> const&) (khtml_part.cpp:1407) JFYI: KDE 3.1 crashes on the same test case, so I guess it's no recent regression but a change in tweakers.net the test case is really mean. konqueror leaves the <a> open when the </div> closes, so it adds the <script> to the <center><a>, which then alters the surrounding <div>, so when the parser finishes, it finds the currently parsed elements changed. This is of course no problem if the <a> tag is closed correctly (or the <center> tag - which closes the <a> tag too). The fix here would propbably to close <a> on </div>, but that might break other pages. Created attachment 4289 [details]
how IE and mozilla handle the invalid HTML
Interesting: only konqueror keeps the <div> open, while mozilla closes the
<center> but leaves the <a> open, IE leaves both open, but still closes the
<div>
I tried with safari and it a) displays the invalid HTML as konqueror does and b) it crashes too [safari insisted I report a bug to apple - which I did with a reference to this bug ;] Re: #11 I think the parser should refcount the elements it has open so that they can't get deleted from underneath it. See patch. Created attachment 4291 [details]
patch that fixes the problem
patch that fixes the problem
Subject: Re: Crash in KHTML on www.tweakers.net JFYI, this www.tweakers.net is apparently fixed (didn't crash today, without the patch that is). I'm using a patched khtml now, I will let you know if I find anything peculiar. So far, so good. best, Jeroen Subject: khtmltests/regression CVS commit by coolo: do never crash again CCMAIL: 73112-done@bugs.kde.org A baseline/unsorted/73112.html-dom 1.1 A tests/unsorted/73112.html 1.1 M +1 -0 baseline/unsorted/.cvsignore 1.2 --- khtmltests/regression/baseline/unsorted/.cvsignore #1.1:1.2 @@ -2,2 +2,3 @@ 70488.html-dom 70608.html-dom +73112.html-render Thanks anyone for the invested time - it's really great to see (again) how fast and precise the developers react to bug reports like this one! *** Bug 70919 has been marked as a duplicate of this bug. *** *** Bug 75274 has been marked as a duplicate of this bug. *** |