419106 - nsSchemaLoader::LoadAsync fails

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

As per comments in bug 404818 and bug 392322, XMLHttpRequest now requires being init'd with the system principal when called from c++.  We'll need to update nsSchemaLoader in extensions/schema-validation to take this into account.

Comment 1

[aaronr]

Attachment: patch

Comment 2

[Neil Deakin]

Comment on attachment 305080 [details] [diff] [review]
patch

Don't you want to get the subject principal here, rather than the system principal? The former is the for the current script, while the latter has chrome privileges.

Comment 3

[aaronr]

(In reply to comment #2)
> (From update of attachment 305080 [details] [diff] [review])
> Don't you want to get the subject principal here, rather than the system
> principal? The former is the for the current script, while the latter has
> chrome privileges.
> 

nsSchemaLoader::LoadAsync can be called from either JS or from c++, so we may or may not have a subject principal.  Right?  In the given testcase we are calling it from c++, but loadAsync lives in the .idl and is scriptable.  But I'm not a security type of guy so I'll do whatever you guys suggest.

Comment 4

[Doron Rosenberg (IBM)]

well, nsSchemaLoader is not accessible to web content, but we should follow Neil's remark - best to check for an subject principal.

Comment 5

[aaronr]

Attachment: patch2

changed patch to check for a subject principal first and if not found, it will use the system principal

Comment 6

[Doron Rosenberg (IBM)]

Comment on attachment 305543 [details] [diff] [review]
patch2

looks good.  The only caller to schema loader (xforms) already does same origin checks, so this should be fine.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Would it be possible to add some initialization method to schemaloader and that
could take principal as a parameter? Then it would be up to the caller
to pass the right principal (in xforms case the principal of document).

Comment 8

[aaronr]

(In reply to comment #7)
> Would it be possible to add some initialization method to schemaloader and that
> could take principal as a parameter? Then it would be up to the caller
> to pass the right principal (in xforms case the principal of document).
> 

Assume that schema loader needs to be called from JS.  Can you get a principal inside JS to pass to this new schema loader initializer?  If not, then this new initializer isn't buying us much, is it?  I guess in that case it would be null and if it is null we can check for subject principal and if that isn't there than exit out.  Is that what you had in mind?

Comment 9

[Doron Rosenberg (IBM)]

He means to send the principal for the url trying to use the schema loader (say the xforms url).

Of course, this will mean no cross-domain loading of schema files...

Comment 10

[Doron Rosenberg (IBM)]

Uh, forgot to mention that per spec, you can include schemas from any location.  I could have sworn we did some cross-domain checks somewhere in xforms, but can't find them.

Comment 11

[aaronr]

Attachment: patch3

added loadAsyncWithPrincipal (noscript) which is called by xforms passing in the system principal and which now does all the heavy lifting.  loadAsync still can be called via JS, but we'll only try to use the subject principal when loadAsync is called.  loadAsync will now call loadAsyncWithPrincipal passing in the subject principal.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 305571 [details] [diff] [review]
patch3

>+#include "nsIPrincipal.idl"
I believe this isn't needed because you have:
>+interface nsIPrincipal;
So remove the include, if it works without.

>+    // no executable code will run and doron says that you really can't get to
"doron says" :)

So we handle schemas like images or .js (right?). That should be ok, IMO

Comment 13

[Doron Rosenberg (IBM)]

well, for xforms js can't get at the schema data, since that instance of the loader isn't accessible via xforms idl.

Comment 14

[aaronr]

Comment on attachment 305571 [details] [diff] [review]
patch3

I'll fix smaug's note at checkin.  You ok with it, Doron?

Comment 15

[aaronr]

Attachment: checked in patch

patch fixes olli's comment.

Comment 16

[aaronr]

Checked into trunk.  Trunk only patch, shouldn't go to the branch.