Closed Bug 313278 Opened 15 years ago Closed 15 years ago

<script xmlns="http://www.w3.org/1999/xhtml"> leaks namespace to younger siblings

Categories

(Core :: XML, defect)

x86
All
defect
Not set
major

Tracking

()

VERIFIED FIXED

People

(Reporter: benjamin, Assigned: peterv)

References

Details

(Keywords: fixed1.8)

Attachments

(3 files)

See attached testcase, basically

<element xmlns="1">
  <script xmlns="xhtml" src="foo" />
  <foo />
</element>

Foo has the wrong namespace. Which ends up screwing some significant number of
published SVG examples and a lot of mixed XHTML+SVG content.
Attached file Testcase
This regressed between 2004-12-15-06 and 2004-12-16-08, so looks like we somehow
fail to pop off the default namespace after the expat landing...
Flags: blocking1.8rc1?
At a guess, we're adding the namespace binding, then bailing out when we block,
and never removing the binding...

Perhaps this is xmlparse.c line 2324?  We call the endElementHandler in line
2321, but we want to make it down to line 2341 before returning the error, no? 
Or do we not want to clear the tempPool here?  We definitely want that loop over
the bindings, I'd think.
Attached patch v1Splinter Review
It's going to be hard for me to test this soon, so if anyone could verify that
this fixes the problem I'd be grateful.
Attachment #200359 - Flags: superreview?(bzbarsky)
Attachment #200359 - Flags: review?(bzbarsky)
Comment on attachment 200359 [details] [diff] [review]
v1

Yeah, that fixes both testcases. r+sr=bzbarsky.

Can you check this in, or do you want me to?
Attachment #200359 - Flags: superreview?(bzbarsky)
Attachment #200359 - Flags: superreview+
Attachment #200359 - Flags: review?(bzbarsky)
Attachment #200359 - Flags: review+
please land and verify on the trunk. 
If you could, I'm off for the night.
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 200359 [details] [diff] [review]
v1

Requesting 1.8 branch approval.  This is a pretty serious regression in our XML
parsing.  The fix is pretty safe -- just move our early returns to be not quite
so early to allow expat to pop the namespaces associated with the just-closed
element.
Attachment #200359 - Flags: approval1.8rc1?
Verified fixed on trunk, we should definitely take this on branch.
Status: RESOLVED → VERIFIED
Attachment #200359 - Flags: approval1.8rc1? → approval1.8rc1+
Checked in on branch. Thanks bz for your help.
Keywords: fixed1.8
Flags: blocking1.8rc1?
You need to log in before you can comment on or make changes to this bug.