Bug 748580 - websockets: omit close code when none passed to close(). r=mcmanus

content/base/test/file_websocket_wsh.py

@@ -31,6 +31,19 @@ def web_socket_do_extra_handshake(request):
   else:
     pass
 
+# Behave according to recommendation of RFC 6455, section # 5.5.1:
+#  "When sending a Close frame in response, the endpoint typically echos the
+#   status code it received."  
+# - Without this, pywebsocket replies with 1000 to any close code.
+#
+#  Note that this function is only called when the client initiates the close
+def web_socket_passive_closing_handshake(request):
+  if request.ws_close_code == 1005:
+    return None, None
+  else:
+    return request.ws_close_code, request.ws_close_reason
+
+
 def web_socket_transfer_data(request):
   if request.ws_protocol == "test-2.1" or request.ws_protocol == "test-2.2":
     msgutil.close_connection(request)
@@ -57,7 +70,6 @@ def web_socket_transfer_data(request):
     if msgutil.receive_message(request) == "client data":
       resp = "server data"
     msgutil.send_message(request, resp.decode('utf-8'))
-    msgutil.close_connection(request)
   elif request.ws_protocol == "test-12":
     msgutil.close_connection(request)
   elif request.ws_protocol == "test-13":

content/base/test/test_websocket.html

@@ -25,8 +25,8 @@
  *  5. client uses an invalid protocol value;
  *  6. counter and encoding check;
  *  7. onmessage event origin property check
- *  8. client calls close() and the server sends the close frame in
- *     acknowledgement;
+ *  8. client calls close() and the server sends the close frame (with no code
+ *     or reason) in acknowledgement;
  *  9. client closes the connection before the ws connection is established;
  * 10. client sends a message before the ws connection is established;
  * 11. a simple hello echo;
@@ -55,7 +55,7 @@
  * 31. ctor using valid 2 element sub-protocol array with 1 element server
  *     will reject and one server will accept.
  * 32. ctor using invalid sub-protocol array that contains duplicate items
- * 33. default close code test
+ * 33. test for sending/receiving custom close code (but no close reason)
  * 34. test for receiving custom close code and reason
  * 35. test for sending custom close code and reason
  * 36. negative test for sending out of range close code
@@ -387,6 +387,10 @@ function test8()
   ws.onclose = function(e)
   {
     shouldCloseCleanly(e);
+    // We called close() with no close code: so pywebsocket will also send no
+    // close code, which translates to code 1005 
+    ok(e.code == 1005, "test-8 close code has wrong value:" + e.code);
+    ok(e.reason == "", "test-8 close reason has wrong value:" + e.reason);
     doTest(9);
   };
 }
@@ -461,7 +465,7 @@ function test11()
   ws.onmessage = function(e)
   {
     ok(e.data == "server data", "bad received message in test-11!");
-    ws.close();
+    ws.close(1000, "Have a nice day");
 
 // this ok() is disabled due to a race condition - it state may have
 // advanced through 2 (closing) and into 3 (closed) before it is evald
@@ -471,6 +475,8 @@ function test11()
   {
     ok(ws.readyState == 3, "onclose bad readyState in test-11!");
     shouldCloseCleanly(e);
+    ok(e.code == 1000, "test 11 got wrong close code: " + e.code);
+    ok(e.reason == "Have a nice day", "test 11 got wrong close reason: " + e.reason);
     doTest(12);
   }
 }
@@ -942,14 +948,15 @@ function test33()
   ws.onopen = function(e)
   {
     ok(true, "test 33 open");
-    ws.close();
+    ws.close(3131);   // pass code but not reason
   };
 
   ws.onclose = function(e)
   {
     ok(true, "test 33 close");
-    ok(e.wasClean, "test 33 closed cleanly");
-    ok(e.code == 1000, "test 33 had normal 1000 error code");
+    shouldCloseCleanly(e);
+    ok(e.code == 3131, "test 33 got wrong close code: " + e.code);
+    ok(e.reason === "", "test 33 got wrong close reason: " + e.reason);
     doTest(34);
   };
 }

netwerk/protocol/websocket/WebSocketChannel.cpp

@@ -1374,31 +1374,38 @@ WebSocketChannel::PrimeNewOutgoingMessage()
 
     mClientClosed = 1;
     mOutHeader[0] = kFinalFragBit | kClose;
