[reSIProcate] [resip/stack][patch 1/2] Code cleanup for ConnectionBase.cxx

[Zhi An Ng <ngzhian@xxxxxxxxx>]

Hi all,

This patch removes an entire portion of wsProcessHandshake() that
deals with an deprecated protocol. [1]
There is also a bit of refactoring. Scanning the message header is its
own function now. Making the handshake response is also its own
function, which handles instances when a header can not be found.

[1] http://www.whatwg.org/specs/web-socket-protocol/

Best,
Zhi An

---diff starts below---
Index: ConnectionBase.cxx
===================================================================
--- ConnectionBase.cxx (revision 10202)
+++ ConnectionBase.cxx (working copy)
@@ -556,104 +556,30 @@
    mMessage->setSource(mWho);
    mMessage->setTlsDomain(mWho.transport->tlsDomain());

-   mMsgHeaderScanner.prepareForMessage(mMessage);
-   char *unprocessedCharPtr;
-   MsgHeaderScanner::ScanChunkResult scanResult =
mMsgHeaderScanner.scanChunk(mBuffer, mBufferPos + bytesRead,
&unprocessedCharPtr);
-   if (scanResult != MsgHeaderScanner::scrEnd)
-   {
-      if(scanResult != MsgHeaderScanner::scrNextChunk)
-      {
-         StackLog(<<"Failed to parse message, more bytes needed");
-         StackLog(<< Data(mBuffer, bytesRead));
-      }
-      delete mMessage;
-      mMessage=0;
-      mBufferPos += bytesRead;
+   if (!scanMsgHeader(bytesRead)) {
       return false;
    }

    try
    {
-      Data wsResponse;
-      wsResponse = "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
-            "Upgrade: WebSocket\r\n"
-            "Connection: Upgrade\r\n"
-            "Sec-WebSocket-Protocol: sip\r\n";
-
-      if(mMessage->exists(h_SecWebSocketKey1) &&
mMessage->exists(h_SecWebSocketKey2))
+      Data wsResponse = makeWsHandshakeResponse();
+      if (!wsResponse.empty())
       {
-         Data SecWebSocketKey1 =
mMessage->const_header(h_SecWebSocketKey1).value();
-         Data SecWebSocketKey2 =
mMessage->const_header(h_SecWebSocketKey2).value();
-         Data Digits1, Digits2;
-         unsigned int SpacesCount1 = 0, SpacesCount2 = 0;
-         unsigned int Value1, Value2;
-         for(unsigned int i = 0; i < SecWebSocketKey1.size(); ++i)
-         {
-            if(SecWebSocketKey1[i] == ' ') ++SpacesCount1;
-            if(isdigit(SecWebSocketKey1[i])) Digits1 += SecWebSocketKey1[i];
-         }
-         Value1 = htonl(Digits1.convertUnsignedLong() / SpacesCount1);
-         for(unsigned int i = 0; i < SecWebSocketKey2.size(); ++i)
-         {
-            if(SecWebSocketKey2[i] == ' ') ++SpacesCount2;
-            if(isdigit(SecWebSocketKey2[i])) Digits2 += SecWebSocketKey2[i];
-         }
-         Value2 = htonl(Digits2.convertUnsignedLong() / SpacesCount2);
-
-         MD5Stream wsMD5Stream;
-         char tmp[9] = { '\0' };
-         memcpy(tmp, &Value1, 4);
-         memcpy(&tmp[4], &Value2, 4);
-         wsMD5Stream << tmp;
-         if(unprocessedCharPtr < (mBuffer + mBufferPos + bytesRead))
-         {
-            unsigned int dataLen = (mBuffer + mBufferPos + bytesRead)
- unprocessedCharPtr;
-            Data content(unprocessedCharPtr, dataLen);
-            wsMD5Stream << content;
-         }
-
-         if(mMessage->exists(h_Origin))
-         {
-            wsResponse += "Sec-WebSocket-Origin: " +
mMessage->const_header(h_Origin).value() + "\r\n";
-         }
-         if(mMessage->exists(h_Host))
-         {
-            wsResponse += Data("Sec-WebSocket-Location: ") +
Data(transport()->transport() == resip::WSS ? "wss://" : "ws://") +
mMessage->const_header(h_Host).value() + Data("/\r\n");
-         }
-         wsResponse += "\r\n" + wsMD5Stream.getBin();
-         ErrLog(<<"WS client wants to use depracated protocol
version, unsupported");
-         delete mMessage;
-         mMessage = 0;
-         mBufferPos = 0;
-         dropConnection = true;
-         return false;
+         mOutstandingSends.push_back(new SendData(
+                  who(),
+                  wsResponse,
+                  Data::Empty,
+                  Data::Empty,
+                  true));
       }
-      else if(mMessage->exists(h_SecWebSocketKey))
-      {
-#ifdef USE_SSL
-         SHA1Stream wsSha1Stream;
-         wsSha1Stream <<
(mMessage->const_header(h_SecWebSocketKey).value() +
Data("258EAFA5-E914-47DA-95CA-C5AB0DC85B11"));
-         Data wsAcceptKey = wsSha1Stream.getBin(160).base64encode();
-         wsResponse += "Sec-WebSocket-Accept: "+ wsAcceptKey +"\r\n"
-               "\r\n";
-#endif
-      }
       else
       {
-         ErrLog(<<"No SecWebSocketKey header");
+         dropConnection = true;
          delete mMessage;
          mMessage = 0;
          mBufferPos = 0;
-         dropConnection = true;
          return false;
       }
-
-      mOutstandingSends.push_back(new SendData(
-            who(),
-            wsResponse,
-            Data::Empty,
-            Data::Empty,
-            true));
    }
    catch(resip::ParseException& e)
    {
@@ -661,7 +587,6 @@
       delete mMessage;
       mMessage = 0;
       mBufferPos = 0;
-      dropConnection = true;
       return false;
    }

@@ -672,7 +597,67 @@
    return true;
 }

+Data
+ConnectionBase::makeWsHandshakeResponse()
+{
+   Data response = "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
+      "Upgrade: WebSocket\r\n"
+      "Connection: Upgrade\r\n"
+      "Sec-WebSocket-Protocol: sip\r\n";
+
+   if(isUsingSecWebSocketKey())
+   {
+#ifdef USE_SSL
+      SHA1Stream wsSha1Stream;
+      wsSha1Stream <<
(mMessage->const_header(h_SecWebSocketKey).value() +
Data("258EAFA5-E914-47DA-95CA-C5AB0DC85B11"));
+      Data wsAcceptKey = wsSha1Stream.getBin(160).base64encode();
+      response += "Sec-WebSocket-Accept: " + wsAcceptKey + "\r\n\r\n";
+#endif
+      return response;
+   }
+   else if(isUsingDeprecatedSecWebSocketKeys())
+   {
+      ErrLog(<<"WS client wants to use depracated protocol version,
unsupported");
+   }
+   else
+   {
+      ErrLog(<<"No SecWebSocketKey header");
+   }
+   return NULL;
+}
+
 bool
+ConnectionBase::scanMsgHeader(int bytesRead)
+{
+   mMsgHeaderScanner.prepareForMessage(mMessage);
+   char *unprocessedCharPtr;
+   MsgHeaderScanner::ScanChunkResult scanResult =
mMsgHeaderScanner.scanChunk(mBuffer, mBufferPos + bytesRead,
&unprocessedCharPtr);
+   if (scanResult != MsgHeaderScanner::scrEnd)
+   {
+      if(scanResult != MsgHeaderScanner::scrNextChunk)
+      {
+         StackLog(<<"Failed to parse message, more bytes needed");
+         StackLog(<< Data(mBuffer, bytesRead));
+      }
+      delete mMessage;
+      mMessage=0;
+      mBufferPos += bytesRead;
+      return false;
+   }
+}
+
+bool ConnectionBase::isUsingDeprecatedSecWebSocketKeys()
+{
+   return mMessage->exists(h_SecWebSocketKey1) &&
+      mMessage->exists(h_SecWebSocketKey2);
+}
+
+bool ConnectionBase::isUsingSecWebSocketKey()
+{
+   return mMessage->exists(h_SecWebSocketKey);
+}
+
+bool
 ConnectionBase::wsProcessData(int bytesRead)
 {
    bool dropConnection = false;