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

gk at torproject.org gk at torproject.org
Tue Oct 17 12:25:00 UTC 2017


commit bad2f0be0d196bf51fb69d8e26147b3df5733a13
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
---
 netwerk/protocol/http/HttpBaseChannel.cpp          |   7 +
 netwerk/protocol/http/HttpBaseChannel.h            |   1 +
 netwerk/protocol/http/NullHttpChannel.cpp          |   6 +
 netwerk/protocol/http/nsIHttpChannel.idl           |   9 +-
 .../protocol/viewsource/nsViewSourceChannel.cpp    |  11 ++
 .../exthandler/nsExternalHelperAppService.cpp      | 219 +++++++++++++++++++--
 uriloader/exthandler/nsExternalHelperAppService.h  |   3 +
 .../exthandler/nsIExternalHelperAppService.idl     |  47 +++++
 8 files changed, 285 insertions(+), 18 deletions(-)

diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp
index e0f7edec05f9..5ea6a002a40f 100644
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1233,6 +1233,13 @@ NS_IMETHODIMP HttpBaseChannel::SetTopLevelContentWindowId(uint64_t aWindowId)
   return NS_OK;
 }
 
+NS_IMETHODIMP HttpBaseChannel::IsPendingUnforced(bool *aIsPendingUnforced)
+{
+  NS_ENSURE_ARG_POINTER(aIsPendingUnforced);
+  *aIsPendingUnforced = mIsPending;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 HttpBaseChannel::GetTransferSize(uint64_t *aTransferSize)
 {
diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h
index c8184a601b9e..3a1a8ba525b0 100644
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -190,6 +190,7 @@ public:
   NS_IMETHOD SetChannelId(const nsACString& aChannelId) override;
   NS_IMETHOD GetTopLevelContentWindowId(uint64_t *aContentWindowId) override;
   NS_IMETHOD SetTopLevelContentWindowId(uint64_t aContentWindowId) override;
+  NS_IMETHOD IsPendingUnforced(bool *aIsPendingUnforced) override;
 
   // nsIHttpChannelInternal
   NS_IMETHOD GetDocumentURI(nsIURI **aDocumentURI) override;
diff --git a/netwerk/protocol/http/NullHttpChannel.cpp b/netwerk/protocol/http/NullHttpChannel.cpp
index 8c048a6b5a7e..8b362cb24233 100644
--- a/netwerk/protocol/http/NullHttpChannel.cpp
+++ b/netwerk/protocol/http/NullHttpChannel.cpp
@@ -82,6 +82,12 @@ NullHttpChannel::SetTopLevelContentWindowId(uint64_t aWindowId)
 }
 
 NS_IMETHODIMP
+NullHttpChannel::IsPendingUnforced(bool *_retval)
+{
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
 NullHttpChannel::GetTransferSize(uint64_t *aTransferSize)
 {
     return NS_ERROR_NOT_IMPLEMENTED;
diff --git a/netwerk/protocol/http/nsIHttpChannel.idl b/netwerk/protocol/http/nsIHttpChannel.idl
index 75ec2c73989a..0de0676ad156 100644
--- a/netwerk/protocol/http/nsIHttpChannel.idl
+++ b/netwerk/protocol/http/nsIHttpChannel.idl
@@ -14,7 +14,7 @@ interface nsIHttpHeaderVisitor;
  * the inspection of the resulting HTTP response status and headers when they
  * become available.
  */
-[builtinclass, scriptable, uuid(c5a4a073-4539-49c7-a3f2-cec3f0619c6c)]
+[builtinclass, scriptable, uuid(e0d8071b-5389-48c2-92c7-6708c968044d)]
 interface nsIHttpChannel : nsIChannel
 {
     /**************************************************************************
@@ -469,4 +469,11 @@ interface nsIHttpChannel : nsIChannel
      * this channels is being load in.
      */
     attribute uint64_t topLevelContentWindowId;
+
+    /**
+     * Returns true if a request is pending due to "natural" causes and
+     * not just because ForcePending() has been called. See isPending()
+     * in nsIRequest.idl for more details about pending requests.
+     */
+    boolean isPendingUnforced();
 };
diff --git a/netwerk/protocol/viewsource/nsViewSourceChannel.cpp b/netwerk/protocol/viewsource/nsViewSourceChannel.cpp
index 9ed71c4ef001..9f9b89438fba 100644
--- a/netwerk/protocol/viewsource/nsViewSourceChannel.cpp
+++ b/netwerk/protocol/viewsource/nsViewSourceChannel.cpp
@@ -740,6 +740,17 @@ nsViewSourceChannel::SetTopLevelContentWindowId(uint64_t aWindowId)
 }
 
 NS_IMETHODIMP
+nsViewSourceChannel::IsPendingUnforced(bool *result)
+{
+  if (mHttpChannel) {
+    return mHttpChannel->IsPendingUnforced(result);
+  }
+
+  NS_ENSURE_TRUE(mChannel, NS_ERROR_FAILURE);
+  return mChannel->IsPending(result);
+}
+
+NS_IMETHODIMP
 nsViewSourceChannel::GetRequestMethod(nsACString & aRequestMethod)
 {
     return !mHttpChannel ? NS_ERROR_NULL_POINTER :
diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp
index 51a7ee0f6ab6..7d2dad321b6c 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -41,6 +41,7 @@
 #include "nsThreadUtils.h"
 #include "nsAutoPtr.h"
 #include "nsIMutableArray.h"
+#include "nsISupportsPrimitives.h" // for nsISupportsPRBool
 
 // used to access our datastore of user-configured helper applications
 #include "nsIHandlerService.h"
@@ -105,6 +106,7 @@
 
 #include "mozilla/Preferences.h"
 #include "mozilla/ipc/URIUtils.h"
+#include "mozilla/Unused.h"
 
 using namespace mozilla;
 using namespace mozilla::ipc;
@@ -132,6 +134,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
 
 /**
@@ -434,6 +439,22 @@ static nsresult GetDownloadDirectory(nsIFile **_directory,
   return NS_OK;
 }
 
+static nsresult shouldCancel(bool *aShouldCancel)
+{
+  NS_ENSURE_ARG_POINTER(aShouldCancel);
+
+  nsCOMPtr<nsISupportsPRBool> cancelObj =
+                            do_CreateInstance(NS_SUPPORTS_PRBOOL_CONTRACTID);
+  cancelObj->SetData(false);
+  nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
+  if (!obs)
+    return NS_ERROR_FAILURE;
+
+  obs->NotifyObservers(cancelObj, "external-app-requested", nullptr);
+  cancelObj->GetData(aShouldCancel);
+  return NS_OK;
+}
+
 /**
  * Structure for storing extension->type mappings.
  * @see defaultMimeEntries
@@ -594,6 +615,107 @@ static const nsDefaultMimeTypeEntry nonDecodableExtensions[] = {
   { APPLICATION_GZIP, "svgz" }
 };
 
+//////////////////////////////////////////////////////////////////////////////////////////////////////
+// begin nsExternalLoadURIHandler class definition and implementation
+//////////////////////////////////////////////////////////////////////////////////////////////////////
+class nsExternalLoadURIHandler final : public nsIHelperAppWarningLauncher
+{
+public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_NSIHELPERAPPWARNINGLAUNCHER
+
+  nsExternalLoadURIHandler(nsIInterfaceRequestor *aWindowContext,
+                           nsIURI *aURI,
+                           nsIHandlerInfo *aHandlerInfo);
+
+protected:
+  ~nsExternalLoadURIHandler();
+
+  nsCOMPtr<nsIInterfaceRequestor> mWindowContext;
+  nsCOMPtr<nsIURI> mURI;
+  nsCOMPtr<nsIHandlerInfo> mHandlerInfo;
+  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_THREADSAFE
+
+nsExternalLoadURIHandler::nsExternalLoadURIHandler(
+                                       nsIInterfaceRequestor *aWindowContext,
+                                       nsIURI *aURI,
+                                       nsIHandlerInfo *aHandlerInfo)
+: mWindowContext(aWindowContext)
+, mURI(aURI)
+, mHandlerInfo(aHandlerInfo)
+{
+  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.
+    rv = mWarningDialog->MaybeShow(this, aWindowContext);
+  }
+
+  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);
+
+  // 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))
+    return mHandlerInfo->LaunchWithURI(mURI, mWindowContext);
+
+  nsresult rv = NS_OK;
+  nsCOMPtr<nsIContentDispatchChooser> chooser =
+    do_CreateInstance("@mozilla.org/content-dispatch-chooser;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return chooser->Ask(mHandlerInfo, mWindowContext, mURI,
+                      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;
+}
+
+//////////////////////////////////////////////////////////////////////////////////////////////////////
+// nsExternalHelperAppService definition and implementation
+//////////////////////////////////////////////////////////////////////////////////////////////////////
 NS_IMPL_ISUPPORTS(
   nsExternalHelperAppService,
   nsIExternalHelperAppService,
@@ -1010,27 +1132,25 @@ nsExternalHelperAppService::LoadURI(nsIURI *aURI,
     return NS_OK; // explicitly denied
   }
 
+  // Give other modules, including extensions, a chance to cancel.
+  bool doCancel = false;
+  rv = shouldCancel(&doCancel);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (doCancel) {
+    return NS_OK;
+  }
+
   nsCOMPtr<nsIHandlerInfo> handler;
   rv = GetProtocolHandlerInfo(scheme, getter_AddRefs(handler));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsHandlerInfoAction preferredAction;
-  handler->GetPreferredAction(&preferredAction);
-  bool alwaysAsk = true;
-  handler->GetAlwaysAskBeforeHandling(&alwaysAsk);
+  RefPtr<nsExternalLoadURIHandler> h =
+                   new nsExternalLoadURIHandler(aWindowContext, uri, handler);
+  if (!h) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-  // 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))
-    return handler->LaunchWithURI(uri, aWindowContext);
-  
-  nsCOMPtr<nsIContentDispatchChooser> chooser =
-    do_CreateInstance("@mozilla.org/content-dispatch-chooser;1", &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-  
-  return chooser->Ask(handler, aWindowContext, uri,
-                      nsIContentDispatchChooser::REASON_CANNOT_HANDLE);
+  return NS_OK;
 }
 
 NS_IMETHODIMP nsExternalHelperAppService::GetApplicationDescription(const nsACString& aScheme, nsAString& _retval)
@@ -1198,6 +1318,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(nsITimerCallback)
    NS_INTERFACE_MAP_ENTRY(nsIBackgroundFileSaverObserver)
@@ -1643,6 +1764,37 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest *request, nsISuppo
     return NS_OK;
   }
 
+  // Give other modules, including extensions, a chance to cancel.
+  // To avoid a problem where OnDataAvailable fires but is not handled
+  // correctly while a modal dialog displayed by Torbutton is open, we
+  // suspend and then we either cancel or resume active requests.
+  // See bugs 21766 and 21886.
+  bool isPending = false;
+  nsCOMPtr<nsIHttpChannel> httpChan = do_QueryInterface(request);
+  if (httpChan) {
+    rv = httpChan->IsPendingUnforced(&isPending);
+  } else {
+    rv = request->IsPending(&isPending);
+  }
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (isPending) {
+    Unused << request->Suspend(); // Best effort: ignore failures.
+  }
+
+  bool doCancel = false;
+  rv = shouldCancel(&doCancel);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (doCancel) {
+    mCanceled = true;
+    request->Cancel(NS_BINDING_ABORTED);
+    return NS_OK;
+  }
+
+  if (isPending) {
+    Unused << request->Resume();  // Best effort: ignore failures.
+  }
+
   rv = SetUpTempFile(aChannel);
   if (NS_FAILED(rv)) {
     nsresult transferError = rv;
@@ -1671,6 +1823,29 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest *request, nsISuppo
     httpInternal->SetChannelIsForDownload(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.
+    rv = mWarningDialog->MaybeShow(this, GetDialogParent());
+  }
+
+  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()
+{
+  // 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...
 
@@ -1731,6 +1906,7 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest *request, nsISuppo
     action = nsIMIMEInfo::saveToDisk;
   }
   
+  nsresult rv = NS_OK;
   if (alwaysAsk)
   {
     // Display the dialog
@@ -1786,6 +1962,15 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest *request, nsISuppo
   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, nsIRequest *aRequest, const nsAFlatString &path)
@@ -2477,7 +2662,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 ceec66661dd4..3146407dcbe5 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.h
+++ b/uriloader/exthandler/nsExternalHelperAppService.h
@@ -210,6 +210,7 @@ private:
  */
 class nsExternalAppHandler final : public nsIStreamListener,
                                    public nsIHelperAppLauncher,
+                                   public nsIHelperAppWarningLauncher,
                                    public nsITimerCallback,
                                    public nsIBackgroundFileSaverObserver
 {
@@ -218,6 +219,7 @@ public:
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSIHELPERAPPLAUNCHER
+  NS_DECL_NSIHELPERAPPWARNINGLAUNCHER
   NS_DECL_NSICANCELABLE
   NS_DECL_NSITIMERCALLBACK
   NS_DECL_NSIBACKGROUNDFILESAVEROBSERVER
@@ -478,6 +480,7 @@ protected:
 
   nsCOMPtr<nsIChannel> mOriginalChannel; /**< in the case of a redirect, this will be the pre-redirect channel. */
   nsCOMPtr<nsIHelperAppLauncherDialog> mDialog;
+  nsCOMPtr<nsIHelperAppWarningDialog> mWarningDialog;
 
   /**
    * Keep request alive in case when helper non-modal dialog shown.
diff --git a/uriloader/exthandler/nsIExternalHelperAppService.idl b/uriloader/exthandler/nsIExternalHelperAppService.idl
index bfdfff5ceaa8..546252f04ba3 100644
--- a/uriloader/exthandler/nsIExternalHelperAppService.idl
+++ b/uriloader/exthandler/nsIExternalHelperAppService.idl
@@ -152,3 +152,50 @@ interface nsIHelperAppLauncher : nsICancelable
    */
   readonly attribute int64_t contentLength;
 };
+
+/**
+  * 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