3

i am creating password change form. when i execute the form and fill the textboxes it give an exception with the message There is already and open DataReader associated with this command which must be closed first.

he is the code which i am using:

private bool CompareStrings(string string1, string string2)
        {
            return String.Compare(string1, string2, true, System.Globalization.CultureInfo.InvariantCulture) == 0 ? true : false;
        }

    private void button1_Click(object sender, EventArgs e)
    {
        try
        {
            SqlConnection con1 = new SqlConnection();
            con1.ConnectionString = "data source=.;Initial catalog=inventory;Integrated Security=true";
            con1.Open();

            SqlCommand cmd = new SqlCommand("SELECT ISNULL(username, '') AS username, ISNULL(password,'') AS password FROM login WHERE username='" + textBox1.Text + "' and password='" + textBox2.Text + "'", con1);

            SqlDataReader dr = cmd.ExecuteReader();

            string userText = textBox1.Text;
            string passText = textBox2.Text;

            while (dr.Read())
            {
                if (this.CompareStrings(dr["username"].ToString(), userText) &&
                    this.CompareStrings(dr["password"].ToString(), passText))
                {
                    SqlCommand cmd2 = new SqlCommand("UPDATE login SET password='" + textBox3.Text + "'where username='" + textBox1.Text + "'", con1);
                    cmd2.ExecuteNonQuery();
                    MessageBox.Show("Password Changed Successfully");
                }
                else
                {
                    MessageBox.Show("Incorrect Old password");                        
                }

            }

            dr.Close();

            con1.Close();

        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
Szymon
  • 42,577
  • 16
  • 96
  • 114
Haider Khattak
  • 567
  • 3
  • 8
  • 32

2 Answers2

2

You cannot execute a command while a SqlDataReader is open on the same connection. You can do either of the two following things to change your code:

  1. Create a second connection and run the update query on that second connection.

  2. Store the data from the reader, close the reader and later update all data. In your case, you could store all usernames to update and update them in one update query using Username in (<yourlisthere>)

Szymon
  • 42,577
  • 16
  • 96
  • 114
2

When you open a DataReader the connection serves only the requests coming from the DataReader. The SqlCommand used to Update the login table cannot run.

Unless you add this to your connectionstring

MultipleActiveResultSets = True;

Here you can find the reference to MARS

And here the words from MSDN about the DataReader

While the SqlDataReader is being used, the associated SqlConnection is busy serving the SqlDataReader, and no other operations can be performed on the SqlConnection other than closing it. This is the case until the Close method of the SqlDataReader is called. For example, you cannot retrieve output parameters until after you call Close.

As a side note, but very important. Do not use string concatenation to build sql commands. Use always a parameterized query

string cmdText = "UPDATE login SET password=@pwd where username=@usr";
using(SqlCommand cmd2 = new SqlCommand(cmdText, con1))
{
    cmd2.Parameters.AddWithValue("@pwd", textBox3.Text);
    cmd2.Parameters.AddWithValue("@usr", textBox1.Text);
    cmd2.ExecuteNonQuery();    
}

A parameterized query will avoid Sql Injection problems and let you simplify your command text.
This is true also for the SELECT query at the beginning of your code. Do not trust the input coming from your user

Another problem that you should be aware of is the storing of clear text passwords in your database. This is considered a very bad practice from a security point of view. You should apply an hash function to you password and store the result. While checking for the correct password you repeat the hash function on the user input and check the result against the hashed password stored in database

Community
  • 1
  • 1
Steve
  • 213,761
  • 22
  • 232
  • 286
  • thanks for the additional info about query. i was not aware of this fact. thanks for the enhancement in my knowledge and the solution you provided worked. – Haider Khattak Oct 28 '13 at 20:55
  • can you please provide me a link where i can learn using hash functions because i am unaware of it... – Haider Khattak Oct 28 '13 at 20:57
  • 1
    [This is one old question](http://stackoverflow.com/questions/13450201/working-with-registry-in-c-sharp-2-0-windows-forms/13450315#13450315) that cover something about hashing and crypting – Steve Oct 28 '13 at 20:58
  • 1
    [This is instead a more advanced question](http://stackoverflow.com/questions/2138429/hash-and-salt-passwords-in-c-sharp) and answer about password hashing – Steve Oct 28 '13 at 21:01