1324720 - registerProtocolHandler API checking improvement

Status: NEW

(Firefox :: File Handling, defect, P3)

Description

[Eden Chuang[:edenchuang]]

According to bug 1270416 comment 60, we want to make sure environment checking in registerProtocolHandler API

 Utils.checkProtocolHandlerAllowed(aProtocol, aURIString,
                                   haveWindow ? aBrowserOrWindow : null);

be reasonable and meaningful.

Currently, this method checks the handler and the registrar must be the same host.

Comment 1

[:Paolo Amadini]

(In reply to Eden Chuang[:edenchuang] from comment #0)
> According to bug 1270416 comment 60, we want to make sure environment
> checking in registerProtocolHandler API
> 
>  Utils.checkProtocolHandlerAllowed(aProtocol, aURIString,
>                                    haveWindow ? aBrowserOrWindow : null);
> 
> be reasonable and meaningful.
> 
> Currently, this method checks the handler and the registrar must be the same
> host.

There might a missing "not" here, as the method does not check whether the handler and the URI template are for the same origin. However, having different origins is indeed not allowed, so there must be a check somewhere else.

Gijs, I've read bug 1270416 comment 60 but I don't understand exactly what you meant in the review. Do you mean moving the origin checks to checkProtocolHandlerAllowed?

Comment 2

[:Gijs (he/him)]

(In reply to :Paolo Amadini from comment #1)
> (In reply to Eden Chuang[:edenchuang] from comment #0)
> > According to bug 1270416 comment 60, we want to make sure environment
> > checking in registerProtocolHandler API
> > 
> >  Utils.checkProtocolHandlerAllowed(aProtocol, aURIString,
> >                                    haveWindow ? aBrowserOrWindow : null);
> > 
> > be reasonable and meaningful.
> > 
> > Currently, this method checks the handler and the registrar must be the same
> > host.
> 
> There might a missing "not" here, as the method does not check whether the
> handler and the URI template are for the same origin. However, having
> different origins is indeed not allowed, so there must be a check somewhere
> else.
> 
> Gijs, I've read bug 1270416 comment 60 but I don't understand exactly what
> you meant in the review. Do you mean moving the origin checks to
> checkProtocolHandlerAllowed?

I think we should move the origin checks here, make sure they're actually correct (what happens if mail.foo.com sets document.domain to foo.com and then adds a handler for foo.com? What about the same thing minus the document.domain setting, what about the inverse (ie foo.com asking for mail.foo.com as a handler)?) and then invoke the checking method before prompting and before adding the handler.

comment #56 says the checking is currently done in prompt(), but I don't see any checks there, and the link to PermissionsUI.jsm is broken anyway because it wasn't a perma-link.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information