Closed Bug 420609 Opened 12 years ago Closed 12 years ago

XMLHttpRequest no longer able to parse XHTML documents with entity references

Categories

(Core :: XML, defect, P2)

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta5

People

(Reporter: bzbarsky, Assigned: gaubugzilla)

References

Details

(Keywords: regression, Whiteboard: regression from 418119)

Attachments

(2 files, 1 obsolete file)

See http://groups.google.com/group/mozilla.dev.apps.firefox/browse_frm/thread/edf78712691b8aa6/468737365d8b148b#468737365d8b148b

The regression range cited in that thread is http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-21+04&maxdate=2008-02-22+04&cvsroot=%2Fcvsroot

At a guess, this is a regression from bug 418119: When we try to load the XHTML DTD, the data document content policy blocks the load of the DTD.  Then we can't parse the XML, since the entities are undefined.
Flags: blocking1.9?
I'm not sure how common this is in the wild and in particular whether it should block the beta...
Attached file Exemplification page
Repost of a page affected by this, initially discussed on mozilla.dev.apps.firefox. Save locally as "test.html", as the XMLHttpRequest object relies on the name.

It can also be retrieved online:
http://xmlhttprequesttest.googlecode.com/svn/trunk/test.html
Yes, bug 418119 should be the reason. I see a request for file:///.../Mozilla%20Firefox/res/dtd/xhtml11.dtd via content policies. From what I can see, nsDataDocumentContentPolicy blocks it. If we want to add an exception for DTDs in nsDataDocumentContentPolicy we probably want to add a dedicated type for DTD requests (similar to bug 375314).
That might be the way to go, yes.
Attached patch Proposed patch (obsolete) — Splinter Review
Assignee: nobody → trev.moz
Status: NEW → ASSIGNED
Attachment #306986 - Flags: review?(bzbarsky)
Comment on attachment 306986 [details] [diff] [review]
Proposed patch

Looks good, but could you also assert that is($("content").textContent, reqest.responseXML.getElementById("content").textContent, "Entities didn't get expanded") ?
Attachment #306986 - Flags: review?(bzbarsky) → review+
Attached patch Patch v 1.1Splinter Review
Added additional check to the testcase - make sure the entities are actually expanded.
Attachment #306986 - Attachment is obsolete: true
Attachment #307190 - Flags: superreview?(bzbarsky)
Attachment #307190 - Flags: review+
Attachment #307190 - Flags: superreview?(bzbarsky) → superreview+
Attachment #307190 - Flags: approval1.9?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Checking in parser/htmlparser/src/nsExpatDriver.cpp;
/cvsroot/mozilla/parser/htmlparser/src/nsExpatDriver.cpp,v  <--  nsExpatDriver.cpp
new revision: 3.109; previous revision: 3.108
done
Checking in content/base/public/nsContentPolicyUtils.h;
/cvsroot/mozilla/content/base/public/nsContentPolicyUtils.h,v  <--  nsContentPolicyUtils.h
new revision: 3.17; previous revision: 3.16
done
Checking in content/base/public/nsIContentPolicy.idl;
/cvsroot/mozilla/content/base/public/nsIContentPolicy.idl,v  <--  nsIContentPolicy.idl
new revision: 3.10; previous revision: 3.9
done
Checking in content/base/src/nsDataDocumentContentPolicy.cpp;
/cvsroot/mozilla/content/base/src/nsDataDocumentContentPolicy.cpp,v  <--  nsDataDocumentContentPolicy.cpp
new revision: 1.5; previous revision: 1.4
done
Checking in content/base/test/Makefile.in;
/cvsroot/mozilla/content/base/test/Makefile.in,v  <--  Makefile.in
new revision: 1.58; previous revision: 1.57
done
RCS file: /cvsroot/mozilla/content/base/test/test_bug420609.xhtml,v
done
Checking in content/base/test/test_bug420609.xhtml;
/cvsroot/mozilla/content/base/test/test_bug420609.xhtml,v  <--  test_bug420609.xhtml
initial revision: 1.1
done
Checking in extensions/permissions/nsContentBlocker.cpp;
/cvsroot/mozilla/extensions/permissions/nsContentBlocker.cpp,v  <--  nsContentBlocker.cpp
new revision: 1.10; previous revision: 1.9
done
Checking in extensions/permissions/nsContentBlocker.h;
/cvsroot/mozilla/extensions/permissions/nsContentBlocker.h,v  <--  nsContentBlocker.h
new revision: 1.5; previous revision: 1.4
done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Target Milestone: mozilla1.9 → mozilla1.9beta5
If we take bug 418119 on branch, we should take this too.
Flags: blocking1.8.1.14?
I do have one question about XMLHttpRequest here.  The principal for XMLHttpRequest will be the principal of the page doing the request, not the principal of the document being requested.  Is that what we want in terms of the URI we pass to content policy?
Problem is that we probably don't want to change nsIContentPolicy on branch - meaning that we will need a different solution.

As to the URI passed to content policy - I think requestOrigin is indeed the URI that initiated the request (called XMLHttpRequest).
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.15?
Flags: blocking1.8.1.15+
Not needed if bug 418119 isn't taken.
Flags: blocking1.8.1.15+ → blocking1.8.1.15-
Whiteboard: regression from 418119
You need to log in before you can comment on or make changes to this bug.