Open Bug 377092 Opened 13 years ago Updated 6 months ago

Expose a way to set the originating principal of a javascript: URI on that URI

Categories

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

x86
macOS
defect
Not set

Tracking

()

mozilla1.9alpha8

People

(Reporter: bzbarsky, Assigned: pavlov)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Needed to fix javascript:-uri images in the short term.  Doing this should let pnglets work, perhaps.
Idea: do this by flagging the source principal of the javascript: URI on the URI (e.g. in nsContentUtils::LoadImage) and the channel using that if it has no owner itself.  That doesn't have the problems that flagging the execution policy on the URI does, and the source principal doesn't depend on whether the URI gets cloned, etc.
Er, probably won't fix pnglets.  But would fix anything that generates the image on the fly without reference to the web page, since we'd run the javascript: in a sandbox.
Flags: blocking1.9?
Note: when we fix this, retest bug 363597 testcase 1 to make sure all is OK.
Blocks: 363597
Need bug 310165 to be fixed first so we know what the loading principal is...
Depends on: 310165
I'm a little confused about what this bug is about. What does "on that URI" refer to in the summary?

Doesn't pinglets that doesn't reference the containing page already work? I.e. don't we execute javascript uris for images in a sandbox?

Either way, this doesn't really seem like a blocker, since however cool pinglets are, i've never heard of them being used on actual sites other than as demos.

Might be wanted-1.9, but I'd like to better understand what the bug is about.

Flags: blocking1.9? → blocking1.9-
> What does "on that URI" refer to in the summary?

Have a way to indicate on a particular nsIURI object for a javascript: URI what principal the creator of the URI has.

> Doesn't pinglets that doesn't reference the containing page already work?

Right now, no.  We don't execute javascript: URIs at all if we don't know the origin principal, because we need that principal to do the "is javascript enabled?" check.  I don't think we want to change that to assuming script is enabled.

Since we currently only pass principals along with channels, and imagelib doesn't expose channels, that means no javascript: in any image loads right now.
Attached patch Fix, plus testsSplinter Review
biesi, would you look over the URI code?  jst, could you review the rest?
Attachment #272130 - Flags: superreview?(jst)
Attachment #272130 - Flags: review?(jst)
Attachment #272130 - Flags: review?(cbiesinger)
Attachment #272130 - Flags: superreview?(jst)
Attachment #272130 - Flags: superreview+
Attachment #272130 - Flags: review?(jst)
Attachment #272130 - Flags: review+
Target Milestone: mozilla1.9alpha4 → mozilla1.9beta1
Comment on attachment 272130 [details] [diff] [review]
Fix, plus tests

+        nsCOMPtr<nsIScriptURI> scriptURI = do_QueryInterface(mURI);
+        if (scriptURI) {

won't you always have a scriptURI here? (i.e. shouldn't that if be an assertion?)

+    *aClassID = (nsCID*) nsMemory::Alloc(sizeof(nsCID));

NS_Alloc

--

how does this interact with image cache? that is... could one site get an image added to the cache with its principal, and the other site could use the same URL (but a different principal) and access that?
or does the error from Equals deal with that?
Comment on attachment 272130 [details] [diff] [review]
Fix, plus tests

is there a particular reason why you pass on some setters to the inner URI? they fail anyway, but it might be clearer to make them return an error too?
> won't you always have a scriptURI here? 

Probably, yeah.  I suppose I could make this assert.

> NS_Alloc

Sure.

> or does the error from Equals deal with that?

Oops.  I need to actually implement Equals (and have it compare principals too).  Good catch!  I was going to do that, and forgot...

I would hope that even if the image cache caches this (which I seem to recall it didn't) that would help.
Comment on attachment 272130 [details] [diff] [review]
Fix, plus tests

ok, afaict imagelib does cache this
Attachment #272130 - Flags: review?(cbiesinger) → review-
biesi points out that there's really nothing I can do on the javascript: end to prevent imagelib from caching javascript: images across different documents.  Unless I make javascript: implement nsICachingChannel, which would screw up other consumers.

Over to pav to get us some sort of imagelib arch where security stuff can possibly work.
Assignee: bzbarsky → pavlov
Note in particular that imagelib does string compares instead of using nsIURI::Equals for cache purposes.  Which is really rather broken.
Blocks: 344890
QA Contact: ian → general
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.