DOMParser produces documents with about:blank uri

RESOLVED FIXED

Status

()

RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: janv, Assigned: janv)

Tracking

({fixed1.4.1})

Trunk
x86
All
fixed1.4.1
Points:
---
Bug Flags:
blocking1.4.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

15 years ago
I use XSLT from JS. My JS looks like this:
...
var parser = new DOMParser();
var dataDoc = parser.parseFromString(string, "text/xml");
...
var fragment = processor.transformToFragment(dataDoc, outDoc);

Everything worked fine until I started using the document() function in XSLT.
That funtion doesn't work at all in this case.
I'm convinced that this function fails in security checks, because dataDoc has
about:blank uri.
After some debugging I found out that there are actually 2 problems.

1. A problem in nsSyncLoadService on line 323
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsSyncLoadService.cpp#323
We should call GetOriginalURI() instead of GetURI, since some uris can be
transformed. For example chrome://... to jar://...

2. A problem in nsDOMParser on line 467
http://lxr.mozilla.org/seamonkey/source/extensions/xmlextras/base/src/nsDOMParser.cpp#467
principla simply doesn't QI to nsICodebasePrincipal.
I found out an alternative way of obtaining baseURI (inspired by nsXMLHttpRequest)

I have a patch that solves these problems for me, but there can be a better way
to fix it.
(Assignee)

Comment 1

15 years ago
Created attachment 125306 [details] [diff] [review]
possible fix
The DOMParser change is actually what I intended to do a while back, but forgot.
Thanks for doing this! There is one thing that you need to change, though:
JS_GetContextPrivate() could return nsnull, so before using |scriptContext|
check that it exists.
(Assignee)

Comment 3

15 years ago
cool, I just wanted to cc you on the bug
thanks for your comment, I'll add the null check and attach a new patch
Status: NEW → ASSIGNED
(Assignee)

Comment 4

15 years ago
Created attachment 125324 [details] [diff] [review]
fix
Attachment #125306 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #125324 - Flags: superreview?(peterv)
Attachment #125324 - Flags: review?(heikki)
Attachment #125324 - Flags: review?(heikki) → review+
Comment on attachment 125324 [details] [diff] [review]
fix

>Index: extensions/xmlextras/base/src/nsDOMParser.cpp
>===================================================================

>+
>+    nsCOMPtr<nsIScriptContext> scriptContext =
>+      NS_STATIC_CAST(nsIScriptContext*, JS_GetContextPrivate(cx));

This is wrong, though it'll work most of the time. You'll need to do it like
nsJSUtils::GetDynamicScriptContext does it. I think jst is going to write a
helper function for this in the near future.
Attachment #125324 - Flags: superreview?(peterv) → superreview+
Comment on attachment 125324 [details] [diff] [review]
fix

Oh, in that case, please also fix XMLHttpRequest. In fact, check all callers:
http://lxr.mozilla.org/seamonkey/ident?i=JS_GetContextPrivate
Let's change that in a different bug when we have an inline helper function in a
header somewhere. No need to whack the tree twice.
(Assignee)

Comment 8

15 years ago
Created attachment 125345 [details] [diff] [review]
revised patch
Attachment #125324 - Attachment is obsolete: true
(Assignee)

Comment 9

15 years ago
Created attachment 125346 [details] [diff] [review]
final patch

no need for the if (supports)
(Assignee)

Updated

15 years ago
Attachment #125345 - Attachment is obsolete: true
(Assignee)

Comment 10

15 years ago
Comment on attachment 125346 [details] [diff] [review]
final patch

carrying over r=heikki and sr=peterv
Attachment #125346 - Flags: superreview+
Attachment #125346 - Flags: review+
(Assignee)

Comment 11

15 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Assignee)

Updated

15 years ago
Attachment #125346 - Flags: approval1.4?

Comment 12

15 years ago
Comment on attachment 125346 [details] [diff] [review]
final patch

moving approval request forward.
Attachment #125346 - Flags: approval1.4? → approval1.4.x?

Comment 13

15 years ago
Comment on attachment 125346 [details] [diff] [review]
final patch

a=mkaply
Attachment #125346 - Flags: approval1.4.x? → approval1.4.x+

Comment 14

15 years ago
Please add fixed1.4.1 keyword when checked in
Flags: blocking1.4.x+
(Assignee)

Comment 15

15 years ago
checked into the 1.4 branch
Keywords: fixed1.4.1

Updated

15 years ago
Blocks: 224532

Comment 16

14 years ago
I'd like to reopen this bug. This is the output I get on Mozilla 1.7.5 / FF 1.0:

var fooParser = new DOMParser();
var fooDoc = fooParser.parseFromString('<foofy/>','text/xml');

'fooDoc' will now have 'about:blank' as its baseURI (i.e. 'fooDoc.baseURI' ->
'about:blank').

This causes security issues when creating content from Strings and then trying
to copy it around to other documents, etc.

Cheers,

- Bill

Comment 18

14 years ago
I have nailed this bug down further. It only happens when evaling code that
performs this parsing when that code has been typed into an iframe in design
mode. The iframe has an event handler that causes the 'contents' of the iframe
to be evaled (i.e. I'm using the iframe as a 'code evaluator').

That iframe's baseURI has an 'about:blank' URI, because it was created with a
src="" attribute value.

I can post a sample snippet here if it will help. This is admittedly a pretty
weird case, but it might point to an incorrect computation of the 'script
context' (as I read the code in nsDOMParser.cpp around line 500).

Let me know if you all need a code snippet.

Thanks!!

Cheers,

- Bill
(In reply to comment #18)
> I can post a sample snippet here if it will help. This is admittedly a pretty

A working testcase is always appreciated.

Comment 20

14 years ago
Created attachment 171300 [details]
Attachment showing that DOMParser still produces 'about:blank' baseURIs

Test case as requested.

Please follow the instructions in the web page on how to exercise the bug.

There is a bit of discussion at the bottom as well.

Cheers,

- Bill

Comment 21

14 years ago
I'd like to have this reopened this as well.  It clear still exists when 
DOMParser is used by a XUL document which is running inside an iframe.

Ed

(In reply to comment #16)
> I'd like to reopen this bug. This is the output I get on Mozilla 1.7.5 / FF 
1.0:

You need to log in before you can comment on or make changes to this bug.