Receiving all data comes from web in system.net.socket

Question

I listen on a port via multi thread async socket. That port receives requests over XDomainRequest. I received the complete data each time till now but I don't trust this code because it may not work in some cases. (network traffic etc.)

Here is my StateObject:

public class StateObject
{
 public Socket workSocket = null;
 public const int BufferSize = 256;
 public byte[] buffer = new byte[BufferSize];
 public StringBuilder sb = new StringBuilder();
 // stores complete byte length comes from XDomainRequest
 public int totalReceivedBuffer = 0;
 // stores packet's byte lengh at each ReceiveCallBack
 public int receivedBuffer = 0;
}

Here is my AcceptCallBack:

public static void AcceptCallback(IAsyncResult ar)
{
 Socket listener = null;
 Socket handler = null;
 try
 {
 _manualResetEvent.Set();
 listener = (Socket)ar.AsyncState;
 handler = listener.EndAccept(ar);
 StateObject state = new StateObject();
 state.workSocket = handler;
 // If I don't put sleep here I cannot learn total byte length
 // which comes from XDomainRequest. (That disturbs me)
 Thread.Sleep(200); 
 state.totalReceivedBuffer = handler.Available;
 handler.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(ReceiveCallback), state);
 listener.BeginAccept(new AsyncCallback(AcceptCallback), listener);
 }
 catch (SocketException ex)
 {
 #region SocketException
 if (ex.ErrorCode == 10054)
 {
 // HATA: "An existing connection was forcibly closed by the remote host"
 Globals.ErrorLogYaz(_log, Globals.GetCurrentMethod(), "Karşı taraf işlem tamamlanmadan bağlantıyı kapattı!", ex.StackTrace);
 }
 handler.Close();
 listener.BeginAccept(new AsyncCallback(AcceptCallback), listener);
 #endregion
 }
 catch (Exception ex)
 {
 Globals.ErrorLogYaz(_log, Globals.GetCurrentMethod(), ex.Message, ex.StackTrace);
 handler.Close();
 listener.BeginAccept(new AsyncCallback(AcceptCallback), listener);
 }
}

In ReceiveCallBack I add packet length to the receivedBuffer and when it becomes equal to totalReceivedBuffer I think I read all data which came from XDomainRequest.

public static void ReceiveCallback(IAsyncResult result)
{
 StateObject state = null;
 Socket handler = null;
 try
 {
 state = (StateObject)result.AsyncState;
 handler = state.workSocket;
 string clientIP = ((IPEndPoint)handler.RemoteEndPoint).Address.ToString();
 // See how many bytes received
 int numBytesReceived = handler.EndReceive(result);
 if (!handler.Connected)
 {
 handler.Close();
 return;
 }
 if (numBytesReceived > 0)
 {
 state.receivedBuffer += numBytesReceived;
 state.sb.Append(Encoding.ASCII.GetString(state.buffer, 0, numBytesReceived));
 // Read all data line by line
 string[] lines = state.sb.ToString().Split('\n');
 if (state.receivedBuffer == state.totalReceivedBuffer)
 {
 // All data came
 // do the job according to lines array
 }
 else
 {
 // continue receiving
 handler.BeginReceive(state.buffer, 0, state.buffer.Length, SocketFlags.None, new AsyncCallback(ReceiveCallback), state);
 }
 }
 }
 catch (ObjectDisposedException ex)
 {
 // Don't care
 }
 catch (SocketException ex)
 {
 #region SocketException
 if (ex.ErrorCode == 10054)
 {
 // HATA: "An existing connection was forcibly closed by the remote host"
 Globals.ErrorLogYaz(_log, Globals.GetCurrentMethod(), "Karşı taraf işlem tamamlanmadan bağlantıyı kapattı!", ex.StackTrace);
 handler.Close();
 }
 #endregion
 }
 catch (Exception ex)
 {
 Globals.ErrorLogYaz(_log, Globals.GetCurrentMethod(), ex.Message, ex.StackTrace);
 handler.Close();
 }
}

Here you can see the sending method:

