connection erron:connection was not closed.connection state open

Question

0.00/5 (No votes)

See more:

C#

using System;
using System.Collections.Generic;
using System.Linq;
using System.Configuration;
using System.Data.SqlClient;
using System.Web;
using System.Web.UI;
using System.Web.UI.WebControls;

namespace projectDressDesign
{
    public partial class registration : System.Web.UI.Page
    {
        SqlConnection myConnection = new SqlConnection(ConfigurationManager.ConnectionStrings["DefaultConnectionString"].ConnectionString);

        protected void Page_Load(object sender, EventArgs e)
        {

        }
        protected void registerBtn_Click1(object sender, EventArgs e)
        
            {
                Page.Validate();
                if (Page.IsValid)
                {
                    myConnection.Open();

                    string userInvalid = "The username entered is invalid, please choose another.";

                    string checkDatabase = "SELECT * FROM Users WHERE Email_Id = @em AND Password = @pas";
                    SqlCommand command = new SqlCommand(checkDatabase, myConnection);
                    command.Parameters.AddWithValue("@em", email.Text);
                    command.Parameters.AddWithValue("@pas", password.Text);
                    command.ExecuteNonQuery();
                    SqlDataReader reader = command.ExecuteReader();

                    if (reader.HasRows)
                    {
                        outputlabel.Text = userInvalid;
                        myConnection.Close();
                    }
                    
                
                else
                {
                   
                    outputlabel.Text = "Succesfully Registered";

                    string query = "Insert into Users (FirstName,LastName,Gender,DateOfBirth,Email_Id,Password) Values (@fn,@ln,@gen,@dob,@em,@pas)";
                    myConnection.Open();
                    SqlCommand insertCommand = new SqlCommand(query, myConnection);
                    insertCommand.Parameters.AddWithValue("@fn", fname.Text);
                    insertCommand.Parameters.AddWithValue("@ln", lname.Text);
                    insertCommand.Parameters.AddWithValue("@gen", gender_dd.SelectedItem.Text);
                    insertCommand.Parameters.AddWithValue("@dob", date.Text);
                    insertCommand.Parameters.AddWithValue("@em", email.Text);
                    insertCommand.Parameters.AddWithValue("@pas", password.Text);

                    insertCommand.ExecuteNonQuery();

                    myConnection.Close();
                    Response.Redirect("login.aspx");
                }
     
            }

        }
    }
}

Posted 29-Aug-15 9:31am

Member 11943373

Updated 29-Aug-15 9:50am

Patrice T

v2

Add a Solution

Comments

Patrice T 29-Aug-15 15:50pm

No repost please

2 solutions

Solution 2

I believe there are some problems with the code that you should go through. You have correctly used parameters, but have a look at following points.

string checkDatabase = "SELECT * FROM Users WHERE Email_Id = @em AND Password = @pas";
SqlCommand command = new SqlCommand(checkDatabase, myConnection);
command.Parameters.AddWithValue("@em", email.Text);
command.Parameters.AddWithValue("@pas", password.Text);

It looks like you're storing the password as plain text in your database. This is something you should never do. The passwords should be stored only using a one-way encryption so that the original password cannot be revealed. When verifying the user you don't need to know the password, you just need to know if it is correct. Have a look at Password Storage: How to do it.[^]

Another thing is how you store the connection in your class. You should use connection variable which would be scoped only to the method where you need it. Also to properly dispose the connection you should use a using[^] statement. And since you use the database connection anyway, why not open the connection before entering the if. In other words, something like:

C#

protected void registerBtn_Click1(object sender, EventArgs e)
{
   Page.Validate();
   using (SqlConnection myConnection = new SqlConnection(ConfigurationManager.ConnectionStrings["DefaultConnectionString"].ConnectionString))
   {
      if (Page.IsValid)
      {
         ...
      }
      else
      {
         ...
      }     
   }
}

For more discussion, see Version 2, close and dispose database objects[^]. Also you would need to add proper error handling...

string userInvalid = "The username entered is invalid, please choose another.";

string checkDatabase = "SELECT * FROM Users WHERE Email_Id = @em AND Password = @pas";
SqlCommand command = new SqlCommand(checkDatabase, myConnection);
command.Parameters.AddWithValue("@em", email.Text);
command.Parameters.AddWithValue("@pas", password.Text);
command.ExecuteNonQuery();
SqlDataReader reader = command.ExecuteReader();

if (reader.HasRows)
{
outputlabel.Text = userInvalid;
myConnection.Close();
}

The third thing is identifying the user. If I interpret your code correctly, you allow people to use the same id if the password is different. In my opinion it would be feasible to allow each id only once. The id is static whereas the password changes over time. For example if the user forgets the passwords and wants to reset it and you have two users with the same id in the system, which one will you reset?

Posted 5-Sep-15 18:52pm

Wendelius

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Arasappan · Accepted Answer · 2015-08-29T16:15:00

Pls change code ;

You used .. myconnection.open ();
Delete that line..
Before condition loop..you open.. once again it open in else condition...
Before open.. atleast close the connection..

Else
{
Connection.close ();
Connection.open ();
....
....

.
Connection.close ();

}

On entering into else condition

............................................................................................

After that
Reader.ExecuteReader ();
Reader.Read ()
If (reader.hasrows)
{
Reader.close ();
}

Else
{
Reader.close ();
}