Bug 1685942 - Only fallback to original conn info when network error happens r=necko-reviewers,valentin,dragana

Differential Revision: https://phabricator.services.mozilla.com/D102703

docshell/base/nsDocShell.cpp

@@ -3602,7 +3602,8 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
     addHostPort = true;
     error = "netInterrupt";
   } else if (NS_ERROR_NET_TIMEOUT == aError ||
-             NS_ERROR_PROXY_GATEWAY_TIMEOUT == aError) {
+             NS_ERROR_PROXY_GATEWAY_TIMEOUT == aError ||
+             NS_ERROR_NET_TIMEOUT_EXTERNAL == aError) {
     NS_ENSURE_ARG_POINTER(aURI);
     // Get the host
     nsAutoCString host;
@@ -6381,6 +6382,7 @@ nsresult nsDocShell::FilterStatusForErrorPage(
   }
 
   if (aStatus == NS_ERROR_NET_TIMEOUT ||
+      aStatus == NS_ERROR_NET_TIMEOUT_EXTERNAL ||
       aStatus == NS_ERROR_PROXY_GATEWAY_TIMEOUT ||
       aStatus == NS_ERROR_REDIRECT_LOOP ||
       aStatus == NS_ERROR_UNKNOWN_SOCKET_TYPE ||

dom/security/nsHTTPSOnlyStreamListener.cpp

@@ -111,7 +111,8 @@ void nsHTTPSOnlyStreamListener::RecordUpgradeTelemetry(nsIRequest* request,
 
     if (aStatus == NS_ERROR_REDIRECT_LOOP) {
       category.AppendLiteral("f_redirectloop");
-    } else if (aStatus == NS_ERROR_NET_TIMEOUT) {
+    } else if (aStatus == NS_ERROR_NET_TIMEOUT ||
+               aStatus == NS_ERROR_NET_TIMEOUT_EXTERNAL) {
       category.AppendLiteral("f_timeout");
     } else if (aStatus == NS_BINDING_ABORTED) {
       category.AppendLiteral("f_aborted");

dom/security/nsHTTPSOnlyUtils.cpp

@@ -448,7 +448,7 @@ TestHTTPAnswerRunnable::OnStartRequest(nsIRequest* aRequest) {
       nsresult httpsOnlyChannelStatus;
       httpsOnlyChannel->GetStatus(&httpsOnlyChannelStatus);
       if (httpsOnlyChannelStatus == NS_OK) {
-        httpsOnlyChannel->Cancel(NS_ERROR_NET_TIMEOUT);
+        httpsOnlyChannel->Cancel(NS_ERROR_NET_TIMEOUT_EXTERNAL);
       }
     }
   }

js/xpconnect/src/xpc.msg

@@ -156,6 +156,7 @@ XPC_MSG_DEF(NS_ERROR_PROXY_SERVICE_UNAVAILABLE          , "The proxy is not avai
 XPC_MSG_DEF(NS_ERROR_PROXY_UNAVAILABLE_FOR_LEGAL_REASONS, "The desired destination is unavailable for legal reasons")
 
 XPC_MSG_DEF(NS_ERROR_NET_TIMEOUT                    , "The connection has timed out")
+XPC_MSG_DEF(NS_ERROR_NET_TIMEOUT_EXTERNAL           , "The request has been cancelled because of a timeout")
 XPC_MSG_DEF(NS_ERROR_OFFLINE                        , "The requested action could not be completed in the offline state")
 XPC_MSG_DEF(NS_ERROR_PORT_ACCESS_NOT_ALLOWED        , "Establishing a connection to an unsafe or otherwise banned port was prohibited")
 XPC_MSG_DEF(NS_ERROR_NET_RESET                      , "The connection was established, but no data was ever received")

netwerk/base/nsSocketTransport2.cpp

@@ -2552,6 +2552,10 @@ nsSocketTransport::GetPeerAddr(NetAddr* addr) {
   // we can freely access mNetAddr from any thread without being
   // inside a critical section.
 
+  if (NS_FAILED(mCondition)) {
+    return mCondition;
+  }
+
   if (!mNetAddrIsSet) {
     SOCKET_LOG(
         ("nsSocketTransport::GetPeerAddr [this=%p state=%d] "
@@ -2571,6 +2575,10 @@ nsSocketTransport::GetSelfAddr(NetAddr* addr) {
   // we can freely access mSelfAddr from any thread without being
   // inside a critical section.
 
+  if (NS_FAILED(mCondition)) {
+    return mCondition;
+  }
+
   if (!mSelfAddrIsSet) {
     SOCKET_LOG(
         ("nsSocketTransport::GetSelfAddr [this=%p state=%d] "

netwerk/protocol/http/Http3Session.cpp

@@ -92,9 +92,10 @@ nsresult Http3Session::Init(const nsHttpConnectionInfo* aConnInfo,
 
   // Get the local and remote address neqo needs it.
   NetAddr selfAddr;
-  if (NS_FAILED(mSocketTransport->GetSelfAddr(&selfAddr))) {
+  nsresult rv = mSocketTransport->GetSelfAddr(&selfAddr);
+  if (NS_FAILED(rv)) {
     LOG3(("Http3Session::Init GetSelfAddr failed [this=%p]", this));
-    return NS_ERROR_FAILURE;
+    return rv;
   }
   char buf[kIPv6CStrBufSize];
   selfAddr.ToStringBuffer(buf, kIPv6CStrBufSize);
@@ -114,9 +115,10 @@ nsresult Http3Session::Init(const nsHttpConnectionInfo* aConnInfo,
   }
 
   NetAddr peerAddr;
-  if (NS_FAILED(mSocketTransport->GetPeerAddr(&peerAddr))) {
+  rv = mSocketTransport->GetPeerAddr(&peerAddr);
+  if (NS_FAILED(rv)) {
     LOG3(("Http3Session::Init GetPeerAddr failed [this=%p]", this));
-    return NS_ERROR_FAILURE;
+    return rv;
   }
   peerAddr.ToStringBuffer(buf, kIPv6CStrBufSize);
 
@@ -142,7 +144,7 @@ nsresult Http3Session::Init(const nsHttpConnectionInfo* aConnInfo,
        peerAddrStr.get(), gHttpHandler->DefaultQpackTableSize(),
        gHttpHandler->DefaultHttp3MaxBlockedStreams(), this));
 
-  nsresult rv = NeqoHttp3Conn::Init(
+  rv = NeqoHttp3Conn::Init(
       mConnInfo->GetOrigin(), mConnInfo->GetNPNToken(), selfAddrStr,
       peerAddrStr, gHttpHandler->DefaultQpackTableSize(),
       gHttpHandler->DefaultHttp3MaxBlockedStreams(),

netwerk/protocol/http/nsHttp.h

@@ -15,6 +15,7 @@
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Tuple.h"
 #include "mozilla/UniquePtr.h"
+#include "NSSErrorsService.h"
 
 class nsICacheEntry;
 
@@ -382,6 +383,13 @@ nsresult HttpProxyResponseToErrorCode(uint32_t aStatusCode);
 Tuple<nsCString, bool> SelectAlpnFromAlpnList(
     const nsTArray<nsCString>& aAlpnList, bool aNoHttp2, bool aNoHttp3);
 
+static inline bool AllowedErrorForHTTPSRRFallback(nsresult aError) {
+  return psm::IsNSSErrorCode(-1 * NS_ERROR_GET_CODE(aError)) ||
+         aError == NS_ERROR_NET_RESET ||
+         aError == NS_ERROR_CONNECTION_REFUSED ||
+         aError == NS_ERROR_UNKNOWN_HOST || aError == NS_ERROR_NET_TIMEOUT;
+}
+
 }  // namespace net
 }  // namespace mozilla
 

netwerk/protocol/http/nsHttpChannel.cpp

@@ -5924,6 +5924,9 @@ nsHttpChannel::Cancel(nsresult status) {
 
   LOG(("nsHttpChannel::Cancel [this=%p status=%" PRIx32 "]\n", this,
        static_cast<uint32_t>(status)));
+  MOZ_ASSERT_IF(!(mConnectionInfo && mConnectionInfo->UsingConnect()) &&
+                    NS_SUCCEEDED(mStatus),
+                !AllowedErrorForHTTPSRRFallback(status));
   if (mCanceled) {
     LOG(("  ignoring; already canceled\n"));
     return NS_OK;

netwerk/protocol/http/nsHttpTransaction.cpp

@@ -1445,6 +1445,9 @@ void nsHttpTransaction::Close(nsresult reason) {
   mConnected = false;
   mTunnelProvider = nullptr;
 
+  bool shouldRestartTransactionForHTTPSRR =
+      mOrigConnInfo && AllowedErrorForHTTPSRRFallback(reason);
+
   //
   // if the connection was reset or closed before we wrote any part of the
   // request or if we wrote the request but didn't receive any part of the
@@ -1476,7 +1479,7 @@ void nsHttpTransaction::Close(nsresult reason) {
   if ((reason == NS_ERROR_NET_RESET || reason == NS_OK ||
        reason ==
            psm::GetXPCOMFromNSSError(SSL_ERROR_DOWNGRADE_WITH_EARLY_DATA) ||
-       mOrigConnInfo) &&
+       shouldRestartTransactionForHTTPSRR) &&
       (!(mCaps & NS_HTTP_STICKY_CONNECTION) ||
        (mCaps & NS_HTTP_CONNECTION_RESTARTABLE) ||
        (mEarlyDataDisposition == EARLY_425))) {
@@ -1515,14 +1518,14 @@ void nsHttpTransaction::Close(nsresult reason) {
     // If this is true, it means we failed to use the HTTPSSVC connection info
     // to connect to the server. We need to retry with the original connection
     // info.
-    bool restartToFallbackConnInfo = !reallySentData && mOrigConnInfo;
+    shouldRestartTransactionForHTTPSRR &= !reallySentData;
 
     if (reason ==
             psm::GetXPCOMFromNSSError(SSL_ERROR_DOWNGRADE_WITH_EARLY_DATA) ||
         (!mReceivedData && ((mRequestHead && mRequestHead->IsSafeMethod()) ||
                             !reallySentData || connReused)) ||
-        restartToFallbackConnInfo) {
-      if (restartToFallbackConnInfo) {
+        shouldRestartTransactionForHTTPSRR) {
+      if (shouldRestartTransactionForHTTPSRR) {
         MaybeReportFailedSVCDomain(reason, mConnInfo);
         PrepareConnInfoForRetry(reason);
         mDontRetryWithDirectRoute = true;
@@ -1539,7 +1542,7 @@ void nsHttpTransaction::Close(nsresult reason) {
       if (mConnInfo && NS_SUCCEEDED(Restart())) {
         // Only record the first restart attempt.
         if (!mRestartCount) {
-          if (restartToFallbackConnInfo) {
+          if (shouldRestartTransactionForHTTPSRR) {
             Telemetry::Accumulate(Telemetry::HTTP_TRANSACTION_RESTART_REASON,
                                   TRANSACTION_RESTART_HTTPSSVC_INVOLVED);
           } else if (!reallySentData) {
@@ -3217,7 +3220,7 @@ void nsHttpTransaction::OnFastFallbackTimer() {
   HandleFallback(fallbackConnInfo);
   mOrigConnInfo.swap(backup);
 
-  if (mResolver) {
+  if (mResolver && fallbackConnInfo) {
     mResolver->PrefetchAddrRecord(fallbackConnInfo->GetRoutedHost(),
                                   mCaps & NS_HTTP_REFRESH_DNS);
   }

toolkit/components/reputationservice/ApplicationReputation.cpp

@@ -1736,7 +1736,7 @@ PendingLookup::Notify(nsITimer* aTimer) {
   MOZ_ASSERT(aTimer == mTimeoutTimer);
   Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_REMOTE_LOOKUP_TIMEOUT,
              true);
-  mChannel->Cancel(NS_ERROR_NET_TIMEOUT);
+  mChannel->Cancel(NS_ERROR_NET_TIMEOUT_EXTERNAL);
   mTimeoutTimer->Cancel();
   return NS_OK;
 }
@@ -1784,7 +1784,7 @@ NS_IMETHODIMP
 PendingLookup::OnStopRequest(nsIRequest* aRequest, nsresult aResult) {
   NS_ENSURE_STATE(mCallback);
 
-  if (aResult != NS_ERROR_NET_TIMEOUT) {
+  if (aResult != NS_ERROR_NET_TIMEOUT_EXTERNAL) {
     Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_REMOTE_LOOKUP_TIMEOUT,
                false);
 

xpcom/base/ErrorList.py

@@ -336,6 +336,8 @@ with modules["NETWORK"]:
     errors["NS_ERROR_NET_HTTP2_SENT_GOAWAY"] = FAILURE(83)
     # HTTP/3 protocol internal error
     errors["NS_ERROR_NET_HTTP3_PROTOCOL_ERROR"] = FAILURE(84)
+    # A timeout error code that can be used to cancel requests.
+    errors["NS_ERROR_NET_TIMEOUT_EXTERNAL"] = FAILURE(85)
 
     # XXX really need to better rationalize these error codes.  are consumers of
     # necko really expected to know how to discern the meaning of these??