Strange packets arrive on C# websocket server from Chrome websocket client

Question

I have a C# server side websocket code, and I am sending test data from Chrome (javascript websocket client).

Following this: https://stackoverflow.com/a/8125509/2508439, and some other link, I created my C# webserver (server side) decode function as following:

public String DecodeMessage(Byte[] bytes)
{
    Console.WriteLine("+DecodeMessage+");
    String incomingData = String.Empty;
    Byte secondByte = bytes[1];
    bool masked = (bytes[1] & 128) != 0;
    //long dataLength = secondByte & 127;
    dataLength = secondByte & 127;
    //Int32 indexFirstMask = 2;
    indexFirstMask = 2;
    if (masked)
    {
        Console.WriteLine("Masked bit SET");
    }
    if (dataLength == 126)
    {
        indexFirstMask = 4;
        dataLength = bytes[3] | bytes[2] << 8;
    }
    else if (dataLength == 127)
    {
        indexFirstMask = 10;
        dataLength = bytes[9] | bytes[8] << 8 | bytes[7] << 16 | bytes[6] << 24 | bytes[5] << 32 |
            bytes[4] << 40 | bytes[3] << 48 | bytes[2] << 56;
    }

    //IEnumerable<Byte> keys = bytes.Skip(indexFirstMask).Take(4);
    keys = bytes.Skip(indexFirstMask).Take(4);
    Int32 indexFirstDataByte = indexFirstMask + 4;

    Byte[] decoded = new Byte[bytes.Length - indexFirstDataByte];
    Console.WriteLine("dataLength : " + dataLength + " ; bytes.Length : " + bytes.Length);
    Int32 j = 0;
    for (Int32 i = indexFirstDataByte; i < bytes.Length; i++)
    {
        decoded[j] = (Byte)(bytes[i] ^ keys.ElementAt(j % 4));
        j++;
    }
    Console.WriteLine("-DecodeMessage-");

    return incomingData = Encoding.UTF8.GetString(decoded, 0, decoded.Length);
}
public String DecodeRemainingMessage(Byte[] bytes, long bytesAlreadyRead)
{
    Console.WriteLine("+DecodeRemainingMessage+");
    String incomingData = String.Empty;

    Int32 indexFirstDataByte = 0;
    if ( indexFirstMask == 10 )//special case, what to do here?
    {
        indexFirstDataByte = 10;
    }

    Byte[] decoded = new Byte[bytes.Length - indexFirstDataByte];
    //Byte[] decoded = new Byte[bytes.Length];
    Int32 j = 0;
    for (Int32 i = indexFirstDataByte; i < bytes.Length; i++)
    {
        decoded[j] = (Byte)(bytes[i] ^ keys.ElementAt(j % 4));
        j++;
    }
    Console.WriteLine("-DecodeRemainingMessage-");

    return incomingData = Encoding.UTF8.GetString(decoded, 0, decoded.Length);
}

Simple packets (125 size or less arrive just fine).

