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

RESOLVED FIXED in mozilla1.8beta3

Status

()

defect
--
critical
RESOLVED FIXED
14 years ago
10 years ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

({crash, testcase})

Trunk
mozilla1.8beta3
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

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.
Status: NEW → ASSIGNED
Keywords: testcase
Posted 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 6

14 years ago
Comment on attachment 187531 [details] [diff] [review]
patch v1

a=chofmann
Attachment #187531 - Flags: approval1.8b3? → approval1.8b3+
Posted 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: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta3
Flags: blocking1.8b3?

Comment 9

10 years ago
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.