1129707 - Investigate openURL() callers and potentially add a principal argument

Status: NEW

(Core :: DOM: Security, defect, P3)

Description

[Christoph Kerschbaumer [:ckerschb}]

In Bug 1087744 [1] we where discussing if we need to extend the argument list of openURL() [2] to potentially pass a loadingPrincipal.

We are going to land Bug 1087744 using the SystemPrincipal as the loadingPrincipal and use this bug to investigate callsites to potentially extend the argument list.

Note: Since we haven't been doing any security checks as of now, it's not super critical and we (Jonas, Tanvi, and myself) think we can use the SystemPrincipal as an interim solution.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1087744#c28
[2] http://mxr.mozilla.org/mozilla-central/source/toolkit/content/contentAreaUtils.js#1102

Comment 1

[Christoph Kerschbaumer [:ckerschb}]

Marking this bug blocking Bug 1087744 so we don't loose the reference.

Comment 2

[Christoph Kerschbaumer [:ckerschb}]

It's hard to find callsites of OpenURL(), I couldn't find one locally by running mochitests and also xpcshell-tests. I think the only way to figure out is by pushing to try:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6161a157d7b

Comment 3

[Christoph Kerschbaumer [:ckerschb}]

Attachment: bug_1129707_extend_openurl_by_a_loadingprincipal.patch

It's really hard to tell which calls of openURL() [1] actually dispatch to the one defined in toolkit/content/contentAreaUtils.js. I cover at least those we have testcases for on TBPL. Not sure if we have coverage for all of those callsites though.

Jonas, Tanvi, at least we have a starting point to investigate. What do you think? Should we just ask a toolkit peer how we can query a principal, and which one we should use in general? At the moment, I simply don't know. Anyway, I rather lean towards extending openURL() by a second argument, in case some code wants to make use of that function in the future. As of now, we don't do any security checks at all when openURL() is called.

[1] http://mxr.mozilla.org/mozilla-central/search?string=openURL%28

Comment 4

[Christoph Kerschbaumer [:ckerschb}]

Benjamin, can you tell us what openURL() does exactly? Doesn openURL() load a particular url and then return the result, or does it rather navigate to a tab given a url? We are wondering if we need to pass a loadingPrincipal (or maybe a triggeringPrincipal in case it is a new toplevel load).

Comment 5

[Benjamin Smedberg]

No, but I'm going to ask Jared who probably knows more.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Most of today's openURL was implemented by http://hg.mozilla.org/mozilla-central/rev/d537a51e968e (bug 349309).

It looks like there's three cases that are handled by the function:
1) If the protocol is not handled by the browser, attempt to open an external application for the URL.
2) If the current application is a browser and there is a recent window available, open the URL in a new tab. This is pretty straightforward, and is a pretty common usage in front-end code to open a user-provided URL. The only common way that I know of to get Firefox to not have a recent window available is on OSX where a user can close all of the browser windows but keep Firefox running and it's menubar present (this uses the "hidden window").
3) When there isn't a recent window available, it looks like a bunch of work is done to load the URL. I'm not sure how this part works but I suspect that bug 349309 can provide more context here.

Comment 7

[Tanvi Vyas[:tanvi]]

Clearing my needinfo.  This is on Christoph's radar.

Comment 8

[Christoph Kerschbaumer [:ckerschb}]

I don't have time to fix that right now - let's put it in the backlog for now.