private static void SendCallback(IAsyncResult ar)
{
 Socket handler = null;
 try
 {
 handler = (Socket)ar.AsyncState;
 // Get IP of client
 string clientIP = ((IPEndPoint)handler.RemoteEndPoint).Address.ToString();
 // End sending data
 int bytesSent = handler.EndSend(ar);
 Globals.InfoLogYaz(_log, Globals.GetCurrentMethod(), clientIP + " ip address received " + bytesSent.ToString() + " bytes!");
 handler.Shutdown(SocketShutdown.Both);
 handler.Close();
 }
 catch (ObjectDisposedException ex)
 {
 // Don't care
 }
 catch (Exception ex)
 {
 Globals.ErrorLogYaz(_log, Globals.GetCurrentMethod(), ex.Message, ex.StackTrace);
 handler.Close();
 }
}

What is the way of making this code safer?

• Snowbody: If your code doesn't work, or you know instances where your code won't work, you really ought to be asking on stackoverflow.com, not here. codereview.SE is for complete, working code.
• Orkun Bekar: @Snowbody The code is working. I also asked it on stackoverflow.com. I got some comments which tells me to write my code here :) Here I want to know if this code causes any data loss, is it safe etc.

Answer 1

What does the sending code look like?

You're using TCP to communicate. TCP is a stream protocol -- bytes go in one end, and come out the other end (or an error is detected). The only guarantee is that every byte sent will be received in the same order it was sent -- the bytes may arrive one at a time or in groups. The groups that they arrive in may not correspond to the groups that they were sent in; any steps along the way may split them up and recombine them, as long as all the bytes are still there in the same order.

So, when you are waiting for a message and ask to receive, you may get the whole message, you may get part of it, or you may get the whole message plus part of the next message (or messages). So your receive code has to be ready to handle all of these.

There are a number of strategies for imposing message-oriented behavior on TCP. You have to design your protocol to follow one of these strategies. One way is to make all your packets the same size, then keep asking for more until you fill up the buffer. Another way is to have the first byte(s) of your message encode the length. A third way is to define an "end-of-message" string that can't occur in a message.

You've stated in comments that a "message" is made up of multiple lines, each ending with a "\n". And you process lines in sequential order.

Two problems with this.

1. What if a packet gets fragmented, resulting in a line being split up between two packets? The data you receive won't end with a "\n" and will be missing part of the line. You're told the number of bytes available for you to take, but that count only includes what's available, and it's possible that when you check, only the first part of the line is available.
2. What if more data becomes available after you ask how much data is available? You're receiving 256 bytes at a time; you may end up with more than the amount of data that's available, which will make your code stall.

2 can be solved easily by changing the following lines:

// continue receiving
handler.BeginReceive(state.buffer, 0, state.buffer.Length, SocketFlags.None, new AsyncCallback(ReceiveCallback), state);

so instead of passing state.buffer.Length you pass state.buffer.Length - state.totalReceivedBuffer. This makes sure you don't receive more than you're expecting.

But it doesn't deal with #1. The way you deal with that is to check to see if the buffer ends with a '\n'. If it doesn't, you've ended up with a fragment. Don't process it. Process everything else, then stick the fragment into the buffer. You'll need to alter the math a bit to make sure that it's properly accounted for and again you don't request more than you're expecting.

(Also, async callbacks are a bit outdated; the new paradigm is async/await -- look into it).

• Orkun Bekar: Could you write some details about this sentence: "There are no guarantees that each "receive" request will match a "send" on the other end." Do you mean sometimes my sent data may be lost and XDomainRequest cannot catch it?
• Snowbody: Check out the edit. Also, you really need to post the protocol definition and/or the sending code.
• Orkun Bekar: Clients have IE9 and XdomainRequest on it. When they write an address to the browser socket listener catches it and looks at the headers. Gets query string inside the headers and gets data which client sent via xdr.send(data) method. Clients send two kinds of data to me. It becomes 140-150 bytes or 700-750 bytes. This is fixed.
• Snowbody: But your buffer is only 256 bytes, how can it possibly hold the 700-750 byte client request?
• Orkun Bekar: Each calling of ReceiveCallBack handles 256 bytes of it. I add them to the sum and when it becomes equal to totalReceivedBytes which I learnt in the AcceptCallBack I think it is OK. But definin an end of file is more secure I think.