ASP.NET C# Datareader loop problem

Hello,

i am trying to make my own membership system in asp.net 2.0 c# (i am using MY SQL)

public void btnSignUp_OnClick(object sender, EventArgs e)
      {
            // CREATE THE CONNECTION
            MySqlConnection con = new MySqlConnection(connectionString);
            try
            {
                  // CREATE THE COMMAND
                  string sql = "SELECT * FROM tbl_user";
                  
                  // CONFIGURE THE CONNECTION
                  MySqlCommand cmd = new MySqlCommand(sql, con);
                  
                  // OPEN THE DATABASE CONNECTION
                  con.Open();

                  // RUN THE QUERY
                  MySqlDataReader reader = cmd.ExecuteReader();

                  // PARSE THE RESULTS
                  while (reader.Read() == true)
                  {
                        object objusername = reader["user_name"];
                        object objpassword = reader["user_password"];

                        // IF USERNAME ALREADY EXISTS
                        if (txtNewUsername.Text == objusername.ToString())
                        {
                              lblSignUpStatus.Text = "Username already taken.";
                        }      
                        

                        // IF USERNAME DOES NOT EXIST
                        if (txtNewUsername.Text != objusername.ToString())
                        {
                              lblSignUpStatus.Text = "you can use this one, its not being used.";
                        }      
                        
                  }
                  // CLOSE THE READER
                  reader.Close();
            }
            finally
            {
                  // CLOSE THE DATABASE CONNECTION
                  con.Close();
            }      
      }

when the submit button is pressed the code tried to find if the username the customer has chosen already exists in the database, if it does already exist then it will show the message "username already taken".

if the username does not exist then the customer is shown an ok message.

the problem here is that there is 2 usernames in the table and when i chose the 2nd username in the table, it shows the message that it is ok to use. if i choose the first username in the table, it wont.

if i take away this

// IF USERNAME DOES NOT EXIST
                        if (txtNewUsername.Text != objusername.ToString())
                        {
                              lblSignUpStatus.Text = "you can use this one, its not being used.";
                        }      

the problem no longer occurs, but i need the code above to validate wether the username can be used or not.

can anyone help?
thanks
thomasmuttonAsked:
Who is Participating?
 
Edwin_CConnect With a Mentor Commented:
Sorry! The correct attack string should be " nobody'; DELETE * FROM tbl_user; SELECT ' ".  Anyway I hope you can see what I mean.
0
 
Edwin_CCommented:
The problem of the code is that even it finds a match, the while loop continues and hence the second if has a chance to show the "OK" message to overwrite the "error" message.  My recommendation is to find existing record directly rather than getting all records and then search one by one because it is very ineffective.

string sql = "SELECT count(*) FROM tbl_user WHERE user_name=@username";
MySqlCommand cmd = new MySqlCommand(sql, con);
cmd.Parameters.AddWithValue("@username", txtNewUsername.Text);
con.Open();
int count = (int)cmd.ExecuteScalar();
con.Close();
if (count>0) {
  lblSignUpStatus.Text = "Username already taken.";
}
else {
 lblSignUpStatus.Text = "you can use this one, its not being used.";
}

Hope that helps

Edwin

0
 
thomasmuttonAuthor Commented:
hello, i see what you are doing there but i seemed to have solved this by.

// CREATE THE COMMAND
                        string sql = "SELECT * FROM tbl_user WHERE user_name = '" + txtNewUsername.Text + "'";

                        // CONFIGURE THE CONNECTION
                        MySqlCommand cmd = new MySqlCommand(sql, con);

                        // OPEN THE DATABASE CONNECTION
                        con.Open();

                        // RUN THE QUERY
                        MySqlDataReader reader = cmd.ExecuteReader();

                        // PARSE THE RESULTS
                        if (reader.HasRows == true)
                        {
                              lblSignUpStatus.Text = "Username already taken.";                        
                        }
                        else if (reader.HasRows == false)
                        {
                              lblSignUpStatus.Text = "you can use this one, its not being used.";
                              SaveNewUser();
                        }

                        // CLOSE THE READER
                        reader.Close();

selecting the row where the username is = to what the user chose and then asking if that row exists or not.

it seems to work fine.
0
 
Edwin_CCommented:
Yes it also works but you have to watch for SQL injection attack in your approach.  Imagine what will happen if someone entered "nobody; DELETE * FROM tbl_user;" in txtNewUsername.  It will get the records in your table deleted.  Using parameters can avoid this.

Also, you are not going to read the record data from your database and so I think it is unnecessary to create an datareader object in this case.

That is my little opinion.
0
 
thomasmuttonAuthor Commented:
hello edwin and thankyou for your advice. I will try to fix the SQL injection. i will give you the points as you are the only person to have helped me. thanks
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.