324601 - [FIX]Documents created via DOMImplementation should have better principal

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Right now they get a principal based on the URI of the document the DOMImplementation came from.  They should probably just get the principal that document has at createDocument call time.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note to self: should store principal, not URI, in global window too (the mOpenerScriptURL stuff).

Comment 2

[Boris Zbarsky [:bzbarsky]]

Note to self -- fix NS_NewDOMDocument to take a principal or something.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Allan, does XForms actually depend on the fact that the document created via DOMImplementation has the baseURI and documentURI set to the documentURI of the document the DOMImplementation came from?

I'm thinking that we should change that, basically (and use about:blank for the URI instead).

Comment 4

[Boris Zbarsky [:bzbarsky]]

I filed bug 325816 on the mOpenerScriptURL, since that depends on bug 323810 unless I want icky conflicts.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

This changes the URI on blank documents to be about:blank and the principal to be the one of the document the DOMImplementation came from...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Actually compiling

Comment 7

[Allan Beaufour]

(In reply to comment #3)
> Allan, does XForms actually depend on the fact that the document created via
> DOMImplementation has the baseURI and documentURI set to the documentURI of the
> document the DOMImplementation came from?

No, we should not need that.

Comment 8

[Allan Beaufour]

(In reply to comment #6)
> Created an attachment (id=210676) [edit]
> Actually compiling

And actually fixing the problem for XForms :)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix buglet timeless found

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix that buglet for real

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 211430 [details] [diff] [review]
Fix that buglet for real

actually, i'm not sure if this is good. Shouldn't the principal be set along with the baseuri of the XMLHttpRequest.

If we can't get to the principal but we can get to the baseuri maybe we should create a new principal based on the baseuri assuming the baseuri is actually a DocumentURI() or similar.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Though really, this seems like a case for a non-principal. For now maybe use an about:blank principal since it should be reset once loading starts, right?

Comment 13

[Boris Zbarsky [:bzbarsky]]

> Shouldn't the principal be set along with the baseuri of the XMLHttpRequest.

Please read nsXMLHttpRequest::GetBaseURI?

> Though really, this seems like a case for a non-principal.

Possibly, but I'd like to fix this bug, since it's actually breaking functionality... I don't know when, or whether, I'll have time to work on non-principal, but I can file a followup bug on it.

Frankly, using the about:blank principal would just be more code than this, for little purpose.  ;)

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 211430 [details] [diff] [review]
Fix that buglet for real

>Index: extensions/xmlextras/base/src/nsXMLHttpRequest.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp,v
>retrieving revision 1.143
>diff -u -p -d -r1.143 nsXMLHttpRequest.cpp
>--- extensions/xmlextras/base/src/nsXMLHttpRequest.cpp	20 Jan 2006 21:33:24 -0000	1.143
>+++ extensions/xmlextras/base/src/nsXMLHttpRequest.cpp	10 Feb 2006 22:27:23 -0000
>@@ -1233,7 +1233,15 @@ nsXMLHttpRequest::OnStartRequest(nsIRequ
>   nsCOMPtr<nsIPrivateDOMImplementation> privImpl =
>     do_QueryInterface(implementation);
>   if (privImpl) {
>-    privImpl->Init(GetBaseURI());
>+    // XXXbz this is probably all wrong when not called from JS... and possibly
>+    // even then! Fixing that requires giving XMLHttpRequest some principals
>+    // when inited.  Until then, cases when we don't actually parse the
>+    // document will give our mDocument he wrong principal.  I'm just not sure
>+    // how wrong it can get...  Shouldn't be too bad as long as mScriptContext
>+    // is sane, I guess.
>+    nsCOMPtr<nsIDocument> doc = GetDocumentFromScriptContext(mScriptContext);
>+    nsIURI* uri = GetBaseURI();
>+    privImpl->Init(uri, uri, doc->GetNodePrincipal());
>   }

Should you call GetBaseURI() before using mScriptContext to make sure that it is set? Also, you should probably handle doc being null.

Comment 15

[Boris Zbarsky [:bzbarsky]]

> Should you call GetBaseURI() before using mScriptContext to make sure that it
> is set?

"Not sure".  I really don't trust this "set mScriptContext lazily whenever we happen to" business, hence the comments.  I'd prefer to fail over with a null principal if it's not already set when we get here.

> Also, you should probably handle doc being null.

That I do.  Will fix.

Comment 16

[Johnny Stenback (:jst)]

Comment on attachment 211430 [details] [diff] [review]
Fix that buglet for real

r+sr=jst

Comment 17

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.  I addressed the comment about doc being null from comment 14 in a separate checkin.  :(