Reputation: 3
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
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