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

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

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.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: testcase

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

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.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

I don't know if it's too late, but I'd like to get this HTML parser crash fix
into 1.8b3.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 187531 [details] [diff] [review]
patch v1

r+sr=jst

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 187531 [details] [diff] [review]
patch v1

This is a safe fix that stops a crash in the HTML parser.

Comment 6

[chris hofmann]

Comment on attachment 187531 [details] [diff] [review]
patch v1

a=chofmann

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v2

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>).

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in.

Comment 9

[Bob Clary [:bc] (inactive)]

parser/htmlparser/tests/crashtests/299036-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3