chimon123
chimon123

Reputation: 3

Connection is not closed properly ASP.NET C#

I have this button click event. Been trying to replace the con.Close() in different lines of code, tried for hours but couldn't fix. Maybe a second pair of eyes can help?

Error: System.InvalidOperationException: 'The connection was not closed. The connection's current state is open.'

protected void Button1_Click(object sender, EventArgs e)
{
    SqlConnection con = new SqlConnection();
    con.ConnectionString = ConfigurationManager.ConnectionStrings["ConnStr"].ConnectionString;
    con.Open();

    string query = "SELECT CATEGORY FROM CATEGORY WHERE C_UserName = '" + Session["id"] + "'  AND  CATEGORY = '" + DropDownList1.SelectedItem.Value + "' ";
    SqlCommand cmd = new SqlCommand(query, con);

    SqlDataReader reader = cmd.ExecuteReader();
    if (reader.HasRows)
    {
        cmd.Parameters.AddWithValue("@CATEGORY", DropDownList1.SelectedItem.Value);

        lblResult.Text = "You have selected this category. Please select a new category";
        con.Close();
    }
    else
    {
        SqlCommand cmd1 = new SqlCommand("UPDATE SET CATEGORY CCID@CCID (CATEGORY, C_USERNAME, CCID) VALUES (@CATEGORY, @C_USERNAME, @CCID)", con);
        cmd1.Parameters.AddWithValue("CATEGORY", DropDownList1.SelectedItem.Value);
        cmd1.Parameters.AddWithValue("C_USERNAME", Session["id"]);
        cmd1.Parameters.AddWithValue("CCID", Label1.Text);

        con.Open();
        int i = cmd1.ExecuteNonQuery();
        con.Close();

        if (i != 0)
        {
            Label2.Text = " Your data is been saved in the database";
            Label2.ForeColor = System.Drawing.Color.ForestGreen;
        }
        else
        {
            Label2.Text = "Something went wrong with selection";
            Label2.ForeColor = System.Drawing.Color.Red;
        }
    }
}

Upvotes: 0

Views: 405

Answers (1)

Athanasios Kataras
Athanasios Kataras

Reputation: 26342

Try this (open connection only once and close only once):

protected void Button1_Click(object sender, EventArgs e) {
  using(SqlConnection con = new SqlConnection()) {
    con.ConnectionString = ConfigurationManager.ConnectionStrings["ConnStr"].ConnectionString;

    string query = "SELECT CATEGORY FROM CATEGORY WHERE C_UserName = '" + Session["id"] + "'  AND  CATEGORY = '" + DropDownList1.SelectedItem.Value + "' ";
    SqlCommand cmd = new SqlCommand(query, con);
    con.Open();
    SqlDataReader reader = cmd.ExecuteReader();
    bool hasRows = reader.HasRows;
    reader.Close();
    if (hasRows) {
      // This line makes no sense after the execution of the query.
      //cmd.Parameters.AddWithValue("@CATEGORY", DropDownList1.SelectedItem.Value);
      
      lblResult.Text = "You have selected this category. Please select a new category";
    } else {
      SqlCommand cmd1 = new SqlCommand("UPDATE SET CATEGORY CCID@CCID (CATEGORY, C_USERNAME, CCID) VALUES (@CATEGORY, @C_USERNAME, @CCID)", con);
      cmd1.Parameters.AddWithValue("CATEGORY", DropDownList1.SelectedItem.Value);
      cmd1.Parameters.AddWithValue("C_USERNAME", Session["id"]);
      cmd1.Parameters.AddWithValue("CCID", Label1.Text);

      int i = cmd1.ExecuteNonQuery();

      if (i != 0) {

        Label2.Text = " Your data is been saved in the database";
        Label2.ForeColor = System.Drawing.Color.ForestGreen;

      } else {
        Label2.Text = "Something went wrong with selection";
        Label2.ForeColor = System.Drawing.Color.Red;

      }
    }
    con.Close();
  }
}

Now let's discuss this line

    string query = "SELECT CATEGORY FROM CATEGORY WHERE C_UserName = '" + Session["id"] + "'  AND  CATEGORY = '" + DropDownList1.SelectedItem.Value + "' ";

This let's attacker manipulate your input with sql injection. To solve this, use the same cmd1.Parameters.AddWithValue("CATEGORY", DropDownList1.SelectedItem.Value); that you are using in the second query. The Session["id"] is somewhat safer as it is not provided by the user but better safe than sorry as the parameters sanitize the input and protect you from sql injection.

Upvotes: 1

Related Questions