0
\$\begingroup\$

I have developed a TCP listener to receive messages from a client machine, take that string and put it in a db, then send back a response with some data relating to that string. First attempt at a tcp network stream send and reply, just need to see if this will result in any easily avoidable "hiccups" or incorrect data being returned.

private void ReceivePortMessages()
{
    try
    {
        _TcpListener.Start();
        Debug.Print(" >> Server Started");
        _TcpClient = _TcpListener.AcceptTcpClient();
        Debug.Print(" >> Accept connection from client");
        Boolean isProcessing = true;
        using (NetworkStream networkStream = _TcpClient.GetStream())
        {
            while (isProcessing)
            {
                try
                {
                    int receivingBufferSize = (int)_TcpClient.ReceiveBufferSize;
                    byte[] bytesFrom = new byte[receivingBufferSize];
                    networkStream.Read(bytesFrom, 0, receivingBufferSize);
                    Stopwatch sw = new Stopwatch();
                    sw.Start();
                    string dataFromClient = System.Text.Encoding.ASCII.GetString(bytesFrom);
                    dataFromClient = dataFromClient.Substring(0, dataFromClient.IndexOf("\0"));
                    if (dataFromClient != string.Empty)
                    {
                        XmlDocument xm = new XmlDocument();
                        xm.LoadXml(string.Format("<root>{0}</root>", dataFromClient));
                        XmlElement root = xm.DocumentElement;
                        string rootName = root.FirstChild.Name;
                        // Data is sent to be processed and sent back.
                        RouteInboundXML(rootName, dataFromClient, sw);
                    }
                }
                catch (ArgumentOutOfRangeException ex)
                {
                    Debug.Print("ReceivePortMessages: Remote client disconnected. " + ex.ToString());
                    _TcpClient.Close();
                    _TcpListener.Stop();
                    ErrorLog.Write("MotionIndustriesXmlProcessing", ex.ToString(), "ReceivePortMessages()");
                    return;
                }
                catch (Exception ex)
                {
                    Debug.Print("ReceivePortMessages: " + ex.ToString());
                    _TcpClient.Close();
                    _TcpListener.Stop();
                    ErrorLog.Write("MotionIndustriesXmlProcessing", ex.ToString(), "ReceivePortMessages()");
                    return;
                }
            }
            Debug.Print(" >> exit");
            isProcessing = false;
        }
    }
    catch (Exception ex)
    {
        Debug.Print("ReceivePortMessages: " + ex.ToString());
        ErrorLog.Write("MotionIndustriesXmlProcessing", ex.ToString(), "ReceivePortMessages()");
    }
    ReceivePortMessages();
}

private void RouteInboundXML(string requestType, string requestXML, Stopwatch sw)
{
    //
    // A bunch of other operations happen and then they call the SendReply() method.
    //
    SendReply();
}

private void SendReply(string reply)
{
    try
    {
        using (NetworkStream networkStream = _TcpClient.GetStream())
        {
            string serverResponse = reply;
            Byte[] sendBytes = Encoding.ASCII.GetBytes(serverResponse);
            networkStream.Write(sendBytes, 0, sendBytes.Length);
            networkStream.Flush();
            Debug.Print(" >> " + serverResponse);
        }
    }
    catch (ArgumentOutOfRangeException ex)
    {
        Debug.Print("SendReply: Remote client disconnected. " + ex.ToString());
        _TcpClient.Close();
        _TcpListener.Stop();
        return;
    }
}
\$\endgroup\$
2
\$\begingroup\$

I see you aren't using any sort of multithreading or asynchrony. That means that your server is an iterative one - i.e. it can only process one client request at a time. If a request takes long, subsequent requests may have to wait to be processed. Depending on what you're doing, this may be acceptable or not.

What I don't quite like with your implementation is the way you're dealing with incoming data. You're doing low-level buffer read operations, and you are then looking for the \0 to see where it ends, and then transforming it into a string.

The reason you're doing this is that if your buffer is 10 bytes long and the incoming data is 5 bytes long, then the buffer will look like this:

Hello\0\0\0\0\0

But that can lead to a number of problems. What if the incoming data is larger than the buffer? What if there's a \0 within the actual data itself?

There is no single way to do this, as this is a protocol design thing. Let's take a look at a few ways you could have done this.

1. Newline-delimited requests

If you know your data will never contain newlines, then you can end each request with a newline, and then easily use StreamReader.ReadLine() to process your data without all the low-level buffer details. Example:

        var server = new TcpListener(6969);
        server.Start();
        var client = server.AcceptTcpClient();
        var stream = client.GetStream();

        using (var reader = new StreamReader(stream))
        {
            string data = reader.ReadLine();

            // do something with the data...
        }

2. Counting bytes read

#1 above isn't always an option, e.g. if your data contains newlines.

An important observation here is that your data can arrive at the application in chunks - not necessarily as one long string as you might expect. So it is a good idea to likewise read it in chunks, similar to the following:

        var server = new TcpListener(6969);
        server.Start();
        var client = server.AcceptTcpClient();
        var stream = client.GetStream();

        using (var reader = new StreamReader(stream))
        {
            var sb = new StringBuilder();
            var buffer = new char[8192];
            int bytesRead = reader.Read(buffer, 0, buffer.Length);

            if (bytesRead > 0)
            {
                var buffer2 = new char[bytesRead];
                Array.Copy(buffer, buffer2, bytesRead);
                sb.Append(buffer2);

                // if sb meets some criteria, process the data...
            }
            else
                Console.WriteLine("Client disconnected!");
        }

Over here we're reading data into a buffer - not much different from what you were doing. But there are a few things we're doing differently:

  1. We are reading in chunks. We aren't assuming that we get all the data at once; we're appending the partial data to a StringBuilder, and then doing something with it once any criteria are met (e.g. there is a terminator character, or a predefined number of bytes have been read).
  2. We're copying into a secondary buffer. If we keep to the example I mentioned earlier where we had a 10-byte buffer but 5 incoming bytes, then we copy just those 5 bytes into a second, 5-byte buffer, to get rid of the extra padding (\0s).
  3. We know how much data was read with each call to StreamReader.Read(), because it returns the number of bytes read. This allows us to both do #2 above and also to detect a disconnect.

3. Length-prefixed requests

In #2 above, I mentioned that you must check the data itself to see that you received a whole, integral request and that you can now start processing it. Some protocols do this using a terminator character (e.g. \n, which may or may not be a candidate for #1 above, or possibly other characters).

Another variant places the number of bytes at the beginning of the request, so that the receiving end knows how many to expect. For example, the agreed protocol would be that the first four characters are a number representing the data length, allowing a maximum of 9999 bytes. The actual data would look something like this:

38  Indiana Jones and the Fate of Atlantis

The code would be quite similar to #2 above, but you'd have to extract the length as soon as you have those four bytes available in your buffer, and then call Read() repeatedly until you have all the bytes you were expecting. At that point, you can process the data.

Encoding

I also noticed that you're using an ASCII encoding to convert your data between bytes and strings. This may be okay for some scenarios, but you'll have trouble if you need to deal with multilingual data. UTF8 may be more appropriate in such cases.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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