Bug 983187 - Test that downloads fail when an RST packet is received.…

… r=mayhemer

This adds a way to simulate failed network connections, allowing the addition of test coverage that would otherwise not be available. This is used in the Downloads tests to ensure that failures at the network level are handled correctly.

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

--HG--
extra : rebase_source : 2597b27de5213b0322520cefafe10a197d0d3b83

media/mtransport/test/webrtcproxychannel_unittest.cpp

@@ -111,6 +111,10 @@ class FakeSocketTransportProvider : public nsISocketTransport {
     MOZ_ASSERT(false);
     return NS_OK;
   }
+  NS_IMETHOD SetLinger(bool aPolarity, int16_t aTimeout) override {
+    MOZ_ASSERT(false);
+    return NS_OK;
+  }
   NS_IMETHOD SetReuseAddrPort(bool reuseAddrPort) override {
     MOZ_ASSERT(false);
     return NS_OK;

netwerk/base/nsISocketTransport.idl

@@ -124,6 +124,13 @@ interface nsISocketTransport : nsITransport
     unsigned long getTimeout(in unsigned long aType);
     void          setTimeout(in unsigned long aType, in unsigned long aValue);
 
+    /**
+     * Sets the SO_LINGER option with the specified values for the l_onoff and
+     * l_linger parameters. This applies PR_SockOpt_Linger before PR_Close and
+     * can be used with a timeout of zero to send an RST packet when closing.
+     */
+    void setLinger(in boolean aPolarity, in short aTimeout);
+
     /**
      * True to set addr and port reuse socket options.
      */

netwerk/base/nsSocketTransport2.cpp

@@ -717,6 +717,8 @@ nsSocketTransport::nsSocketTransport()
       mSocketTransportService(gSocketTransportService),
       mInput(this),
       mOutput(this),
+      mLingerPolarity(false),
+      mLingerTimeout(0),
       mQoSBits(0x00),
       mKeepaliveEnabled(false),
       mKeepaliveIdleTimeS(-1),
@@ -1978,7 +1980,8 @@ class ThunkPRClose : public Runnable {
   PRFileDesc *mFD;
 };
 
-void STS_PRCloseOnSocketTransport(PRFileDesc *fd) {
+void STS_PRCloseOnSocketTransport(PRFileDesc *fd, bool lingerPolarity,
+                                  int16_t lingerTimeout) {
   if (gSocketTransportService) {
     // Can't PR_Close() a socket off STS thread. Thunk it to STS to die
     gSocketTransportService->Dispatch(new ThunkPRClose(fd), NS_DISPATCH_NORMAL);
@@ -1999,13 +2002,22 @@ void nsSocketTransport::ReleaseFD_Locked(PRFileDesc *fd) {
          gSocketTransportService->MaxTimeForPrClosePref())) {
       // If shutdown last to long, let the socket leak and do not close it.
       SOCKET_LOG(("Intentional leak"));
-    } else if (OnSocketThread()) {
-      SOCKET_LOG(("nsSocketTransport: calling PR_Close [this=%p]\n", this));
-      CloseSocket(
-          mFD, mSocketTransportService->IsTelemetryEnabledAndNotSleepPhase());
     } else {
-      // Can't PR_Close() a socket off STS thread. Thunk it to STS to die
-      STS_PRCloseOnSocketTransport(mFD);
+      if (mLingerPolarity || mLingerTimeout) {
+        PRSocketOptionData socket_linger;
+        socket_linger.option = PR_SockOpt_Linger;
+        socket_linger.value.linger.polarity = mLingerPolarity;
+        socket_linger.value.linger.linger = mLingerTimeout;
+        PR_SetSocketOption(mFD, &socket_linger);
+      }
+      if (OnSocketThread()) {
+        SOCKET_LOG(("nsSocketTransport: calling PR_Close [this=%p]\n", this));
+        CloseSocket(
+            mFD, mSocketTransportService->IsTelemetryEnabledAndNotSleepPhase());
+      } else {
+        // Can't PR_Close() a socket off STS thread. Thunk it to STS to die
+        STS_PRCloseOnSocketTransport(mFD, mLingerPolarity, mLingerTimeout);
+      }
     }
     mFD = nullptr;
   }
@@ -2756,6 +2768,16 @@ nsSocketTransport::SetReuseAddrPort(bool reuseAddrPort) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsSocketTransport::SetLinger(bool aPolarity, int16_t aTimeout) {
+  MutexAutoLock lock(mLock);
+
+  mLingerPolarity = aPolarity;
+  mLingerTimeout = aTimeout;
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsSocketTransport::SetQoSBits(uint8_t aQoSBits) {
   // Don't do any checking here of bits.  Why?  Because as of RFC-4594

netwerk/base/nsSocketTransport2.h

@@ -398,6 +398,10 @@ class nsSocketTransport final : public nsASocketHandler,
   // socket timeouts are not protected by any lock.
   uint16_t mTimeouts[2];
 
+  // linger options to use when closing
+  bool mLingerPolarity;
+  int16_t mLingerTimeout;
+
   // QoS setting for socket
   uint8_t mQoSBits;
 

netwerk/protocol/http/TunnelUtils.cpp

@@ -1889,6 +1889,11 @@ SocketTransportShim::SetReuseAddrPort(bool aReuseAddrPort) {
   return mWrapped->SetReuseAddrPort(aReuseAddrPort);
 }
 
+NS_IMETHODIMP
+SocketTransportShim::SetLinger(bool aPolarity, int16_t aTimeout) {
+  return mWrapped->SetLinger(aPolarity, aTimeout);
+}
+
 NS_IMETHODIMP
 SocketTransportShim::GetQoSBits(uint8_t *aQoSBits) {
   return mWrapped->GetQoSBits(aQoSBits);

netwerk/test/httpserver/httpd.js

@@ -398,7 +398,7 @@ nsHttpServer.prototype =
 
     try {
       var conn = new Connection(input, output, this, socket.port, trans.port,
-                                connectionNumber);
+                                connectionNumber, trans);
       var reader = new RequestReader(conn);
 
       // XXX add request timeout functionality here!
@@ -1067,7 +1067,8 @@ ServerIdentity.prototype =
  * @param number : uint
  *   a serial number used to uniquely identify this connection
  */
-function Connection(input, output, server, port, outgoingPort, number) {
+function Connection(input, output, server, port, outgoingPort, number,
+                    transport) {
   dumpn("*** opening new connection " + number + " on port " + outgoingPort);
 
   /** Stream of incoming data. */
@@ -1088,6 +1089,9 @@ function Connection(input, output, server, port, outgoingPort, number) {
   /** The serial number of this connection. */
   this.number = number;
 
+  /** Reference to the underlying transport. */
+  this.transport = transport;
+
   /**
    * The request for which a response is being generated, null if the
    * incoming request has not been fully received or if it had errors.
@@ -3559,10 +3563,19 @@ Response.prototype =
    * @param e : Error
    *   the exception which precipitated this abort, or null if no such exception
    *   was generated
+   * @param truncateConnection : Boolean
+   *   ensures that we truncate the connection using an RST packet, so the
+   *   client testing code is aware that an error occurred, otherwise it may
+   *   consider the response as valid.
    */
-  abort(e) {
+  abort(e, truncateConnection = false) {
     dumpn("*** abort(<" + e + ">)");
 
+    if (truncateConnection) {
+      dumpn("*** truncate connection");
+      this._connection.transport.setLinger(true, 0);
+    }
+
     // This response will be ended by the processor if one was created.
     var copier = this._asyncCopier;
     if (copier) {

toolkit/components/downloads/test/unit/common_test_Download.js

@@ -1451,6 +1451,51 @@ add_task(async function test_error_source_partial() {
   Assert.equal(download.target.size, 0);
 });
 
+/**
+ * Ensures a download error is reported when an RST packet is received.
+ */
+add_task(async function test_error_source_netreset() {
+  if (AppConstants.platform == "win") {
+    return;
+  }
+
+  let download;
+  try {
+    if (!gUseLegacySaver) {
+      // When testing DownloadCopySaver, we want to check that the promise
+      // returned by the "start" method is rejected.
+      download = await promiseNewDownload(httpUrl("netreset.txt"));
+
+      Assert.ok(download.error === null);
+
+      await download.start();
+    } else {
+      // When testing DownloadLegacySaver, we cannot be sure whether we are
+      // testing the promise returned by the "start" method or we are testing
+      // the "error" property checked by promiseDownloadStopped.  This happens
+      // because we don't have control over when the download is started.
+      download = await promiseStartLegacyDownload(httpUrl("netreset.txt"));
+      await promiseDownloadStopped(download);
+    }
+    do_throw("The download should have failed.");
+  } catch (ex) {
+    if (!(ex instanceof Downloads.Error) || !ex.becauseSourceFailed) {
+      throw ex;
+    }
+    // A specific error object is thrown when reading from the source fails.
+  }
+
+  // Check the properties now that the download stopped.
+  Assert.ok(download.stopped);
+  Assert.ok(!download.canceled);
+  Assert.ok(download.error !== null);
+  Assert.ok(download.error.becauseSourceFailed);
+  Assert.ok(!download.error.becauseTargetFailed);
+  Assert.equal(download.error.result, Cr.NS_ERROR_NET_RESET);
+
+  Assert.equal(false, await OS.File.exists(download.target.path));
+});
+
 /**
  * Ensures download error details are reported on local writing failures.
  */

toolkit/components/downloads/test/unit/head.js

@@ -11,6 +11,7 @@
 
 // Globals
 
+ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 ChromeUtils.import("resource://gre/modules/Integration.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
@@ -718,6 +719,20 @@ add_task(function test_common_initialize() {
                               "Blocked by Windows Parental Controls");
     });
 
+  // This URL sends some data followed by an RST packet
+  gHttpServer.registerPathHandler("/netreset.txt",
+    function(aRequest, aResponse) {
+      info("Starting response that will be aborted.");
+      aResponse.processAsync();
+      aResponse.setHeader("Content-Type", "text/plain", false);
+      aResponse.write(TEST_DATA_SHORT);
+      promiseExecuteSoon().then(() => {
+        aResponse.abort(null, true);
+        aResponse.finish();
+        info("Aborting response with network reset.");
+      }).then(null, Cu.reportError);
+    });
+
   // During unit tests, most of the functions that require profile access or
   // operating system features will be disabled. Individual tests may override
   // them again to check for specific behaviors.

toolkit/components/downloads/test/unit/test_DownloadPaths.js

@@ -5,8 +5,6 @@
  * Tests for the "DownloadPaths.jsm" JavaScript module.
  */
 
-ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
-
 function testSanitize(leafName, expectedLeafName) {
   Assert.equal(DownloadPaths.sanitize(leafName), expectedLeafName);
 }