Bug 372486 – Document and enforce that channels can't be reopenedpatch by Sylvain Pasche <sylvain.pasche@gmail.com> r=biesi sr=darin

netwerk/base/public/nsIChannel.idl

@@ -165,6 +165,9 @@ interface nsIChannel : nsIRequest
      * NOTE: nsIChannel implementations are not required to implement this
      * method.  Moreover, since this method may block the calling thread, it
      * should not be called on a thread that processes UI events.
+     *
+     * NOTE: Implementations should throw NS_ERROR_IN_PROGRESS if the channel
+     * is reopened.
      */
     nsIInputStream open();
 
@@ -189,6 +192,9 @@ interface nsIChannel : nsIRequest
      * keeping itself alive until it has called onStopRequest on aListener or
      * called onChannelRedirect.
      *
+     * NOTE: Implementations should throw NS_ERROR_ALREADY_OPENED if the
+     * channel is reopened.
+     *
      * @param aListener the nsIStreamListener implementation
      * @param aContext an opaque parameter forwarded to aListener's methods
      * @see nsIChannelEventSink for onChannelRedirect

netwerk/base/public/nsNetError.h

@@ -130,6 +130,13 @@
 #define NS_ERROR_IN_PROGRESS \
     NS_ERROR_GENERATE_FAILURE(NS_ERROR_MODULE_NETWORK, 15)
 