-    mOutHeader[1] = 0x02; // payload len = 2, maybe more for reason
-    mOutHeader[1] |= kMaskBit;
+    mOutHeader[1] = kMaskBit;
 
     // payload is offset 6 including 4 for the mask
     payload = mOutHeader + 6;
 
-    // length is 8 plus any reason information
-    mHdrOutToSend = 8;
-
     // The close reason code sits in the first 2 bytes of payload
     // If the channel user provided a code and reason during Close()
     // and there isn't an internal error, use that.
-    if (NS_SUCCEEDED(mStopOnClose) && mScriptCloseCode) {
-      *((PRUint16 *)payload) = PR_htons(mScriptCloseCode);
-      if (!mScriptCloseReason.IsEmpty()) {
-        NS_ABORT_IF_FALSE(mScriptCloseReason.Length() <= 123,
-                          "Close Reason Too Long");
-        mOutHeader[1] += mScriptCloseReason.Length();
-        mHdrOutToSend += mScriptCloseReason.Length();
-        memcpy (payload + 2,
-                mScriptCloseReason.BeginReading(),
-                mScriptCloseReason.Length());
+    if (NS_SUCCEEDED(mStopOnClose)) {
+      if (mScriptCloseCode) {
+        *((PRUint16 *)payload) = PR_htons(mScriptCloseCode);
+        mOutHeader[1] += 2;
+        mHdrOutToSend = 8;
+        if (!mScriptCloseReason.IsEmpty()) {
+          NS_ABORT_IF_FALSE(mScriptCloseReason.Length() <= 123,
+                            "Close Reason Too Long");
+          mOutHeader[1] += mScriptCloseReason.Length();
+          mHdrOutToSend += mScriptCloseReason.Length();
+          memcpy (payload + 2,
+                  mScriptCloseReason.BeginReading(),
+                  mScriptCloseReason.Length());
+        }
+      } else {
+        // No close code/reason, so payload length = 0.  We must still send mask
+        // even though it's not used.  Keep payload offset so we write mask
+        // below.
+        mHdrOutToSend = 6;
       }
     } else {
       *((PRUint16 *)payload) = PR_htons(ResultToCloseCode(mStopOnClose));
+      mOutHeader[1] += 2;
+      mHdrOutToSend = 8;
     }
 
     if (mServerClosed) {
@@ -1504,30 +1511,29 @@ WebSocketChannel::PrimeNewOutgoingMessage()
 
   ApplyMask(mask, mCurrentOut->BeginWriting(), mCurrentOut->Length());
 
+  PRInt32 len = mCurrentOut->Length();
+
   // for small frames, copy it all together for a contiguous write
-  if (mCurrentOut->Length() <= kCopyBreak) {
-    memcpy(mOutHeader + mHdrOutToSend, mCurrentOut->BeginWriting(),
-           mCurrentOut->Length());
-    mHdrOutToSend += mCurrentOut->Length();
-    mCurrentOutSent = mCurrentOut->Length();
+  if (len && len <= kCopyBreak) {
+    memcpy(mOutHeader + mHdrOutToSend, mCurrentOut->BeginWriting(), len);
+    mHdrOutToSend += len;
+    mCurrentOutSent = len;
   }
 
-  if (mCompressor) {
+  if (len && mCompressor) {
     // assume a 1/3 reduction in size for sizing the buffer
     // the buffer is used multiple times if necessary
     PRUint32 currentHeaderSize = mHdrOutToSend;
     mHdrOutToSend = 0;
 
-    EnsureHdrOut(32 +
-                 (currentHeaderSize + mCurrentOut->Length() - mCurrentOutSent)
-                 / 2 * 3);
+    EnsureHdrOut(32 + (currentHeaderSize + len - mCurrentOutSent) / 2 * 3);
     mCompressor->Deflate(mOutHeader, currentHeaderSize,
                          mCurrentOut->BeginReading() + mCurrentOutSent,
-                         mCurrentOut->Length() - mCurrentOutSent);
+                         len - mCurrentOutSent);
 
     // All of the compressed data now resides in {mHdrOut, mHdrOutToSend}
     // so do not send the body again
-    mCurrentOutSent = mCurrentOut->Length();
+    mCurrentOutSent = len;
   }
 
   // Transmitting begins - mHdrOutToSend bytes from mOutHeader and