In case of size about 125 and less than 65535, also arrive fine (kind of: there is some detail but I'm not going in that right now [*]).

Packets above 65535: the whole decode function goes crazy:

Only the first time packet is decoded properly, and after that, what ever data I receive is totally binary (unreadable), and after the first packet arrives, in consecutive packets:

if (dataLength == 126)
{
    ...
}
else if (dataLength == 127) ...

both conditions are never fulfilled, and dataLength is always less than 126, which is then decoded as (small) packet, and hence never reconstructed properly.

Can anyone highlight what I may be doing wrong?

Thanks

[*]=> data below 65535 length sometimes comes in more than two packets, which then behaves the same way as the larger packets, and packets after the first time this function is hit never gets reconstructed again properly.

edit 1: @Marc

Based on your comment, I have put the 'masked bit check' in above function, and I can see it is always set to '1' (as expected since this is only server side code for now).

I am also parsing the control frame in a different function, and in this function, provided my code is correct, I may be getting lots of junk data.

To elaborate, please see these functions below:

whole logical code:

The enum:

public enum ControlFrame { NA=0, CloseConnection=1, Ping=2, Pong=4, Text=8, Binary=16, ContinueFrame =32, FinalFrame=64 };

The parse control frame function:

private int ParseControlFrame(byte controlFrame)
{
    int rv = (int)ControlFrame.NA;
    bool isFinalFrame = (controlFrame & 0x80) == 0x80 ;
    byte opCode = (byte)((controlFrame & 0x0F));
    if ( opCode >= 0x3 && opCode <= 0x7    ||
        opCode >= 0xB && opCode <= 0xF    )//special frame, ignore it
    {
        Console.WriteLine("Reserved Frame received");
        return rv;
    }
    if (opCode == 0x8 || opCode == 0x0 || opCode == 0x1 || opCode == 0x2 || opCode == 0x9 || opCode == 0xA) //proceed furter
    {
        if (opCode == 0x0) //continue frame
        {
            rv |= (int)ControlFrame.ContinueFrame;
            Console.WriteLine("Continue Frame received");
        }
        if (opCode == 0x1) //text frame
        {
            rv |= (int)ControlFrame.Text;
            Console.WriteLine("Text Frame received");
        }
        if (opCode == 0x2) //binary frame
        {
            rv |= (int)ControlFrame.Binary;
            Console.WriteLine("Binary frame received");
        }
        if (opCode == 0x8) //connection closed
        {
            rv |= (int)ControlFrame.CloseConnection;
            Console.WriteLine("CloseConnection Frame received");
        } 
        if (opCode == 0x9) //ping
        {
            rv |= (int)ControlFrame.Ping;
            Console.WriteLine("PING received");
        }
        if (opCode == 0xA) //pong
        {
            rv |= (int)ControlFrame.Pong;
            Console.WriteLine("PONG received");
        }
    }
    else // invalid control bit, must close the connection
    {
        Console.WriteLine("invalid control frame received, must close connection");
        rv = (int)ControlFrame.CloseConnection;
    }
    if (isFinalFrame) //Final frame ...
    {
        rv |= (int)ControlFrame.FinalFrame;
        Console.WriteLine("Final frame received");
    }
    //else
    //{
    //    rv |= (int)ControlFrame.ContinueFrame;
    //    Console.WriteLine("Continue frame received");
    //}
    return rv;
}

Logical flow (code snippet from actual):

if (stream.DataAvailable)
{
    long bytesAlreadyRead = 0;
    bool breakMain = false;
    while (client.Available > 0 )
    {
        byte[] bytes = new byte[client.Available];

        stream.Read(bytes, 0, bytes.Length);

        Console.WriteLine("if (stream.DataAvailable):\nclient.Available : " + client.Available +
            " ; bytes.Length : " + bytes.Length);
        //translate bytes of request to string
        String data = Encoding.UTF8.GetString(bytes);
        Console.WriteLine("Message received on: " + DateTime.Now);

        if (bytesAlreadyRead == 0)
        {
            int controlFrame = ParseControlFrame(bytes[0]);
            if (controlFrame == (int)ControlFrame.NA ||
                (int)(controlFrame & (int)ControlFrame.Ping) > 0 ||
                (int)(controlFrame & (int)ControlFrame.Pong) > 0)   //ignore it
            {
            }
            else
            {
                if ((int)(controlFrame & (int)ControlFrame.CloseConnection) > 0)
                {
                    Console.WriteLine("Connection #" + c.Key + " Closed on: " + DateTime.Now);
                    tcpClients.Remove(c.Key);
                    breakMain = true;
                    break;
                }
                else
                {
                    string result = c.Value.DecodeMessage(bytes);
                    File.WriteAllText("recvfile.txt", result);
                }
            }
        }
        else
        {
            string result = c.Value.DecodeRemainingMessage(bytes, bytesAlreadyRead);
            File.AppendAllText("recvfile.txt", "\n");
            File.AppendAllText("recvfile.txt", result);
        }
        bytesAlreadyRead += bytes.Length;
    }
    if ( breakMain == true )
    {
        break;
    }
}

I don't get garbage but data is missed.

If I don't put this check, then, I start receiving garbage.

Based on Console.WriteLine output, I see something similar for data less than 65535:

Message received on: 12/29/2017 12:59:00 PM
Text Frame received
Final frame received
Masked bit SET
Message received on: 12/29/2017 12:59:12 PM
Text Frame received
Final frame received
Masked bit SET

For data above 65535:

Message received on: 12/29/2017 1:02:51 PM
Text Frame received
Final frame received
Masked bit SET
Message received on: 12/29/2017 1:02:51 PM
Reserved Frame received

i.e. Less than 65535, I am ok (most of the time).

With above 65535, things get strange.

When you mentioned:

I wonder if what is happening is that you're getting multiple messages in a single Read call (perfectly normal in TCP), consuming the first message, and incorrectly treating the entire bytes as consumed.

I never thought of this before, maybe I need to handle this somehow as well?

edit 2:

Based on your comment, i have modified the 'if (stream.DataAvailable)' logic, so it keeps on reading data in while loop until all data stored locally is actually flushed out.

So i may be close to solving it (thanks for your feedback), but the 2nd time DecodeMessage() function is called, it still decrypts in garbage (binary) data.

I am working on figuring it out!

Thanks

edit 3:

Ok, based on your suggestion, I have sorted out most of the logic. However, special case in 'DecodeRemainingMessage' function is what still remains a mystery. [I shifted some variables from local to class scope, hence they are commented out in functions]...

If I got it correct, I should not need to place any special condition here, but in that case, I still receive garbage.

Any pointers?

[sorry for the messy code, will update it once I get the right picture!]

Thanks

edit 4: Following all your advises in comments/chat helped me get to the point where I updated decode logic greatly, and was still unable to get right data in case of above 65535 bytes. But when I tried final logic with Firefox, I got all the data properly! So many thanks to you, and, I still need to figure out how to handle buggy Chrome client! Thanks!!

Answer 1

Edit: your code assumes that the incoming frames are always masked. This is probably OK if your code is only ever a server, but you might want to check whether bytes[1] & 128 is set (masked) or clear (not masked). If it isn't masked: the header is 4 bytes shorter. You should be OK if this is only ever a server, as (from 5.2 in RFC6455):

All frames sent from client to server have this bit set to 1.

but: it would be good to double-check. You'd be amazed how many buggy clients there are in the wild that violate the specification.

--

The overall code looks fine, and is broadly comparable to what I have here. I can't see anything immediately wrong. This makes me suspect that the issue here is TCP streaming; it isn't obvious that you method is doing anything to report back what quantity of bytes should be logically consumed by this frame - i.e. the total header length plus the payload length. For comparison to my code, this would be out int headerLength and frame.PayloadLength combined.

I wonder if what is happening is that you're getting multiple messages in a single Read call (perfectly normal in TCP), consuming the first message, and incorrectly treating the entire bytes as consumed. This would mean that you start reading in the wrong place for the next frame header. A single Read invoke can return a fragment of one frame, exactly one frame, or more than one frame - the only thing it can't return is 0 bytes, unless the socket has closed.