C# Socket: Client Mishandle 'a' as the Client's id

Question

There are two programs that I made that didn't work. There are the server and the client. The server accepts many client by giving a user an ID (starting from 0). The server sends out the command to the specific client based up the server's id. (Example: 200 client are connected to 1 server. The server's selected id is '5', so the server will send the command to all of the client, and the client will ask the server what ID he wants to execute his command on, and if it's '5', that client will execute and send data to the server). The client has many commands, but to create the smallest code with the same error, I only use 1 command (dir). Basically, the server sends the command to the client and if it matches with the client current id and the server current id, it will process the command. By default, the server's current ID is 10. Here are the list of the commands to help the people who wants to answer:

Server Command:

list (Shows all of the users ID connected and the server's current ID) --> Happens on server
dir (request the client to send its dir listing) --> Sent by the client, read by the Server
set (set the server's current id to any number) (example: 'set 4')

Client:

using System;
using System.Speech.Synthesis;
using System.Windows.Forms;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Net.Sockets;
using System.Net;
namespace clientControl
{
    class Program
    {
        public static string directory = @"C:\";
        public static int id = -10;
        public static Socket sck = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
        static void Main(string[] args)
        {
            Connect();
            getSession();
            ReadResponse(sck);
        }
        static byte[] readResponseFunc()
        {
            long fileSize = 0;
            string fileSizeInString = null;
            byte[] fileSizeInByteArray = new byte[1024];
            int fileSizeLength = sck.Receive(fileSizeInByteArray);
            for (int i = 0; i < fileSizeLength; i++)
            {
                fileSizeInString = fileSizeInString + (char)fileSizeInByteArray[i];
            }
            try
            {
                fileSize = Convert.ToInt64(fileSizeInString);
            }
            catch { Console.WriteLine(fileSizeInString); }
            sck.Send(Encoding.ASCII.GetBytes("a"));

            byte[] responseUnknown = new byte[1];
            sck.Receive(responseUnknown);
            if (Encoding.ASCII.GetString(responseUnknown) == "b")
            {
                byte[] dataInByteArray = new byte[fileSize];
                int dataLength = sck.Receive(dataInByteArray);
                return dataInByteArray;
            }
            return Encoding.ASCII.GetBytes("ERROR RESPONSE FUNC");
        }
        static void getSession()
        {
            byte[] message_1 = Encoding.ASCII.GetBytes("make_id");
            sck.Send(Encoding.ASCII.GetBytes(message_1.Length.ToString()));
            byte[] responseUnknown = new byte[1];
            if (Encoding.ASCII.GetString(responseUnknown) == "a")
            {
                sck.Send(Encoding.ASCII.GetBytes("b"));
                sck.Send(message_1);
            }
            byte[] receivedID = readResponseFunc();
            id = Convert.ToInt32(Encoding.ASCII.GetString(receivedID));
        }
        static bool SocketConnected(Socket s)
        {
            bool part1 = s.Poll(1000, SelectMode.SelectRead);
            bool part2 = (s.Available == 0);
            if (part1 && part2)
                return false;
            else
                return true;
        }
        static void ReadResponse(Socket sck)
        {
            while (true)
            {

                if (SocketConnected(sck) == true)
                {
                    try
                    {
                        string response = Encoding.ASCII.GetString(readResponseFunc());

                        byte[] message_1 = Encoding.ASCII.GetBytes("get_id");
                        sck.Send(Encoding.ASCII.GetBytes(message_1.Length.ToString()));
                        byte[] responseUnknown = new byte[1];
                        if (Encoding.ASCII.GetString(responseUnknown) == "a")
                        {
                            sck.Send(Encoding.ASCII.GetBytes("b"));
                            sck.Send(message_1);
                        }
                        byte[] response_2InByteArray = readResponseFunc();
                        string response_2 = Encoding.ASCII.GetString(response_2InByteArray);
                        if (Convert.ToInt32(response_2) == id)
                        {
                            if (response == "dir")
                            {
                                string resultOfDirring = "Current Directory: " + directory + "\n\n";
                                string[] folderListingArray = Directory.GetDirectories(directory);
                                foreach (string dir in folderListingArray)
                                {
                                    string formed = "DIRECTORY: " + Path.GetFileName(dir);
                                    resultOfDirring = resultOfDirring + formed + Environment.NewLine;
                                }
                                string[] fileListingArray = Directory.GetFiles(directory);
                                foreach (var file in fileListingArray)
                                {
                                    FileInfo fileInfo = new FileInfo(file);
                                    string formed = "FILE: " + Path.GetFileName(file) + " - FILE SIZE: " + fileInfo.Length + " BYTES";
                                    resultOfDirring = resultOfDirring + formed + Environment.NewLine;
                                }
                                byte[] message_11 = Encoding.ASCII.GetBytes(resultOfDirring);
                                sck.Send(Encoding.ASCII.GetBytes(message_11.Length.ToString()));
                                byte[] responseUnknown_11 = new byte[1];
                                if (Encoding.ASCII.GetString(responseUnknown_11) == "a")
                                {
                                    sck.Send(Encoding.ASCII.GetBytes("b"));
                                    sck.Send(message_11);
                                }
                            }
                        }
                        else { }
                    }
                    catch { if (SocketConnected(sck) == false) { Console.WriteLine("Client Disconnected: " + sck.RemoteEndPoint); break; }; }
                }
                else if (SocketConnected(sck) == false) { Console.WriteLine("Client Disconnected: " + sck.RemoteEndPoint); break; }
            }
        }
        static void Connect()
        {
            while (true)
            {
                try
                {
                    sck.Connect(IPAddress.Parse("127.0.0.1"), 80);
                    break;
                }
                catch { }
            }
        }
    }
}

Server:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Sockets;
using System.IO;
using System.Text;
using System.Threading;

namespace serverControl
{
    class Program
    {
        public static int ftpNum = 1;
        public static List<int> listOfClient = new List<int>();
        public static TcpListener server = new TcpListener(IPAddress.Parse("127.0.0.1"), 80);
        public static string message = null;
        public static int id = 0;
        public static int selected_id = 10;
        static void Main(string[] args)
        {
            server.Start();
            Thread startHandlingClientThread = new Thread(startHandlingClient);
            startHandlingClientThread.Start();
            while (true)
            {
                Console.Write(":> ");
                string rawmessage = Console.ReadLine();
                if (rawmessage == "list")
                {
                    Console.WriteLine("SELECTED ID: " + selected_id);
                    Console.WriteLine("List of Clients ID:");
                    for (int i = 0; i < listOfClient.Count; i++)
                    {
                        Console.WriteLine(listOfClient[i]);
                    }
                    message = rawmessage+"PREVENT_REPETITION_IN_COMMAND";
                }
                else if (rawmessage.Contains("set ")) { int wantedChangeId = Convert.ToInt32(rawmessage.Replace("set ", "")); selected_id = wantedChangeId; message = rawmessage+ "PREVENT_REPETITION_IN_COMMAND"; }
                else
                {
                    message = rawmessage;
                }
            }
        }

        static byte[] readResponseFunc(Socket sck)
        {
            long fileSize = 0;
            string fileSizeInString = null;
            byte[] fileSizeInByteArray = new byte[1024];
            int fileSizeLength = sck.Receive(fileSizeInByteArray);
            for (int i = 0; i < fileSizeLength; i++)
            {
                fileSizeInString = fileSizeInString + (char)fileSizeInByteArray[i];
            }
            fileSize = Convert.ToInt64(fileSizeInString);

            sck.Send(Encoding.ASCII.GetBytes("a"));

            byte[] responseUnknown = new byte[1];
            sck.Receive(responseUnknown);
            if (Encoding.ASCII.GetString(responseUnknown) == "b")
            {
                byte[] dataInByteArray = new byte[fileSize];
                int dataLength = sck.Receive(dataInByteArray);
                return dataInByteArray;
            }
            return Encoding.ASCII.GetBytes("ERROR RESPONSE FUNC");
        }
        static void startHandlingClient()
        {
            while (true)
            {
                handleClient(server);
            }
        }
        static void handleClient(TcpListener clientToAccept)
        {
            Socket sck = clientToAccept.AcceptSocket();
            Thread myNewThread = new Thread(() => ReadResponse(sck));
            myNewThread.Start();
        }
            static bool SocketConnected(Socket s)
            {
                bool part1 = s.Poll(1000, SelectMode.SelectRead);
                bool part2 = (s.Available == 0);
                if (part1 && part2)
                    return false;
                else
                    return true;
            }
        static void ReadResponse(Socket sck)
        {
            Thread myNewThread = new Thread(() => SendtoClient(sck));
            myNewThread.Start();
            Thread.Sleep(2000);
            while (true)
            {

                if (SocketConnected(sck) == true)
                {
                    try
                    {
                        byte[] dataInByteArray = readResponseFunc(sck);

                        string response = Encoding.ASCII.GetString(dataInByteArray);
                        Console.WriteLine("res: " + response);
                        if (response != "make_id" && response != "get_id") { Console.WriteLine(response); }
                        if (response == "make_id")
                        {
                            Console.WriteLine("Someone wants an ID");
                            byte[] message_1 = Encoding.ASCII.GetBytes(id.ToString());
                            listOfClient.Add(id);
                            // START
                            sck.Send(Encoding.ASCII.GetBytes(message_1.Length.ToString()));
                            byte[] responseUnknown = new byte[1];
                            if (Encoding.ASCII.GetString(responseUnknown) == "a")
                            {
                                sck.Send(Encoding.ASCII.GetBytes("b"));
                                sck.Send(message_1);
                            }
                            id++;
                        }
                        if (response == "get_id")
                        {
                            byte[] message_1 = Encoding.ASCII.GetBytes(selected_id.ToString());
                            sck.Send(Encoding.ASCII.GetBytes(message_1.Length.ToString()));
                            byte[] responseUnknown = new byte[1];
                            if (Encoding.ASCII.GetString(responseUnknown) == "a")
                            {
                                sck.Send(Encoding.ASCII.GetBytes("b"));
                                sck.Send(message_1);
                            }
                        }
                    }
                    catch { if (SocketConnected(sck) == false) { Console.WriteLine("Client Disconnected: " + sck.RemoteEndPoint); break; }; }
                }
                else if (SocketConnected(sck) == false) { Console.WriteLine("Client Disconnected: " + sck.RemoteEndPoint); break; }
            }
        }
        static void SendtoClient(Socket sck)
        {
            string tempmessage = null;
            while (true)
            {
                if (SocketConnected(sck) == true)
                {
                    if (tempmessage != message)
                    {
                        if (!message.Contains("PREVENT_REPETITION_IN_COMMAND"))
                        {
                            byte[] message_1 = Encoding.ASCII.GetBytes(message);
                            sck.Send(Encoding.ASCII.GetBytes(message_1.Length.ToString()));
                            byte[] responseUnknown = new byte[1];
                            if (Encoding.ASCII.GetString(responseUnknown) == "a")
                            {
                                sck.Send(Encoding.ASCII.GetBytes("b"));
                                sck.Send(message_1);
                            }
                        }
                        tempmessage = message;
                    }
                }
                else if (SocketConnected(sck) == false)
                { Console.WriteLine("Client Disconnected: " + sck.RemoteEndPoint); break; }
            }
        }
    }   
}

Problem: The problem is within the GetSession or the ReadResponseFunc function. The client thinks that his ID received by the server is 'a' (it's suppose to be an integer). I don't want a solution that suggest me to use other libs or the TcpClient class

Bounty:

I'll put up a bounty with no expiry time to those who solve the problem.

Example of using `Socket`, for reference: http://stackoverflow.com/questions/34586733/sending-a-value-from-server-to-client-with-sockets/34669446#34669446 — Ian, May 05 '17 at 02:08

score 4 · Accepted Answer · answered Apr 24 '17 at 14:54

4

The logic in your code is very confusing. My question to you: Why are you sending 'a' and 'b' back and forth between the server and client? Is it some sort of confirmation that the message has been received?

Anyways, throughout the quick tests I've done just now, it seems that the problem is Line 59 of your server:

sck.Send(Encoding.ASCII.GetBytes("a"));

Here's what I figured out during testing:

Server executes.
Client executes.
Client sends server the length of "make_id" (Line 51 of client)
Client waits for a response to return.
Server reads the value and sends back 'a' (Line 59 of server)

You may want to spend some time to straighten out your protocol so it's less confusing and more organized. That would help people like me and you spot bugs much more easily.

answered Apr 24 '17 at 14:54

Bobby

186
3

I agree to your analysis. I also agree to your suggestion that the OP needs to redesign the code and create a suitable higher-level protocol: I suggest encapsulating messages in between 0x02 (STX) and 0x03 (ETX), and/or adding the length of the message at its start. Also use a defined message structure (header), e.g. bytes 0 and 1 containing a message ID or something similar. – Tobias Knauss Apr 24 '17 at 20:43
I have to send the File Length and the File. This is my code ti send before adding the sending 'a' and 'b' part:: - sck.Send(encoding.ascii.getbytbytes(data.Length)) and the next Line sck.send(data);. However the server thinks that the data is sent is packed into 1 (received message is: 4dir). So I created the program to send the 'a' and 'b' so It doesn't mix with the other response. – Adola Apr 25 '17 at 03:08
I've debug and found the problem: `Server reads the value and sends back 's' (Line 59 of server)` just as you mentioned. But, I can't still fix the problem... Can you edit or at least tell me why it sends 'a'. – Adola Apr 30 '17 at 12:18
The reason why it sends 'a' is in your code. Take a look at line 57 of the server - it receives the length of "make_id" which is 7. Then, the next statement directly after that is on line 59, which sends 'a' to the client. Straight off the top of my head, you can probably remove line 59 and the first message should go through correctly. However, I cannot guarantee the integrity of the rest of the code's functionality. – Bobby May 02 '17 at 15:23
Now, I personally suggest that you should instead send the length of the data in bytes rather than in ASCII string. That is because the length of an int is always 4 bytes. The ASCII string won't always stay the same size, which is why you are using 'a'. This way however, you won't need 'a' or 'b' to differentiate between the data. All you would need to do is read the first 4 bytes, convert that to an int which describes the data's length and then read the remaining bytes. If you prefer to use your own implementation, then you will have to have your client handle the incoming 'a' message. – Bobby May 02 '17 at 15:23

score 1 · Answer 2 · answered Apr 24 '17 at 21:05

The user 'Bobby' has already found your problem. Credits go to him.

I further suggest that you use less threads, as thread synchronisation needs some effort when doing it right: all data that is accessed from different threads must be secured by locks, so that no outdated values remain in the CPU caches. Use .net Monitor from the "threading synchronisation primitives" for that job.

About the threads themselves:
You should have only one thread in the server. This thread takes all clients from a list (secured by Monitor), in which they were added when connection attempts were received. On each client it checks for incoming messages, evaluates them and replies with own messages if needed.

The client also has just one thread, that will loop (dont forget a sleep in the loop or you will have 100% usage of the used CPU core), send messages when desired and wait for replies when messages were sent.

About the messages:
I already gave some proposals in a comment to Bobby's answer. I suggest you create a CMessage class that has a Serialize() and Deserialze() to create a byte array to send from the CMessage members (serializing the content) or vice versa filling the members from the received bytes. You then may use this class in both programs and you'll have common solution.

C# Socket: Client Mishandle 'a' as the Client's id

Bounty:

2 Answers2