+/**
+ * Returned from nsIChannel::asyncOpen when trying to open the channel again
+ * (reopening is not supported).
+ */
+#define NS_ERROR_ALREADY_OPENED \
+    NS_ERROR_GENERATE_FAILURE(NS_ERROR_MODULE_NETWORK, 73)
+
 /**
  * The content encoding of the source document was incorrect, for example
  * returning a plain HTML document advertised as Content-Encoding: gzip

netwerk/base/src/nsBaseChannel.cpp

@@ -88,6 +88,7 @@ nsBaseChannel::nsBaseChannel()
   , mStatus(NS_OK)
   , mQueriedProgressSink(PR_TRUE)
   , mSynthProgressEvents(PR_FALSE)
+  , mWasOpened(PR_FALSE)
 {
   mContentType.AssignLiteral(UNKNOWN_CONTENT_TYPE);
 }
@@ -448,11 +449,14 @@ nsBaseChannel::Open(nsIInputStream **result)
 {
   NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_INITIALIZED);
   NS_ENSURE_TRUE(!mPump, NS_ERROR_IN_PROGRESS);
+  NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_IN_PROGRESS);
 
   nsresult rv = OpenContentStream(PR_FALSE, result);
   if (rv == NS_ERROR_NOT_IMPLEMENTED)
     return NS_ImplementChannelOpen(this, result);
 
+  mWasOpened = NS_SUCCEEDED(rv);
+
   return rv;
 }
 
@@ -461,6 +465,7 @@ nsBaseChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *ctxt)
 {
   NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_INITIALIZED);
   NS_ENSURE_TRUE(!mPump, NS_ERROR_IN_PROGRESS);
+  NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_ALREADY_OPENED);
   NS_ENSURE_ARG(listener);
 
   // Ensure that this is an allowed port before proceeding.
@@ -488,6 +493,8 @@ nsBaseChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *ctxt)
 
   // At this point, we are going to return success no matter what.
 
+  mWasOpened = PR_TRUE;
+
   SUSPEND_PUMP_FOR_SCOPE();
 
   if (mLoadGroup)

netwerk/base/src/nsBaseChannel.h

@@ -249,6 +249,7 @@ private:
   nsresult                            mStatus;
   PRPackedBool                        mQueriedProgressSink;
   PRPackedBool                        mSynthProgressEvents;
+  PRPackedBool                        mWasOpened;
 };
 
 #endif // !nsBaseChannel_h__

netwerk/base/src/nsBaseContentStream.cpp

@@ -74,6 +74,7 @@ NS_IMPL_THREADSAFE_RELEASE(nsBaseContentStream)
 NS_INTERFACE_MAP_BEGIN(nsBaseContentStream)
   NS_INTERFACE_MAP_ENTRY(nsIInputStream)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIAsyncInputStream, mNonBlocking)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIInputStream)
 NS_INTERFACE_MAP_END_THREADSAFE
 
 //-----------------------------------------------------------------------------

netwerk/protocol/http/src/nsHttpChannel.cpp

@@ -106,6 +106,7 @@ nsHttpChannel::nsHttpChannel()
     , mStartPos(LL_MAXUINT)
     , mRedirectionLimit(gHttpHandler->RedirectionLimit())
     , mIsPending(PR_FALSE)
+    , mWasOpened(PR_FALSE)
     , mApplyConversion(PR_TRUE)
     , mAllowPipelining(PR_TRUE)
     , mCachedContentIsValid(PR_FALSE)
@@ -3501,6 +3502,7 @@ nsHttpChannel::SetContentLength(PRInt32 value)
 NS_IMETHODIMP
 nsHttpChannel::Open(nsIInputStream **_retval)
 {
+    NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_IN_PROGRESS);
     return NS_ImplementChannelOpen(this, _retval);
 }
 
@@ -3511,6 +3513,7 @@ nsHttpChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *context)
 
     NS_ENSURE_ARG_POINTER(listener);
     NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
+    NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_ALREADY_OPENED);
 
     nsresult rv;
 
@@ -3536,6 +3539,7 @@ nsHttpChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *context)
         mCaps &= ~(NS_HTTP_ALLOW_KEEPALIVE | NS_HTTP_ALLOW_PIPELINING);
     
     mIsPending = PR_TRUE;
+    mWasOpened = PR_TRUE;
 
     mListener = listener;
     mListenerContext = context;

netwerk/protocol/http/src/nsHttpChannel.h

@@ -273,6 +273,7 @@ private:
 
     // state flags
     PRUint32                          mIsPending                : 1;
+    PRUint32                          mWasOpened                : 1;
     PRUint32                          mApplyConversion          : 1;
     PRUint32                          mAllowPipelining          : 1;
     PRUint32                          mCachedContentIsValid     : 1;

netwerk/test/unit/test_reopen.js

@@ -0,0 +1,146 @@
+// This testcase verifies that channels can't be reopened
+// See https://bugzilla.mozilla.org/show_bug.cgi?id=372486
+
+do_import_script("netwerk/test/httpserver/httpd.js");
+
+const NS_ERROR_IN_PROGRESS = 0x804b000f;
+const NS_ERROR_ALREADY_OPENED = 0x804b0049;
+
+var chan = null;
+var httpserv = null;
+
+var test_index = 0;
+var test_array = [
+  test_data_channel,
+  test_http_channel,
+  test_file_channel,
+  // Commented by default as it relies on external ressources
+  //test_ftp_channel,
+  end
+];
+
+// Utility functions
+
+var BinaryInputStream =
+          Components.Constructor("@mozilla.org/binaryinputstream;1",
+                                 "nsIBinaryInputStream", "setInputStream");
+
+function makeChan(url) {
+  var ios = Cc["@mozilla.org/network/io-service;1"]
+              .getService(Ci.nsIIOService);
+  return chan = ios.newChannel(url, null, null)
+                   .QueryInterface(Ci.nsIChannel);
+}
+
+function new_file_channel(file) {
+  var ios = Cc["@mozilla.org/network/io-service;1"]
+              .getService(Ci.nsIIOService);
+  return ios.newChannelFromURI(ios.newFileURI(file));
+}
+
+
+function check_throws(closure, error) {
+  var thrown = false;
+  try {
+    closure();
+  } catch (e) {
+    do_check_eq(e.result, error);
+    thrown = true;
+  }
+  do_check_true(thrown);
+}
+
+function check_open_throws(error) {
+  check_throws(function() {
+    chan.open(listener, null);
+  }, error);
+}
+
+function check_async_open_throws(error) {
+  check_throws(function() {
+    chan.asyncOpen(listener, null);
+  }, error);
+}
+
+var listener = {
+  onStartRequest: function test_onStartR(request, ctx) {
+    check_async_open_throws(NS_ERROR_IN_PROGRESS);
+  },
+
+  onDataAvailable: function test_ODA(request, cx, inputStream,
+                                     offset, count) {
+    new BinaryInputStream(inputStream).readByteArray(count); // required by API
+    check_async_open_throws(NS_ERROR_IN_PROGRESS);
+  },
+
+  onStopRequest: function test_onStopR(request, ctx, status) {
+    // Once onStopRequest is reached, the channel is marked as having been
+    // opened
+    check_async_open_throws(NS_ERROR_ALREADY_OPENED);
+    do_timeout(0, "after_channel_closed()");
+  }
+};
+
+function after_channel_closed() {
+  check_async_open_throws(NS_ERROR_ALREADY_OPENED);
+
+  run_next_test();
+}
+
+function run_next_test() {
+  test_array[test_index++]();
+}
+
+function test_channel(createChanClosure) {
+  // First, synchronous reopening test
+  chan = createChanClosure();
+  var inputStream = chan.open();
+  check_open_throws(NS_ERROR_IN_PROGRESS);
+  check_async_open_throws(NS_ERROR_ALREADY_OPENED);
+  
+  // Then, asynchronous one
+  chan = createChanClosure();
+  chan.asyncOpen(listener, null);
+  check_open_throws(NS_ERROR_IN_PROGRESS);
+  check_async_open_throws(NS_ERROR_IN_PROGRESS);
+}
+
+function test_data_channel() {
+  test_channel(function() {
+    return makeChan("data:text/plain,foo");
+  });
+}
+
+function test_http_channel() {
+  test_channel(function() {
+    return makeChan("http://localhost:4444/");
+  });
+}
+
+function test_file_channel() {
+  var file = do_get_file("netwerk/test/Makefile.in");
+  test_channel(function() {
+    return new_file_channel(file);
+  });
+}
+
+// Uncomment test_ftp_channel in test_array to test this
+function test_ftp_channel() {
+  test_channel(function() {
+    return makeChan("ftp://ftp.mozilla.org/pub/mozilla.org/README");
+  });
+}
+
+function end() {
+  httpserv.stop();
+  do_test_finished();
+}
+
+function run_test() {
+  // start server
+  httpserv = new nsHttpServer();
+  httpserv.start(4444);
+  
+  do_test_pending();
+  run_next_test();
+}