1
\$\begingroup\$

I can recently coded an emulation system and in this project I have a PacketHandler for each player connected to a socket server, and each player receives packets (data) from the client application.

Can someone please let me know if this code can be improved in any way? I will post the code below for the PlayerPacketHandler class, if you need any other classes that are used in this class let me know and I can add them.

Thanks.

How is ProcessPacketData called?

private void IncomingPacket(IAsyncResult iAr)
{
    try
    {
        var bytesReceived = _playerSocket.EndReceive(iAr);

        if (bytesReceived == 0)
        {
            Dispose();
            return;
        }

        var packet = new byte[bytesReceived];
        Array.Copy(_buffer, packet, bytesReceived);
        _playerPacketHandler.ProcessPacketData(packet);
    }
    catch (Exception)
    {
        Dispose();
    }
    finally
    {
        try
        {
            _playerSocket.BeginReceive(_buffer, 0, _buffer.Length, SocketFlags.None, IncomingPacket, _playerSocket);
        }
        catch
        {
            Dispose();
        }
    }
}

The class I am asking to be reviewed:

using System;
using System.IO;
using Sahara.Core.Logging;

namespace Sahara.Base.Game.Players
{
    internal class PlayerPacketHandler
    {
        private readonly Player _player;
        private readonly LogManager _logManager;
        private bool _decryptedData;
        private byte[] _dataReceived;
        private bool _dataCompleted;

        public PlayerPacketHandler(Player player)
        {
            _player = player;
            _logManager = Sahara.GetServer().GetLogManager();
        }

        public void ProcessPacketData(byte[] data)
        {
            try
            {
                if (_player.RC4Client != null && !_decryptedData)
                {
                    _player.RC4Client.Decrypt(ref data);
                    _decryptedData = true;
                }

                if (_dataCompleted)
                {
                    byte[] fullDataRcv = new byte[_dataReceived.Length + data.Length];
                    Buffer.BlockCopy(_dataReceived, 0, fullDataRcv, 0, _dataReceived.Length);
                    Buffer.BlockCopy(data, 0, fullDataRcv, _dataReceived.Length, data.Length);

                    ProcessPacketData(fullDataRcv);
                    _dataCompleted = false;

                    return;
                }

                using (var reader = new BinaryReader(new MemoryStream(data)))
                {
                    if (data.Length < 4)
                        return;

                    int messageLength = HabboEncoding.DecodeInt32(reader.ReadBytes(4));

                    if ((reader.BaseStream.Length - 4) < messageLength)
                    {
                        _dataReceived = data;
                        _dataCompleted = true;
                        return;
                    }

                    if (messageLength < 0 || messageLength > 5120)
                        return;

                    var packet = reader.ReadBytes(messageLength);

                    using (var binaryReader = new BinaryReader(new MemoryStream(packet)))
                    {
                        int packetHeader = HabboEncoding.DecodeInt16(binaryReader.ReadBytes(2));

                        byte[] Content = new byte[packet.Length - 2];
                        Buffer.BlockCopy(packet, 2, Content, 0, packet.Length - 2);

                        IncomingPacket clientPacket = new IncomingPacket(packetHeader, Content);
                        onNewPacket.Invoke(clientPacket);

                        _decryptedData = false;
                    }
                }
            }
            catch (Exception exception)
            {
                var method = System.Reflection.MethodBase.GetCurrentMethod().Name;
                _logManager.Log("Error in " + method + ": " + exception.Message, LogType.Error);
                _logManager.Log(exception.StackTrace, LogType.Error);
            }
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ IncomingPacket calls Dispose but continues nevertheless with accepting data. Unfortunately, the code of this class is missing, so we cannot review it. This question is off-topic because it lacks sufficient context to review thoroughly. \$\endgroup\$ – dfhwze Aug 21 '19 at 19:08
-1
\$\begingroup\$

My two cents : MemoryStream is a disposable object (inherited from Stream)

using (var reader = new BinaryReader(new MemoryStream(data)))

should grant you with a warning CA2000.

You may check this point with something like

Stream stream = null ;
try
{
   Stream = new MemoryStream(data) ;
   using (var reader = new BinaryReader(stream))
...
}
finally
{
    if (stream != null)
    {
        stream.Dispose() ;
    }
}

This should ensure you are freeing the stream in time.

Hope this helps !

| improve this answer | |
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.