BerGEX BerGEX - 2 months ago 6
C# Question

Having a error with WPF C# Login form

I am creating a simple Hospital Management System, and I was having a problem connecting to the database.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.UI;
using System.Web.UI.WebControls;
using System.Data.Sql;
using System.Data.SqlClient;
public partial class Login : System.Web.UI.Page
{
SqlConnection connectionstring = new SqlConnection("Server=.\\SQLEXPRESS;Database=TestDB;User Id=test; Password = woooow; ");



protected void Button1_Click1(object sender, EventArgs e)
{
string cmdText = "SELECT 1 FROM Login WHERE Username = '" + TextBox1.ToString() + "' AND Password = '" + TextBox2.toString()+ "'";

// using (SqlConnection cnn = new SqlConnection("Server=.\\SQLEXPRESS;Database=TestDB"))
using (SqlCommand SelectCommand = new SqlCommand(cmdText, connectionstring))
{
SqlDataReader myReader;
connectionstring.Open();
myReader = SelectCommand.ExecuteReader();
int count = 0;

while (myReader.Read())
{
count = count + 1;
}

if (count == 1)
{
//open form
}
else
{

}
}

}


}

this is the code I use for the Login form in a normal C# application. but looks like I am doing something wrong in TextBox1.toString() and TextBox2.toString().
How can I take the exact value of the textbox? by googling around, I saw many posts which say it, but everything is different from each other and making me really confused about it.

So, which is the best way to do that?

Thanks.

Answer

TextBox1.ToString() returns the fully qualified name of the class textbox that is

System.Windows.Forms.TextBox, Text: 

The exact property that returns the content of a TextBox is the Text property, however, simply replacing your TextBox1.ToString() with TextBox1.Text, while working, could be the cause of other potential and more dangerous errors.

SqlCommand should use the parameters collection, not build the sql text concatenating strings. This is well known as a source of errors (parsing decimals, dates and strings with embedded quotes) and a big security risk called Sql Injection.

Instead your code with parameters is

public partial class Login : System.Web.UI.Page
{
    private string conString = "Server=.\\SQLEXPRESS;Database=TestDB;User Id=test; Password = woooow; ";

    protected void Button1_Click1(object sender, EventArgs e)
    {
        int count = 0;
        string cmdText = @"SELECT 1 FROM Login 
                           WHERE Username = @uname 
                             AND Password = @pwd";

        using (SqlConnection cnn = new SqlConnection(conString))
        using (SqlCommand cmd = new SqlCommand(cmdText, cnn))      
        {
           cnn.Open();
           cmd.Parameters.Add("@uname", SqlDbType.NVarChar).Value = textBox1.Text;
           cmd.Parameters.Add("@pwd", SqlDbType.NVarChar).Value = textBox1.Text;
           using(SqlDataReader myReader = SelectCommand.ExecuteReader())
           {
               while (myReader.Read())
                 count = count + 1;
           }
       }
       if (count == 1)
       {
          //open form
       }
       else
       {

       }
   }
}

Notice that I have also made your connection string global and created a local SqlConnection object. To be sure to dispose the object the best approach is having everything local and put the disposable object inside a using statement.

Consider also that storing password in clear text is another security risk. This question explain The Best way to store passwords in a database