"Use of unassigned variable" error

Question

I am developing this website in ASP.NET and using C#. I am Getting the error that :Use of unassigned variable usn. The database is also not empty. My code is:

protected void Button1_Click(object sender, EventArgs e)
{

    SqlConnection cn = new SqlConnection();
    SqlCommand cm = new SqlCommand();
    SqlDataReader dr;
    cn.ConnectionString = @"Data Source=.\SQLEXPRESS;AttachDbFilename=C:\Users\Vijaylaxmi\Desktop\TrainReserveold\App_Data\Database.mdf;Integrated Security=True;User Instance=True";
    cn.Open();
    cm.Connection = cn;
    String usn;
    cm.CommandText = "Select UserName from User where UserName='" + TextBox1.Text + "'";
    dr = cm.ExecuteReader();
    while (dr.Read())
    {
        usn = dr.GetString(0);
    }
   if (String.Compare(usn, TextBox1.Text) != 0)
    {
        Response.Write("Invalid user name... try again");
        TextBox1.Text = "";
        TextBox2.Text = "";
        TextBox1.Focus();
    }
   Response.Write("user valid now");
}

You should be wrapping the creation of the `SqlConnection`, `SqlCommand` and `SqlDataReader` in `using` statements. — Oded, Dec 21 '11 at 17:08
Now you are being rude. As soon as possible? I am not paid for this, you know. And this just made me lose all interest in answering. — Oded, Dec 21 '11 at 17:08
also I would look at rewriting the way that you are creating your Connection and Command I will post an example that will make it easier for you to follow what you have can lead to your own confusion — MethodMan, Dec 21 '11 at 17:10

score 3 · Answer 1 · answered Dec 21 '11 at 17:14

Several problems I see here. In specific response to your question, you want to replace this:

dr = cm.ExecuteReader();
while(dr.Read())
{
  usn = dr.GetString(0);
}

with this:

usn = cm.ExecuteScalar().ToString();

Be sure to check for DBNull first, just in case.

More generally, you want to
a) Parameterize your SQL (or, better, use a stored proc) instead of using raw input. This will protect you from SQL Injection attacks.
b) Not include your connection string directly in code. Put it in a config file. Most certainly don't post it on the internet.

MethodMan · Answer 2 · 2011-12-21T17:54:58.200

1

assing the usn string up top as

string usn = string.empty; then go from there
//create a Stored Procedure and put your Select Statement in there.. to avoid Sql Injection
cmd.CommandText = "name of your stored proc";
cmd.CommandType = System.Data.CommandType.StoredProcedure;

I would also read my sql connectiong string from a web.config or app.config depending on the type of application you are running.

edited Dec 21 '11 at 17:54

answered Dec 21 '11 at 17:09

MethodMan

18,625
6
34
52

1

It would probably be better to assign NULL, since the empty user name is probably not valid, even if no other users exist in the DB. – Jonas Høgh Dec 21 '11 at 17:10
Empty is just an initializer either way.. I would truly recommend that he refactor his connection with a using something like this and then create / assign his command object within the using using (SqlConnection sqlConnSqlConnection = new SqlConnection(strConnectionString)){ } – MethodMan Dec 21 '11 at 17:49
All of the code smells badly. But I guess Rome wasn't built in a day. No reason to introduce MORE bugs :) – Jonas Høgh Dec 21 '11 at 17:53
Jonas there are a lot of good idea's and comments here what he will have to do is basically rewrite or refactor his existing code to use things such as stored procedures to execute SQL without introducing SQL injection, also get on board with some of todays newer constructs for example with wrapping things in a using(){} our job is not to rewrite his code but to add guidance and suggestions on how to help him fix current and potential issues. – MethodMan Dec 21 '11 at 17:57

score 0 · Answer 3 · edited Dec 21 '11 at 17:27

0

change your cm.CommandText = "Select UserName from User where UserName= to

  cm.CommandText = string.Format("Select UserName from User where UserName= '{0}'",Textbox1.Text);

edited Dec 21 '11 at 17:27

Mike Hofer

16,477
11
74
110

answered Dec 21 '11 at 17:13

MethodMan

18,625
6
34
52

1

This will NOT prevent SQL injection – Jonas Høgh Dec 21 '11 at 17:37
yes he can always change that to supply the sql command in a stored proc.. I am aware but good point.. – MethodMan Dec 21 '11 at 17:45
Stored procedures don't prevent injection either. Command parameters do. – Jonas Høgh Dec 21 '11 at 20:05
yes I am aware of that Jonas I am assuming that he would know that he would have to use the parameters.AddWithValue but thats not part of his question anyways I appreciate your accurate input – MethodMan Dec 21 '11 at 20:18
i am very thankful to you sir. Because i really not so much familiar with using stored procedure and this helped me a lot...... – VijayLaxmi Dec 22 '11 at 03:17

"Use of unassigned variable" error

3 Answers3