Closed Bug 699365 Opened 10 years ago Closed 9 years ago

"ASSERTION: Unsupported parser command!" with CSS filter

Categories

(Core :: DOM: HTML Parser, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11
Tracking Status
firefox10 --- unaffected

People

(Reporter: jruderman, Assigned: hsivonen)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, regression, testcase)

Attachments

(3 files, 1 obsolete file)

Attached file testcase
###!!! ASSERTION: Unsupported parser command!: '!nsCRT::strcmp(aCommand, "view")', file parser/html/nsHtml5Parser.cpp, line 720

This assertion is part of code added in:

changeset:   175a0afe3c43
user:        Henri Sivonen
date:        Fri Jul 30 13:15:38 2010 +0300
summary:     Bug 482921 part 1 - Implement HTML syntax highlighting using the new parser. r=Olli.Pettay.
Attached file stack trace
aCommand is the string "external-resource".
(In reply to Jesse Ruderman from comment #1)
> aCommand is the string "external-resource".

Interesting. I wasn't aware such a parser command existed! It has been unsupported with the new HTML parser all along.

Has "external-resource" ever been properly tested with the HTML mode of the old parser? Or is it an XML thing that's leaking to HTML parsing accidentally?

We seriously need to change parser commands into an enum to make it obvious what the full set of possible commands is.
Hiding this bug in case this reveals a security problem. I am not permitted to mark this a security bug, so I'm marking this as a different kind of confidential bug. Jesse, are you empowered to change this into a security bug.
Group: mozilla-corporation-confidential
Looks like external-resource isn't a true *parser* command, since the old parser never checks it. It's checked in
https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#2353
Group: mozilla-corporation-confidential → core-security
What does comment 4 mean: is it or not a security problem?
Keywords: regression
I see no evidence of this being a security bug. Opening this up. Henri, can you look into what we should do about this in the parser? Asserting isn't right, but I'm not sure what is...
Assignee: nobody → hsivonen
Group: core-security
So it looks like this parser command isn't really meant for the parser. It's meant as a magic value for nsDocument and the parser seems to be expected to fall back to a default.

Not quite nice to use a parser command like this, but it seems no actual harm is being done.
Attachment #576934 - Flags: review?(bugs)
Attachment #576934 - Attachment is obsolete: true
Attachment #576934 - Flags: review?(bugs)
Attachment #576935 - Flags: review?(bugs)
Attachment #576935 - Flags: review?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0354f6df10f7
Status: NEW → ASSIGNED
Flags: in-testsuite+
Target Milestone: --- → mozilla11
Backed out the test due to leak on Mac:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcba9e0fcf4f
Flags: in-testsuite+ → in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/0354f6df10f7
https://hg.mozilla.org/mozilla-central/rev/bcba9e0fcf4f

marking fixed, though still lacks the test.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 576935 [details] [diff] [review]
Check for extenal-resource in assertions, with hg add

(This comment has been posted on all the bugs mentioned in this comment, except bug 710142, so that the release drivers see it regardless of the order in which they process approval requests.)

The new View Source implementation landed before Firefox 10 moved to Aurora. Afterwards, a bunch of regressions were identified. Many of the regression fixes didn't land before Firefox 10 moved to Aurora but they have now been fixed on trunk except for bug 710142.

To avoid shipping with regressions, we either need to land all the regression fixes on Aurora for Firefox 10 (followed by a fix for bug 710142 in Beta if it doesn't make it before Dec 20th) or switch back to the old View Source implementation on Aurora. The new View Source implementation provides much better diagnostics for Web developers than the old View Source implementation.

So I'd like to ask the release drivers to either approve bug 535530, bug 699356, bug 699365, bug 700034, bug 700361, bug 703965, bug 704667 and bug 705473 plus bug 695640, which is a non-regression tweak, or to approve bug 710175 for reverting to the old View Source implementation for Firefox 10.
Attachment #576935 - Flags: approval-mozilla-aurora?
Comment on attachment 576935 [details] [diff] [review]
Check for extenal-resource in assertions, with hg add

[triage comment]
We decided to back out the new view source parser (bug 710175) rather than take this fixup for Firefox 10.

Denying for Aurora.
Attachment #576935 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Firefox 10 no longer affected due to bug 710175 landing.
Attachment #576935 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
You need to log in before you can comment on or make changes to this bug.