[tbb-commits] [tor-browser/tor-browser-83.0-10.0-1] Bug 19273: Avoid JavaScript patching of the external app helper dialog.

sysrqb at torproject.org sysrqb at torproject.org
Thu Nov 26 14:53:02 UTC 2020


commit 3365ab1778dc83cac32bac5d8f3f184081b583ae
Author: Kathy Brade <brade at pearlcrescent.com>
Date:   Tue Jun 28 15:13:05 2016 -0400

    Bug 19273: Avoid JavaScript patching of the external app helper dialog.
    
    When handling an external URI or downloading a file, invoke Torbutton's
    external app blocker component (which will present a download warning
    dialog unless the user has checked the "Automatically download files
    from now on" box).
    
    For e10s compatibility, avoid using a modal dialog and instead use
    a callback interface (nsIHelperAppWarningLauncher) to allow Torbutton
    to indicate the user's desire to cancel or continue each request.
    
    Other bugs fixed:
     Bug 21766: Crash with e10s enabled while trying to download a file
     Bug 21886: Download is stalled in non-e10s mode
     Bug 22471: Downloading files via the PDF viewer download button is broken
     Bug 22472: Fix FTP downloads when external helper app dialog is shown
     Bug 22610: Avoid crashes when canceling external helper app downloads
     Bug 22618: Downloading pdf file via file:/// is stalling
---
 .../exthandler/nsExternalHelperAppService.cpp      | 207 +++++++++++++++++----
 uriloader/exthandler/nsExternalHelperAppService.h  |   3 +
 .../exthandler/nsIExternalHelperAppService.idl     |  47 +++++
 3 files changed, 222 insertions(+), 35 deletions(-)

diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp
index 8e38cbf6e3fa..93e01ed472c3 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -133,6 +133,9 @@ static const char NEVER_ASK_FOR_SAVE_TO_DISK_PREF[] =
 static const char NEVER_ASK_FOR_OPEN_FILE_PREF[] =
     "browser.helperApps.neverAsk.openFile";
 
+static const char WARNING_DIALOG_CONTRACT_ID[] =
+    "@torproject.org/torbutton-extAppBlocker;1";
+
 // Helper functions for Content-Disposition headers
 
 /**
@@ -389,6 +392,22 @@ static nsresult GetDownloadDirectory(nsIFile** _directory,
   return NS_OK;
 }
 
+static already_AddRefed<nsIInterfaceRequestor> GetDialogParentAux(
+    BrowsingContext* aBrowsingContext, nsIInterfaceRequestor* aWindowContext) {
+  nsCOMPtr<nsIInterfaceRequestor> dialogParent = aWindowContext;
+
+  if (!dialogParent && aBrowsingContext) {
+    dialogParent = do_QueryInterface(aBrowsingContext->GetDOMWindow());
+  }
+  if (!dialogParent && aBrowsingContext && XRE_IsParentProcess()) {
+    RefPtr<Element> element = aBrowsingContext->Top()->GetEmbedderElement();
+    if (element) {
+      dialogParent = do_QueryInterface(element->OwnerDoc()->GetWindow());
+    }
+  }
+  return dialogParent.forget();
+}
+
 /**
  * Structure for storing extension->type mappings.
  * @see defaultMimeEntries
@@ -576,6 +595,111 @@ static const char* descriptionOverwriteExtensions[] = {
     "avif", "pdf", "svg", "webp", "xml",
 };
 
+//////////////////////////////////////////////////////////////////////////////////////////////////////
+// begin nsExternalLoadURIHandler class definition and implementation
+//////////////////////////////////////////////////////////////////////////////////////////////////////
+class nsExternalLoadURIHandler final : public nsIHelperAppWarningLauncher {
+ public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_NSIHELPERAPPWARNINGLAUNCHER
+
+  nsExternalLoadURIHandler(nsIHandlerInfo* aHandlerInfo, nsIURI* aURI,
+                           nsIPrincipal* aTriggeringPrincipal,
+                           BrowsingContext* aBrowsingContext);
+
+ protected:
+  ~nsExternalLoadURIHandler();
+
+  nsCOMPtr<nsIHandlerInfo> mHandlerInfo;
+  nsCOMPtr<nsIURI> mURI;
+  nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
+  RefPtr<BrowsingContext> mBrowsingContext;
+  nsCOMPtr<nsIHelperAppWarningDialog> mWarningDialog;
+};
+
+NS_IMPL_ADDREF(nsExternalLoadURIHandler)
+NS_IMPL_RELEASE(nsExternalLoadURIHandler)
+
+NS_INTERFACE_MAP_BEGIN(nsExternalLoadURIHandler)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIHelperAppWarningLauncher)
+  NS_INTERFACE_MAP_ENTRY(nsIHelperAppWarningLauncher)
+NS_INTERFACE_MAP_END
+
+nsExternalLoadURIHandler::nsExternalLoadURIHandler(
+    nsIHandlerInfo* aHandlerInfo, nsIURI* aURI,
+    nsIPrincipal* aTriggeringPrincipal, BrowsingContext* aBrowsingContext)
+    : mHandlerInfo(aHandlerInfo),
+      mURI(aURI),
+      mTriggeringPrincipal(aTriggeringPrincipal),
+      mBrowsingContext(aBrowsingContext)
+
+{
+  nsresult rv = NS_OK;
+  mWarningDialog = do_CreateInstance(WARNING_DIALOG_CONTRACT_ID, &rv);
+  if (NS_SUCCEEDED(rv) && mWarningDialog) {
+    // This will create a reference cycle (the dialog holds a reference to us
+    // as nsIHelperAppWarningLauncher), which will be broken in ContinueRequest
+    // or CancelRequest.
+    nsCOMPtr<nsIInterfaceRequestor> dialogParent =
+        GetDialogParentAux(aBrowsingContext, nullptr);
+    rv = mWarningDialog->MaybeShow(this, dialogParent);
+  }
+
+  if (NS_FAILED(rv)) {
+    // If for some reason we could not open the download warning prompt,
+    // continue with the request.
+    ContinueRequest();
+  }
+}
+
+nsExternalLoadURIHandler::~nsExternalLoadURIHandler() {}
+
+NS_IMETHODIMP nsExternalLoadURIHandler::ContinueRequest() {
+  MOZ_ASSERT(mURI);
+  MOZ_ASSERT(mHandlerInfo);
+
+  // Break our reference cycle with the download warning dialog (set up in
+  // LoadURI).
+  mWarningDialog = nullptr;
+
+  nsHandlerInfoAction preferredAction;
+  mHandlerInfo->GetPreferredAction(&preferredAction);
+  bool alwaysAsk = true;
+  mHandlerInfo->GetAlwaysAskBeforeHandling(&alwaysAsk);
+
+  nsresult rv = NS_OK;
+  // If we are not supposed to ask, and the preferred action is to use
+  // a helper app or the system default, we just launch the URI.
+  if (!alwaysAsk && (preferredAction == nsIHandlerInfo::useHelperApp ||
+                     preferredAction == nsIHandlerInfo::useSystemDefault)) {
+    rv = mHandlerInfo->LaunchWithURI(mURI, mBrowsingContext);
+    // We are not supposed to ask, but when file not found the user most likely
+    // uninstalled the application which handles the uri so we will continue
+    // by application chooser dialog.
+    if (rv != NS_ERROR_FILE_NOT_FOUND) {
+      return rv;
+    }
+  }
+
+  nsCOMPtr<nsIContentDispatchChooser> chooser =
+      do_CreateInstance("@mozilla.org/content-dispatch-chooser;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return chooser->Ask(mHandlerInfo, mURI, mTriggeringPrincipal,
+                      mBrowsingContext,
+                      nsIContentDispatchChooser::REASON_CANNOT_HANDLE);
+}
+
+NS_IMETHODIMP nsExternalLoadURIHandler::CancelRequest(nsresult aReason) {
+  NS_ENSURE_ARG(NS_FAILED(aReason));
+
+  // Break our reference cycle with the download warning dialog (set up in
+  // LoadURI).
+  mWarningDialog = nullptr;
+
+  return NS_OK;
+}
+
 static StaticRefPtr<nsExternalHelperAppService> sExtHelperAppSvcSingleton;
 
 /**
@@ -602,6 +726,9 @@ nsExternalHelperAppService::GetSingleton() {
   return do_AddRef(sExtHelperAppSvcSingleton);
 }
 
+//////////////////////////////////////////////////////////////////////////////////////////////////////
+// nsExternalHelperAppService definition and implementation
+//////////////////////////////////////////////////////////////////////////////////////////////////////
 NS_IMPL_ISUPPORTS(nsExternalHelperAppService, nsIExternalHelperAppService,
                   nsPIExternalAppLauncher, nsIExternalProtocolService,
                   nsIMIMEService, nsIObserver, nsISupportsWeakReference)
@@ -1045,30 +1172,13 @@ nsExternalHelperAppService::LoadURI(nsIURI* aURI,
   rv = GetProtocolHandlerInfo(scheme, getter_AddRefs(handler));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsHandlerInfoAction preferredAction;
-  handler->GetPreferredAction(&preferredAction);
-  bool alwaysAsk = true;
-  handler->GetAlwaysAskBeforeHandling(&alwaysAsk);
-
-  // if we are not supposed to ask, and the preferred action is to use
-  // a helper app or the system default, we just launch the URI.
-  if (!alwaysAsk && (preferredAction == nsIHandlerInfo::useHelperApp ||
-                     preferredAction == nsIHandlerInfo::useSystemDefault)) {
-    rv = handler->LaunchWithURI(uri, aBrowsingContext);
-    // We are not supposed to ask, but when file not found the user most likely
-    // uninstalled the application which handles the uri so we will continue
-    // by application chooser dialog.
-    if (rv != NS_ERROR_FILE_NOT_FOUND) {
-      return rv;
-    }
+  RefPtr<nsExternalLoadURIHandler> h = new nsExternalLoadURIHandler(
+      handler, uri, aTriggeringPrincipal, aBrowsingContext);
+  if (!h) {
+    return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  nsCOMPtr<nsIContentDispatchChooser> chooser =
-      do_CreateInstance("@mozilla.org/content-dispatch-chooser;1", &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return chooser->Ask(handler, uri, aTriggeringPrincipal, aBrowsingContext,
-                      nsIContentDispatchChooser::REASON_CANNOT_HANDLE);
+  return NS_OK;
 }
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -1213,6 +1323,7 @@ NS_INTERFACE_MAP_BEGIN(nsExternalAppHandler)
   NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
   NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
   NS_INTERFACE_MAP_ENTRY(nsIHelperAppLauncher)
+  NS_INTERFACE_MAP_ENTRY(nsIHelperAppWarningLauncher)
   NS_INTERFACE_MAP_ENTRY(nsICancelable)
   NS_INTERFACE_MAP_ENTRY(nsIBackgroundFileSaverObserver)
   NS_INTERFACE_MAP_ENTRY(nsINamed)
@@ -1575,18 +1686,7 @@ void nsExternalAppHandler::MaybeApplyDecodingForExtension(
 
 already_AddRefed<nsIInterfaceRequestor>
 nsExternalAppHandler::GetDialogParent() {
-  nsCOMPtr<nsIInterfaceRequestor> dialogParent = mWindowContext;
-
-  if (!dialogParent && mBrowsingContext) {
-    dialogParent = do_QueryInterface(mBrowsingContext->GetDOMWindow());
-  }
-  if (!dialogParent && mBrowsingContext && XRE_IsParentProcess()) {
-    RefPtr<Element> element = mBrowsingContext->Top()->GetEmbedderElement();
-    if (element) {
-      dialogParent = do_QueryInterface(element->OwnerDoc()->GetWindow());
-    }
-  }
-  return dialogParent.forget();
+  return GetDialogParentAux(mBrowsingContext, mWindowContext);
 }
 
 NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
@@ -1717,6 +1817,34 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
     loadInfo->SetForceAllowDataURI(true);
   }
 
+  mWarningDialog = do_CreateInstance(WARNING_DIALOG_CONTRACT_ID, &rv);
+  if (NS_SUCCEEDED(rv) && mWarningDialog) {
+    // This will create a reference cycle (the dialog holds a reference to us
+    // as nsIHelperAppWarningLauncher), which will be broken in ContinueRequest
+    // or CancelRequest.
+    nsCOMPtr<nsIInterfaceRequestor> dialogParent = GetDialogParent();
+    rv = mWarningDialog->MaybeShow(this, dialogParent);
+  }
+
+  if (NS_FAILED(rv)) {
+    // If for some reason we could not open the download warning prompt,
+    // continue with the request.
+    ContinueRequest();
+  }
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP nsExternalAppHandler::ContinueRequest() {
+  nsAutoCString MIMEType;
+  if (mMimeInfo) {
+    mMimeInfo->GetMIMEType(MIMEType);
+  }
+
+  // Break our reference cycle with the download warning dialog (set up in
+  // OnStartRequest).
+  mWarningDialog = nullptr;
+
   // now that the temp file is set up, find out if we need to invoke a dialog
   // asking the user what they want us to do with this content...
 
@@ -1800,6 +1928,7 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
     action = nsIMIMEInfo::saveToDisk;
   }
 
+  nsresult rv = NS_OK;
   if (alwaysAsk) {
     // Display the dialog
     mDialog = do_CreateInstance(NS_HELPERAPPLAUNCHERDLG_CONTRACTID, &rv);
@@ -1857,6 +1986,14 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
   return NS_OK;
 }
 
+NS_IMETHODIMP nsExternalAppHandler::CancelRequest(nsresult aReason) {
+  // Break our reference cycle with the download warning dialog (set up in
+  // OnStartRequest).
+  mWarningDialog = nullptr;
+
+  return Cancel(aReason);
+}
+
 // Convert error info into proper message text and send OnStatusChange
 // notification to the dialog progress listener or nsITransfer implementation.
 void nsExternalAppHandler::SendStatusChange(ErrorType type, nsresult rv,
@@ -2521,7 +2658,7 @@ NS_IMETHODIMP nsExternalAppHandler::Cancel(nsresult aReason) {
   }
 
   // Break our reference cycle with the helper app dialog (set up in
-  // OnStartRequest)
+  // ContinueRequest)
   mDialog = nullptr;
 
   mRequest = nullptr;
diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h
index 1edde606dc65..e732b6063f59 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.h
+++ b/uriloader/exthandler/nsExternalHelperAppService.h
@@ -219,6 +219,7 @@ class nsExternalHelperAppService : public nsIExternalHelperAppService,
  */
 class nsExternalAppHandler final : public nsIStreamListener,
                                    public nsIHelperAppLauncher,
+                                   public nsIHelperAppWarningLauncher,
                                    public nsIBackgroundFileSaverObserver,
                                    public nsINamed {
  public:
@@ -226,6 +227,7 @@ class nsExternalAppHandler final : public nsIStreamListener,
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSIHELPERAPPLAUNCHER
+  NS_DECL_NSIHELPERAPPWARNINGLAUNCHER
   NS_DECL_NSICANCELABLE
   NS_DECL_NSIBACKGROUNDFILESAVEROBSERVER
   NS_DECL_NSINAMED
@@ -485,6 +487,7 @@ class nsExternalAppHandler final : public nsIStreamListener,
   nsCOMPtr<nsITransfer> mTransfer;
 
   nsCOMPtr<nsIHelperAppLauncherDialog> mDialog;
+  nsCOMPtr<nsIHelperAppWarningDialog> mWarningDialog;
 
   /**
 
diff --git a/uriloader/exthandler/nsIExternalHelperAppService.idl b/uriloader/exthandler/nsIExternalHelperAppService.idl
index 657e15bc0742..ebdb1cdacf78 100644
--- a/uriloader/exthandler/nsIExternalHelperAppService.idl
+++ b/uriloader/exthandler/nsIExternalHelperAppService.idl
@@ -177,3 +177,50 @@ interface nsIHelperAppLauncher : nsICancelable
    */
   readonly attribute uint64_t browsingContextId;
 };
+
+/**
+  * nsIHelperAppWarningLauncher is implemented by two classes:
+  *   nsExternalLoadURIHandler
+  *   nsExternalAppHandler
+  */
+[scriptable, uuid(cffd508b-4aaf-43ad-99c6-671d35cbc558)]
+interface nsIHelperAppWarningLauncher : nsISupports
+{
+  /**
+   * Callback invoked by the external app warning dialog to continue the
+   * request.
+   * NOTE: This will release the reference to the nsIHelperAppWarningDialog.
+   */
+  void continueRequest();
+
+  /**
+   * Callback invoked by the external app warning dialog to cancel the request.
+   * NOTE: This will release the reference to the nsIHelperAppWarningDialog.
+   *
+   * @param aReason
+   *        Pass a failure code to indicate the reason why this operation is
+   *        being canceled. It is an error to pass a success code.
+   */
+  void cancelRequest(in nsresult aReason);
+};
+
+/**
+ * nsIHelperAppWarningDialog is implemented by Torbutton's external app
+ * blocker (src/components/external-app-blocker.js).
+ */
+[scriptable, uuid(f4899a3f-0df3-42cc-9db8-bdf599e5a208)]
+interface nsIHelperAppWarningDialog : nsISupports
+{
+  /**
+   * Possibly show a launch warning dialog (it will not be shown if the user
+   * has chosen to not see the warning again).
+   *
+   * @param aLauncher
+   *        A nsIHelperAppWarningLauncher to be invoked after the user confirms
+   *        or cancels the download.
+   * @param aWindowContext
+   *        The window associated with the download.
+   */
+  void maybeShow(in nsIHelperAppWarningLauncher aLauncher,
+                 in nsISupports aWindowContext);
+};





More information about the tbb-commits mailing list