I am experimenting on a server-client authentication system and my goal is to send few details like mac id, ip address etc to the TCP server, Server then receives it, check a database if the specified address is present, if present it will return " 0 " as return data. client then reads it and fires up the OnAuthorized Function.
so, the problem is, Client sends the data, sever reads it and then returns 0 but on the code, i see
0\0\0\0\0\0\
instead of the "0" On the Console it displays 0 but the next line is pretty down so i assume its whitespace charecters but Trim() should remove them right ? but doesnt. i was using breakpoints and checked the return value in HTML Visualizer and saw the single character 0 but it isn't reflecting on the actual string. here is the code.
Client
IPEndPoint IPEndPoint = new IPEndPoint(ServerIP, Constants.SecurityServerPort);
Client.Connect(IPEndPoint);
Logger.LogGenericInfo("Socket created to {0}", Client.RemoteEndPoint.ToString());
byte[] sendmsg = Encoding.ASCII.GetBytes(MsgToSend);
int Send = Client.Send(sendmsg);
int Received = Client.Receive(Data);
if(Data != null)
{
ReceivedData = Encoding.ASCII.GetString(Data).Trim(' ');
Logger.LogGenericInfo(ReceivedData);
}
Client.Shutdown(SocketShutdown.Both);
Client.Close();
Server:
IPEndPoint localEndpoint = new IPEndPoint(IPAddress.Any, 7205);
ConsoleKeyInfo key;
int count = 0;
Socket sock = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
sock.Bind(localEndpoint);
sock.Listen(5);
while (true)
{
Console.WriteLine("\nWaiting for clients..{0}", count);
Socket confd = sock.Accept();
int b = confd.Receive(Buffer);
data += Encoding.ASCII.GetString(Buffer, 0, b);
Console.WriteLine("" + data);
data = null;
confd.Send(Message);
Console.WriteLine("\n<< Continue 'y' , Exit 'e'>>");
key = Console.ReadKey();
if (key.KeyChar == 'e')
{
Console.WriteLine("\nExiting..Handled {0} clients", count);
confd.Close();
System.Threading.Thread.Sleep(5000);
break;
}
confd.Close();
count++;
}
Server return Data = private static byte[] Message = Encoding.ASCII.GetBytes("0");
The client receives the data into its buffer but you don't use the value you stored of how many bytes were received:
int Received = Client.Receive(Data);
You then turn the whole buffer into a string, which will include many NUL characters (ascii 0) as well as your zero character digit (ascii 48). If the buffer is re-used this will repeat the old data too and could represent a security risk:
if(Data != null)
{
ReceivedData = Encoding.ASCII.GetString(Data).Trim(' ');
By the way this if will always be true. Perhaps you mean if(Received > 0)
You then try to trim these off by using trim but you tell c# to trim spaces (ascii 32) off, and it isn't a space that is polluting, its a NUL (ascii 0)..
You'd be better off doing what you do on the server side and using Received number of bytes to limit how many characters GetString returns. This is what you do on the server side:
int b = confd.Receive(Buffer);
data += Encoding.ASCII.GetString(Buffer, 0, b);
You use b, on the server. On the client you don't use Received. I'm not really sure why you say data += as this will add your current data to the previous data, but maybe this is intentional
By the way, please avoid naming method scoped variables (variables that are define inside method bodies, like int Received, with a capital letter at the start. In c# this convention is for publicly accessible properties, methods, events, class level variables etc. It's quite difficult to read code that doesn't follow these conventions because these variables aren't defined in the place where their name implies they would be defined
You might also run into problems with this code in that the server will only respond to a client one time, then pause while it waits for console input from you. Again this might be intentional but it seems unlikely. I'd recommend you switch to using async style programming for receiving and acting upon data at the server end then the server will become able to serve multiple clients at the same time. Right now, it will struggle to do this even if you remove the console interaction. While you might intend this server to be very simple, it's not an ideal intro to "how servers should be done" to have this "syncronous" way of "read some data, do some work, write some data" because the first client has to wait while the work is done, and the second client has to wait until the first client goes away.
Last point (promise) - there are a few ways (like microservice architecture implementations) built into c# where all this low level faffing, writing bytes to sockets etc, it taken away from us and done really well by Microsoft, and all we have to do is define a service, and the name of the operation(by writing a method named that thing) and write the server side code that implements the operation.. Then on the client side we basically tell visual studio "I want to use that service" and it writes all the client side code needed to link it up, so the client side essentially comes down to two lines (create client, call method) and the server side in this case is about 3 lines (declare method, read value, return response value) and its multithreaded, async, works perfectly, isn't a confusing mash of bytes and encodings, socket operations, loops blah
Doing it the way you've done here is great for learning and appreciating the underneath workings, but it's kinda like writing your own display driver rather than using Nvidia's one; there's not much point
A simple hello world example (about the same as what you're doing): https://learn.microsoft.com/en-us/dotnet/framework/wcf/feature-details/how-to-create-a-basic-wcf-web-http-service
More detailed info on servicehost : https://learn.microsoft.com/en-us/dotnet/framework/wcf/hosting-services