TCP authentication server and client

This is the first time I'm dealing with C# as I'm accustomed to Java sockets, so I do want your full review and anything you think I can accomplish better or optimize in the code will be very appreciated.

In these few classes what I have tried to accomplish is a login and registration server based on C# .Net working as a windows application communicating with a GUI built with Unity5 along with a client side with a MySQL database.

Along with that system I have implemented an email activation system on registration that requires link activation.

Everything works pretty well and I thought this could be a good time to stop working and let other people help me fully understand if my TCP socket implementation and practice is correct.

First is the Windows Application Login Server.

```
using System;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using System.Net;
using System.Net.Mail;
using System.Net.Mime;
using System.Net.Sockets;
using System.IO;
using System.Security.Cryptography;
using MySql.Data.MySqlClient;

namespace LoginServer
{
public partial class frmLoginServer : Form
{
public string TBLog
{
get { return tbLog.Text; }
set { tbLog.AppendText(value); }
}

public List _clientSockets { get; set; }

private byte[] _buffer = new byte[1024];
private Socket _serverSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

//MySql Objects
private string connStr = @"server=localhost;user=root;password=1234;database=exodus_db";
private MySqlConnection mysqlConn = null;
private MySqlDataReader mysqlRdr = null;
private HTTPServer webServer = new HTTPServer();

public frmLoginServer()
{
InitializeComponent();
CheckForIllegalCrossThreadCalls = false;
if (!Directory.

You seem to be doing a lot of low-level byte manipulation when using the sockets. Once you have a NetworkStream, just wrap it inside a StreamReader/StreamWriter and let them handle sending strings over the wire.

When designing a protocol, please take into account that sending Hello World and calling = _stream.Read (_buffer, 0, _buffer.Length); on the client rarely results in reading Hello World in one go. It can return more data like Hello WorldSecond Message Sent By Server or can return just a part, Hello W, making the rest available on a future call to Read().

Begin/End methods are part of the old .NET async APIs. While they still work ok, using the newer async/await (.NET 4.5+) results in much less code which is almost as easy to read as synchronous one.

For instance: .NET offers the TcpListener which provides TcpListener.AcceptTcpClientAsync(). The TcpClient when provides GetStream() which provides you with a NetworkStream which you can pass to a StreamReader which then has ReadLineAsync(). Should you use newlines as a message separator in your protocol, this ends up providing you with each individual message, as a string, without you having to manage any buffers or threads manually.

Also pay attention when calling UI code from a method that may not execute on the UI thread. WinForms have a property InvokeRequired that can be used to check if you can directly access UI elements or need to call Invoke to marshal them on the UI thread.

Don't use SELECT * in a query to then access the columns by index. It's too fragile. You add an extra column and boom, the code starts to fail. Either explicitly specify the columns (preferred) or at least ask the reader what is the index for column X instead of hard-coding it.

When disposing of resources, using using is much cleaner than adding a finally block which closes the resource. You're using using in GenerateSalt(), but that's about it.

When writing to a file, consider helper functions:

File.Create(file).Dispose();
using (TextWriter tw = new StreamWriter(file))
{
    tw.WriteLine("This is the log file of the " + DateTime.Now.ToString("yyyy-M-d"));
    tw.Close();
}

can simply be replaced by a call to File.WriteAllText() or File.WriteAllLines().

StackExchange Code Review Q#131457, answer score: 4