Bug 1520483 - Return proper error if the nss layer encounters an error on the http tunnel. r=valentin,kershaw

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

--HG--
extra : moz-landing-system : lando

netwerk/protocol/http/TunnelUtils.cpp

@@ -47,7 +47,7 @@ TLSFilterTransaction::TLSFilterTransaction(nsAHttpTransaction *aWrapped,
       mSegmentWriter(aWriter),
       mFilterReadCode(NS_ERROR_NOT_INITIALIZED),
       mForce(false),
-      mReadSegmentBlocked(false),
+      mReadSegmentReturnValue(NS_OK),
       mNudgeCounter(0) {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   LOG(("TLSFilterTransaction ctor %p\n", this));
@@ -149,7 +149,7 @@ nsresult TLSFilterTransaction::OnReadSegment(const char *aData, uint32_t aCount,
   LOG(("TLSFilterTransaction %p OnReadSegment %d (buffered %d)\n", this, aCount,
        mEncryptedTextUsed));
 
-  mReadSegmentBlocked = false;
+  mReadSegmentReturnValue = NS_OK;
   MOZ_ASSERT(mSegmentReader);
   if (!mSecInfo) {
     return NS_ERROR_FAILURE;
@@ -196,17 +196,12 @@ nsresult TLSFilterTransaction::OnReadSegment(const char *aData, uint32_t aCount,
         return NS_OK;
       }
       // mTransaction ReadSegments actually obscures this code, so
-      // keep it in a member var for this::ReadSegments to insepct. Similar
+      // keep it in a member var for this::ReadSegments to inspect. Similar
       // to nsHttpConnection::mSocketOutCondition
       PRErrorCode code = PR_GetError();
-      mReadSegmentBlocked = (code == PR_WOULD_BLOCK_ERROR);
-      if (mReadSegmentBlocked) {
-        return NS_BASE_STREAM_WOULD_BLOCK;
-      }
+      mReadSegmentReturnValue = ErrorAccordingToNSPR(code);
 
-      nsresult rv = ErrorAccordingToNSPR(code);
-      Close(rv);
-      return rv;
+      return mReadSegmentReturnValue;
     }
     aCount -= written;
     aData += written;
@@ -287,9 +282,14 @@ nsresult TLSFilterTransaction::OnWriteSegment(char *aData, uint32_t aCount,
     if (code == PR_WOULD_BLOCK_ERROR) {
       return NS_BASE_STREAM_WOULD_BLOCK;
     }
-    nsresult rv = ErrorAccordingToNSPR(code);
-    Close(rv);
-    return rv;
+    // If reading from the socket succeeded (NS_SUCCEEDED(mFilterReadCode)),
+    // but the nss layer encountered an error remember the error.
+    if (NS_SUCCEEDED(mFilterReadCode)) {
+      mFilterReadCode = ErrorAccordingToNSPR(code);
+      LOG(("TLSFilterTransaction::OnWriteSegment %p nss error %" PRIx32 ".\n",
+           this, static_cast<uint32_t>(mFilterReadCode)));
+    }
+    return mFilterReadCode;
   }
   *outCountRead = bytesRead;
 
@@ -320,7 +320,7 @@ int32_t TLSFilterTransaction::FilterInput(char *aBuf, int32_t aAmount) {
          " read=%d input from net "
          "1 layer stripped, 1 still on\n",
          static_cast<uint32_t>(mFilterReadCode), outCountRead));
-    if (mReadSegmentBlocked) {
+    if (mReadSegmentReturnValue == NS_BASE_STREAM_WOULD_BLOCK) {
       mNudgeCounter = 0;
     }
   }
@@ -341,19 +341,18 @@ nsresult TLSFilterTransaction::ReadSegments(nsAHttpSegmentReader *aReader,
     return NS_ERROR_UNEXPECTED;
   }
 
-  mReadSegmentBlocked = false;
+  mReadSegmentReturnValue = NS_OK;
   mSegmentReader = aReader;
   nsresult rv = mTransaction->ReadSegments(this, aCount, outCountRead);
   LOG(("TLSFilterTransaction %p called trans->ReadSegments rv=%" PRIx32 " %d\n",
        this, static_cast<uint32_t>(rv), *outCountRead));
-  if (NS_SUCCEEDED(rv) && mReadSegmentBlocked) {
-    rv = NS_BASE_STREAM_WOULD_BLOCK;
+  if (NS_SUCCEEDED(rv) && (mReadSegmentReturnValue == NS_BASE_STREAM_WOULD_BLOCK)) {
     LOG(("TLSFilterTransaction %p read segment blocked found rv=%" PRIx32 "\n",
          this, static_cast<uint32_t>(rv)));
     Unused << Connection()->ForceSend();
   }
 
-  return rv;
+  return NS_SUCCEEDED(rv) ? mReadSegmentReturnValue : rv;
 }
 
 nsresult TLSFilterTransaction::WriteSegments(nsAHttpSegmentWriter *aWriter,
@@ -456,8 +455,10 @@ TLSFilterTransaction::Notify(nsITimer *timer) {
   if (timer != mTimer) {
     return NS_ERROR_UNEXPECTED;
   }
-  DebugOnly<nsresult> rv = StartTimerCallback();
-  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  nsresult rv = StartTimerCallback();
+  if (NS_FAILED(rv)) {
+    Close(rv);
+  }
   return NS_OK;
 }
 
@@ -475,7 +476,7 @@ nsresult TLSFilterTransaction::StartTimerCallback() {
     // This class can be called re-entrantly, so cleanup m* before ->on()
     RefPtr<NudgeTunnelCallback> cb(mNudgeCallback);
     mNudgeCallback = nullptr;
-    cb->OnTunnelNudged(this);
+    return cb->OnTunnelNudged(this);
   }
   return NS_OK;
 }

netwerk/protocol/http/TunnelUtils.h

@@ -100,11 +100,11 @@ class TLSFilterTransaction;
 
 class NudgeTunnelCallback : public nsISupports {
  public:
-  virtual void OnTunnelNudged(TLSFilterTransaction *) = 0;
+  virtual nsresult OnTunnelNudged(TLSFilterTransaction *) = 0;
 };
 
 #define NS_DECL_NUDGETUNNELCALLBACK \
-  void OnTunnelNudged(TLSFilterTransaction *) override;
+  nsresult OnTunnelNudged(TLSFilterTransaction *) override;
 
 class TLSFilterTransaction final : public nsAHttpTransaction,
                                    public nsAHttpSegmentReader,
@@ -181,7 +181,7 @@ class TLSFilterTransaction final : public nsAHttpTransaction,
 
   nsresult mFilterReadCode;
   bool mForce;
-  bool mReadSegmentBlocked;
+  nsresult mReadSegmentReturnValue;
   uint32_t mNudgeCounter;
 };
 

netwerk/protocol/http/nsHttpConnection.cpp

@@ -701,14 +701,14 @@ npnComplete:
   return true;
 }
 
-void nsHttpConnection::OnTunnelNudged(TLSFilterTransaction *trans) {
+nsresult nsHttpConnection::OnTunnelNudged(TLSFilterTransaction *trans) {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   LOG(("nsHttpConnection::OnTunnelNudged %p\n", this));
   if (trans != mTLSFilter) {
-    return;
+    return NS_OK;
   }
   LOG(("nsHttpConnection::OnTunnelNudged %p Calling OnSocketWritable\n", this));
-  Unused << OnSocketWritable();
+  return OnSocketWritable();
 }
 
 // called on the socket thread