From 394ac42d425135fc0b8bb3fdd9821597f7294b34 Mon Sep 17 00:00:00 2001
From: Christoph Kerschbaumer <ckerschb@christophkerschbaumer.com>
Date: Wed, 5 Aug 2020 15:18:01 +0000
Subject: [PATCH] Bug 1657348: Simplify CouldBeHttpsOnlyError within docshell.
 r=JulianWels

Differential Revision: https://phabricator.services.mozilla.com/D86009
---
 docshell/base/nsDocShell.cpp               | 10 ++-----
 dom/security/nsHTTPSOnlyStreamListener.cpp |  4 +--
 dom/security/nsHTTPSOnlyUtils.cpp          | 35 ++++++++++++++++++----
 dom/security/nsHTTPSOnlyUtils.h            |  3 +-
 4 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
index 9a0cd91d86e5..ee0753eb0fdc 100644
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3581,14 +3581,8 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
 
   // If the HTTPS-Only Mode upgraded this request and the upgrade might have
   // caused this error, we replace the error-page with about:httpsonlyerror
-  if (aFailedChannel && nsHTTPSOnlyUtils::CouldBeHttpsOnlyError(aError)) {
-    nsCOMPtr<nsILoadInfo> loadInfo = aFailedChannel->LoadInfo();
-    uint32_t httpsOnlyStatus = loadInfo->GetHttpsOnlyStatus();
-    if ((httpsOnlyStatus &
-         nsILoadInfo::HTTPS_ONLY_UPGRADED_LISTENER_REGISTERED) &&
-        !(httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_EXEMPT)) {
-      errorPage.AssignLiteral("httpsonlyerror");
-    }
+  if (nsHTTPSOnlyUtils::CouldBeHttpsOnlyError(aFailedChannel, aError)) {
+    errorPage.AssignLiteral("httpsonlyerror");
   }
 
   if (nsCOMPtr<nsILoadURIDelegate> loadURIDelegate = GetLoadURIDelegate()) {
diff --git a/dom/security/nsHTTPSOnlyStreamListener.cpp b/dom/security/nsHTTPSOnlyStreamListener.cpp
index daefdca73451..ca0daa39097e 100644
--- a/dom/security/nsHTTPSOnlyStreamListener.cpp
+++ b/dom/security/nsHTTPSOnlyStreamListener.cpp
@@ -41,8 +41,8 @@ nsHTTPSOnlyStreamListener::OnStartRequest(nsIRequest* request) {
 NS_IMETHODIMP
 nsHTTPSOnlyStreamListener::OnStopRequest(nsIRequest* request,
                                          nsresult aStatus) {
-  // DNS errors are unrelated to the HTTPS-Only mode, so they can be ignored.
-  if (nsHTTPSOnlyUtils::CouldBeHttpsOnlyError(aStatus)) {
+  nsCOMPtr<nsIChannel> channel = do_QueryInterface(request);
+  if (nsHTTPSOnlyUtils::CouldBeHttpsOnlyError(channel, aStatus)) {
     RecordUpgradeTelemetry(request, aStatus);
     LogUpgradeFailure(request, aStatus);
   }
diff --git a/dom/security/nsHTTPSOnlyUtils.cpp b/dom/security/nsHTTPSOnlyUtils.cpp
index 710b7b5212a5..39be06f78ef5 100644
--- a/dom/security/nsHTTPSOnlyUtils.cpp
+++ b/dom/security/nsHTTPSOnlyUtils.cpp
@@ -125,9 +125,34 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeWebSocket(nsIURI* aURI,
 }
 
 /* static */
-bool nsHTTPSOnlyUtils::CouldBeHttpsOnlyError(nsresult aError) {
-  // This list of error codes is largely drawn from
-  // nsDocShell::DisplayLoadError()
+bool nsHTTPSOnlyUtils::CouldBeHttpsOnlyError(nsIChannel* aChannel,
+                                             nsresult aError) {
+  // If there is no failed channel, then there is nothing to do here.
+  if (!aChannel) {
+    return false;
+  }
+
+  // If HTTPS-Only Mode is not enabled, then there is nothing to do here.
+  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
+  bool isPrivateWin = loadInfo->GetOriginAttributes().mPrivateBrowsingId > 0;
+  if (!IsHttpsOnlyModeEnabled(isPrivateWin)) {
+    return false;
+  }
+
+  // If the listener is not registerd, then there is nothing to do here.
+  uint32_t httpsOnlyStatus = loadInfo->GetHttpsOnlyStatus();
+  if (!(httpsOnlyStatus &
+        nsILoadInfo::HTTPS_ONLY_UPGRADED_LISTENER_REGISTERED)) {
+    return false;
+  }
+
+  // If the load is exempt, then there is nothing to do here.
+  if (httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_EXEMPT) {
+    return false;
+  }
+
+  // If it's one of those errors, then most likely it's not a HTTPS-Only error
+  // (This list of errors is largely drawn from nsDocShell::DisplayLoadError())
   return !(NS_ERROR_UNKNOWN_PROTOCOL == aError ||
            NS_ERROR_FILE_NOT_FOUND == aError ||
            NS_ERROR_FILE_ACCESS_DENIED == aError ||
@@ -213,8 +238,8 @@ bool nsHTTPSOnlyUtils::LoopbackOrLocalException(nsIURI* aURI) {
   nsresult rv = aURI->GetAsciiHost(asciiHost);
   NS_ENSURE_SUCCESS(rv, false);
 
-  // Let's make a quick check if the host matches these loopback strings before
-  // we do anything else
+  // Let's make a quick check if the host matches these loopback strings
+  // before we do anything else
   if (asciiHost.EqualsLiteral("localhost") || asciiHost.EqualsLiteral("::1")) {
     return true;
   }
diff --git a/dom/security/nsHTTPSOnlyUtils.h b/dom/security/nsHTTPSOnlyUtils.h
index 1a95bd626743..85f361988e28 100644
--- a/dom/security/nsHTTPSOnlyUtils.h
+++ b/dom/security/nsHTTPSOnlyUtils.h
@@ -44,10 +44,11 @@ class nsHTTPSOnlyUtils {
   /**
    * Checks if the error code is on a block-list of codes that are probably not
    * related to a HTTPS-Only Mode upgrade.
+   * @param  aChannel The failed Channel.
    * @param  aError Error Code from Request
    * @return        false if error is not related to upgrade
    */
-  static bool CouldBeHttpsOnlyError(nsresult aError);
+  static bool CouldBeHttpsOnlyError(nsIChannel* aChannel, nsresult aError);
 
   /**
    * Logs localized message to either content console or browser console