Closed Bug 299036 Opened 17 years ago Closed 17 years ago

Crash in [@ CNavDTD::DidHandleStartTag] while handling misplaced content

Categories

(Core :: DOM: HTML Parser, defect)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.8beta3

People

(Reporter: mrbkap, Assigned: mrbkap)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(2 files, 1 obsolete file)

This is a mangleme crasher. The problem is that in CNavDTD::DidHandleStartTag
relies (in a couple of cases) on the next token (that's about to be handled)
being in the tokenizer. However, when we're handling misplaced content (such as
<table><textarea>), this is not the case.

This turns out to be a decently difficult problem to solve with a general
solution because CNavDTD::HandleSavedTokens keeps its own count of how many
tokens it has to process, and updating that count outside of
::HandleSavedTokens() is hard without adding a member specifically for this case
(which I don't really want to do).

My patch will fix the crash, but leave the other problem (which is with <pre>
and <xmp> in misplaced content) alone for a better solution.
Attached file testcase
Status: NEW → ASSIGNED
Keywords: testcase
Attached patch patch v1 (obsolete) — Splinter Review
This fix simply moves the newline stripping code yet another level back, into
the tokenizer. This has the advantage that we no longer need to worry about
stripping newlines (from <textarea>) in the DTD, so it doesn't ever have to
worry about where it's getting parsed (either directly from HandleToken or
through HandleSavedTokens). This is a *lot* cleaner (and more robust!) than any
fix in the DTD would have been.
Attachment #187531 - Flags: superreview?(dbaron)
Attachment #187531 - Flags: review?(jst)
I don't know if it's too late, but I'd like to get this HTML parser crash fix
into 1.8b3.
Flags: blocking1.8b3?
Comment on attachment 187531 [details] [diff] [review]
patch v1

r+sr=jst
Attachment #187531 - Flags: superreview?(dbaron)
Attachment #187531 - Flags: superreview+
Attachment #187531 - Flags: review?(jst)
Attachment #187531 - Flags: review+
Comment on attachment 187531 [details] [diff] [review]
patch v1

This is a safe fix that stops a crash in the HTML parser.
Attachment #187531 - Flags: approval1.8b3?
Comment on attachment 187531 [details] [diff] [review]
patch v1

a=chofmann
Attachment #187531 - Flags: approval1.8b3? → approval1.8b3+
Attached patch patch v2Splinter Review
This is the patch that I will check in in a bit (it makes \n\r = 2 newlines
instead of 1 and ensures that we trim trailing newlines even if there isn't a
</textarea>).
Attachment #187531 - Attachment is obsolete: true
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta3
Flags: blocking1.8b3?
parser/htmlparser/tests/crashtests/299036-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3
Flags: in-testsuite+
Crash Signature: [@ CNavDTD::DidHandleStartTag]
You need to log in before you can comment on or make changes to this bug.