tags:

views:

248

answers:

1

Hey, I'm having an issue seperating packets using a custom binary protocol. Currently the server side code looks like this.

    public void HandleConnection(object state)
    {
        TcpClient client = threadListener.AcceptTcpClient();
        NetworkStream stream = client.GetStream();
        byte[] data = new byte[4096];

        while (true)
        {
            int recvCount = stream.Read(data, 0, data.Length);
            if (recvCount == 0) break;
            LogManager.Debug(Utility.ToHexDump(data, 0, recvCount));
            //processPacket(new MemoryStream(data, 0, recvCount));
        }
        LogManager.Debug("Client disconnected");
        client.Close();
        Dispose();
    }

I've been watching the hex dumps of the packets, and sometimes the entire packet comes in one shot, let's say all 20 bytes. Other times it comes in fragmented, how do I need to buffer this data to be able to pass it to my processPacket() method correctly. I'm attempting to use a single byte opcode header only, should I add something like a (ushort)contentLength to the header aswell? I'm trying to make the protocol as lightweight as possible, and this system won't be sending very large packets(< 128 bytes).

The client side code I'm testing with is, as follows.

    public void auth(string user, string password)
    {
        using (TcpClient client = new TcpClient())
        {
            client.Connect(IPAddress.Parse("127.0.0.1"), 9032);
            NetworkStream networkStream = client.GetStream();

            using (BinaryWriter writer = new BinaryWriter(networkStream))
            {
                writer.Write((byte)0); //opcode
                writer.Write(user.ToUpper());
                writer.Write(password.ToUpper());
                writer.Write(SanitizationMgr.Verify()); //App hash
                writer.Write(Program.Seed);
            }
        }
    }

I'm not sure if that could be what's messing it up, and binary protocol doesn't seem to have much info on the web, especially where C# is involved. Any comment's would be helpful. =)

Solved with this, not sure if it's correct, but it seems to give my handlers just what they need.

    public void HandleConnection(object state)
    {
        TcpClient client = threadListener.AcceptTcpClient();
        NetworkStream stream = client.GetStream();
        byte[] data = new byte[1024];

        uint contentLength = 0;
        var packet = new MemoryStream();
        while (true)
        {
            int recvCount = stream.Read(data, 0, data.Length);
            if (recvCount == 0) break;

            if (contentLength == 0 && recvCount < headerSize)
            {
                LogManager.Error("Got incomplete header!");
                Dispose();
            }

            if(contentLength == 0) //Get the payload length
                contentLength = BitConverter.ToUInt16(data, 1);

            packet.Write(data, (int) packet.Position, recvCount); //Buffer the data we got into our MemStream
            if (packet.Length < contentLength + headerSize) //if it's not enough, continue trying to read
                continue;

            //We have a full packet, pass it on
            //LogManager.Debug(Utility.ToHexDump(packet));
            processPacket(packet);

            //reset for next packet
            contentLength = 0;
            packet = new MemoryStream();
        }
        LogManager.Debug("Client disconnected");
        client.Close();
        Dispose();
    }
A: 

You should just treat it as a stream. Don't rely on any particular chunking behaviour.

Is the amount of data you need always the same? If not, you should change the protocol (if you can) to prefix the logical "chunk" of data with the length in bytes.

In this case you're using BinaryWriter on one side, so attaching a BinaryReader to the NetworkStream returned by TcpClient.GetStream() would seem like the easiest approach. If you really want to capture all the data for a chunk at a time though, you should go back to my idea of prefixing the data with its length. Then just loop round until you've got all the data.

(Make sure you've got enough data to read the length though! If your length prefix is 4 bytes, you don't want to read 2 bytes and miss the next 2...)

Jon Skeet
Hmm, I will add a the contentLength to the header, thanks. I'm using a binary reader in processPacket(), but obviously that's failing horribly when it's only being passed a partial packet.
Endian
@Endian: Why don't you just pass it the network stream instead of a packet?
Jon Skeet
@Jon: Packet more in the sense of a chunk that my binaryReader can read without getting a buffer underrun. I've edited the original post with what I'm doing now, after your suggestion and it seems to be working, thanks for the help. :)
Endian