From 738962d0bff1f0faa101faa7b6775f841f4b46e6 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Wed, 16 Oct 2019 14:56:23 +0000 Subject: [PATCH] Bug 1586148 - fix web protocol handler behaviour under fission, r=mccr8 Differential Revision: https://phabricator.services.mozilla.com/D48238 --HG-- extra : moz-landing-system : lando --- dom/base/Navigator.cpp | 1 + toolkit/modules/E10SUtils.jsm | 79 +++++++++++++++++++ .../exthandler/tests/mochitest/browser.ini | 1 - 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/dom/base/Navigator.cpp b/dom/base/Navigator.cpp index 3737cd623d93..6023e5ba6ff4 100644 --- a/dom/base/Navigator.cpp +++ b/dom/base/Navigator.cpp @@ -833,6 +833,7 @@ void Navigator::RegisterContentHandler(const nsAString& aMIMEType, // This list should be kept up-to-date with the spec: // https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers +// If you change this list, please also update the copy in E10SUtils.jsm. static const char* const kSafeSchemes[] = { "bitcoin", "geo", "im", "irc", "ircs", "magnet", "mailto", "mms", "news", "nntp", "openpgp4fpr", "sip", "sms", "smsto", diff --git a/toolkit/modules/E10SUtils.jsm b/toolkit/modules/E10SUtils.jsm index 41ac4c0faf64..f8926a22f4ee 100644 --- a/toolkit/modules/E10SUtils.jsm +++ b/toolkit/modules/E10SUtils.jsm @@ -67,6 +67,12 @@ XPCOMUtils.defineLazyServiceGetter( "@mozilla.org/network/serialization-helper;1", "nsISerializationHelper" ); +XPCOMUtils.defineLazyServiceGetter( + this, + "extProtService", + "@mozilla.org/uriloader/external-protocol-service;1", + "nsIExternalProtocolService" +); function debug(msg) { Cu.reportError(new Error("E10SUtils: " + msg)); @@ -99,6 +105,46 @@ const PRIVILEGEDMOZILLA_REMOTE_TYPE = "privilegedmozilla"; const LARGE_ALLOCATION_REMOTE_TYPE = "webLargeAllocation"; const DEFAULT_REMOTE_TYPE = WEB_REMOTE_TYPE; +// This list is duplicated between Navigator.cpp and here because navigator +// is not accessible in this context. Please update both if the list changes. +const kSafeSchemes = [ + "bitcoin", + "geo", + "im", + "irc", + "ircs", + "magnet", + "mailto", + "mms", + "news", + "nntp", + "openpgp4fpr", + "sip", + "sms", + "smsto", + "ssh", + "tel", + "urn", + "webcal", + "wtai", + "xmpp", +]; + +// Note that even if the scheme fits the criteria for a web-handled scheme +// (ie it is compatible with the checks registerProtocolHandler uses), it may +// not be web-handled - it could still be handled via the OS by another app. +function hasPotentiallyWebHandledScheme({ scheme }) { + // Note that `scheme` comes from a URI object so is already lowercase. + if (kSafeSchemes.includes(scheme)) { + return true; + } + if (!scheme.startsWith("web+") || scheme.length < 5) { + return false; + } + // Check the rest of the scheme only consists of ascii a-z chars + return /^[a-z]+$/.test(scheme.substr("web+".length)); +} + function validatedWebRemoteType( aPreferredRemoteType, aTargetUri, @@ -120,6 +166,39 @@ function validatedWebRemoteType( return PRIVILEGEDMOZILLA_REMOTE_TYPE; } + // If we're in the parent and we were passed a web-handled scheme, + // transform it now to avoid trying to load it in the wrong process. + if (aRemoteSubframes && hasPotentiallyWebHandledScheme(aTargetUri)) { + if ( + Services.appinfo.processType != Services.appinfo.PROCESS_TYPE_DEFAULT && + Services.appinfo.remoteType.startsWith(FISSION_WEB_REMOTE_TYPE_PREFIX) + ) { + // If we're in a child process, assume we're OK to load this non-web + // URL for now. We'll either load it externally or re-evaluate once + // we know the "real" URL to which we'll redirect. + return Services.appinfo.remoteType; + } + // This doesn't work (throws) in the child - see + // https://bugzilla.mozilla.org/show_bug.cgi?id=1589082 + // Even if it did, it'd cause sync IPC + // ( https://bugzilla.mozilla.org/show_bug.cgi?id=1589085 ), and this code + // can get called several times per page load so that seems like something + // we'd want to avoid. + let handlerInfo = extProtService.getProtocolHandlerInfo(aTargetUri.scheme); + try { + if (!handlerInfo.alwaysAskBeforeHandling) { + let app = handlerInfo.preferredApplicationHandler; + app.QueryInterface(Ci.nsIWebHandlerApp); + // If we get here, the default handler is a web app. + // Target to the origin of that web app: + let uriStr = app.uriTemplate.replace(/%s/, aTargetUri.spec); + aTargetUri = Services.io.newURI(uriStr); + } + } catch (ex) { + // It's not strange for this to throw, we just ignore it and fall through. + } + } + // If the domain is whitelisted to allow it to use file:// URIs, then we have // to run it in a file content process, in case it uses file:// sub-resources. const sm = Services.scriptSecurityManager; diff --git a/uriloader/exthandler/tests/mochitest/browser.ini b/uriloader/exthandler/tests/mochitest/browser.ini index bd32d0540307..b8e9c8937ad7 100644 --- a/uriloader/exthandler/tests/mochitest/browser.ini +++ b/uriloader/exthandler/tests/mochitest/browser.ini @@ -13,4 +13,3 @@ support-files = [browser_download_privatebrowsing.js] [browser_remember_download_option.js] [browser_web_protocol_handlers.js] -skip-if = fission