Already has an open DataReader

Question

I have a registration form for my new user:

Then, it also has a button:

When the button Register is clicked, I set command to check if the email already exists.

It does check the values and able to find the values in the debug process.

But an error occurs saying that

There is already an open DataReader associated with this Command which must be closed first.

Here is my ASP.NET markup:

<div id="success" runat="server" class="alert alert-success" visible="false">
            Account registration success!
            <br />
            Account sent for approval. Thank you.
</div>

<div id="emailavail" runat="server" class="alert alert-success" visible="false">
            Email is Available.
</div>

<div id="error" runat="server" class="alert alert-danger" visible="false">
            Email is already in use
            <br />
            Please choose another email / Login if already registered
</div>

<div class="form-group">
        <br />
            <label class="control-label col-lg-4">User Type</label>
            <div class="col-lg-8">
                <asp:Label ID="UserType" runat="server" class="form-control" text="Customer" required />
            </div>
</div>

<div class="form-group">
            <label class="control-label col-lg-4">Email</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtEmail" runat="server" class="form-control" type="email" 
                    MaxLength="80" required />
            </div>
</div>

<div class="form-group">
            <label class="control-label col-lg-4">Password</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtPassword" runat="server" class="form-control" TextMode="Password" MaxLength="20" required />
            </div>
</div>

<div class="form-group">
            <label class="control-label col-lg-4">First Name</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtFN" runat="server" class="form-control" MaxLength="80" required />
            </div>
</div>

<div class="form-group">
            <label class="control-label col-lg-4">Last Name</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtLN" runat="server" class="form-control" MaxLength="50" required />
            </div>
</div>

<div class="form-group">
            <label class="control-label col-lg-4">Company Name</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtCName" runat="server" class="form-control" MaxLength="50" required />
            </div>
</div>

<div class="form-group">
            <label class="control-label col-lg-4">Street</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtStreet" runat="server" class="form-control" MaxLength="50" required />
            </div>
</div>

<div class="form-group">
            <label class="control-label col-lg-4">Municipality</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtMunicipality" runat="server" class="form-control" MaxLength="100" required />
            </div>
</div>

        <div class="form-group">
            <label class="control-label col-lg-4">City</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtCity" runat="server" class="form-control" MaxLength="50" required />
            </div>
        </div>

        <div class="form-group">
            <label class="control-label col-lg-4">Company Phone</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtCPhone" runat="server" class="form-control" MaxLength="12" type="number" required />
            </div>
        </div>

        <div class="form-group">
            <label class="control-label col-lg-4">Mobile</label>
            <div class="col-lg-8">
                <asp:TextBox ID="txtMobile" runat="server" class="form-control" MaxLength="12" type="number" required />
            </div>
        </div>
    </div>
    <div class="col-lg-10">
        <span class="pull-right">
        <asp:Button ID="btnCancel" runat="server" class="btn" style="color:White" text="Cancel" PostBackUrl="~/Default.aspx" BackColor="Black" />
            <asp:Button ID="btnRegister" runat="server" class="btn btn-success" 
            Text="Register" style="color:White" onclick="btnRegister_Click" />

        </span>
    </div>

And here is the code-behind:

protected void btnRegister_Click(object sender, EventArgs e)
{
    con.Open();

    SqlCommand cmd = new SqlCommand();
    cmd.Connection = con;

    cmd.CommandText = "SELECT Email FROM Registration WHERE Email=@Email";
    cmd.Parameters.AddWithValue("@Email", txtEmail.Text);

    SqlDataReader dr = cmd.ExecuteReader();

    if (dr.HasRows)
    {
        error.Visible = true;
        emailavail.Visible = false;
        success.Visible = false;

        con.Close();
    }
    else
    {
        cmd.Connection = con;
        cmd.CommandText = "INSERT INTO Users VALUES (@TypeID, @Email, @Password, " +
            "@FirstName, @LastName, @CompanyName, @Street, @Municipality, @City, @CompanyPhone, @Mobile, " +
            "@Status, @DateAdded, @DateModified)";

        cmd.Parameters.AddWithValue("@TypeID", "2");
        cmd.Parameters.AddWithValue("@Email", txtEmail.Text);
        cmd.Parameters.AddWithValue("@Password", Helper.CreateSHAHash(txtPassword.Text));
        cmd.Parameters.AddWithValue("@FirstName", txtFN.Text);
        cmd.Parameters.AddWithValue("@LastName", txtLN.Text);
        cmd.Parameters.AddWithValue("@CompanyName", txtCName.Text);
        cmd.Parameters.AddWithValue("@Street", txtStreet.Text);
        cmd.Parameters.AddWithValue("@Municipality", txtMunicipality.Text);
        cmd.Parameters.AddWithValue("@City", txtCity.Text);
        cmd.Parameters.AddWithValue("@CompanyPhone", txtCPhone.Text);
        cmd.Parameters.AddWithValue("@Mobile", txtMobile.Text);
        cmd.Parameters.AddWithValue("@Status", "Pending");
        cmd.Parameters.AddWithValue("@DateAdded", DateTime.Now);
        cmd.Parameters.AddWithValue("@DateModified", DBNull.Value);
    }

    cmd.ExecuteNonQuery();
    con.Close();
}

I did try:

Removing my selectedindexchanged for Label NAME because it does SELECT the email from users again.
I tried moving the con.close because as what I have researched it has something to do there.

Please help it got me hours for this.

Thank you in advance!

I am still new to c#

You have open reader, but you try to send new command. Always close first connection and just open new one for INSERT. — Dmitry Razumikhin, Feb 28 '16 at 16:20
Or, use a "IF NOT EXISTS" syntax to check and insert actions in one SQL command. — Dmitry Razumikhin, Feb 28 '16 at 16:24
You need to enable [MultipleActiveResultSets](https://msdn.microsoft.com/en-us/library/cfa084cz(v=vs.110).aspx) — Steve, Feb 28 '16 at 16:27
where will i close it? i want to happen it in the else command (insert when the email is available) — Pauline Gail Mallari Chan, Feb 28 '16 at 16:43
Possible duplicate of [There is already an open DataReader associated with this Command which must be closed first](http://stackoverflow.com/questions/6062192/there-is-already-an-open-datareader-associated-with-this-command-which-must-be-c) — Dour High Arch, Feb 28 '16 at 17:55
@TomTom why can't i include it? isn't helpful for your further understanding as i believe being clear here is a must. — Pauline Gail Mallari Chan, Feb 29 '16 at 15:03
Because it is not helpfull, it is irrelevant noise. No one cares how the form looks and it takes half of the space of your question. Noise. Simply noise. — TomTom, Feb 29 '16 at 15:04

score 4 · Answer 1 · edited May 23 '17 at 12:16

Let's re-read your exception message again:

There is already an open DataReader associated with this Command which must be closed first

You have a few options.

You can manually dispose your reader after con.Close() line like

dr.Dispose();

or you can use using statement to dispose your reader automatically for that (which I recommended).

using(SqlDataReader dr = cmd.ExecuteReader())
{
   ...
} // <-- On this line your dr will disposed

or you can create a new SqlCommand object for your INSERT statement like;

cmd = new SqlCommand();
cmd.Connection = con;
...

Also realated: There is already an open DataReader associated with this Command which must be closed first

@PaulineGailMallariChan Why don't you try and see? – Soner Gönül Feb 28 '16 at 16:44 — Soner Gönül, Feb 28 '16 at 16:44

Nzall · Answer 2 · 2016-02-28T17:48:56.477

4

While Soner Gönül already gave a solution that works for this situation, I wanted to give another perspective that makes your entire question moot (the answer is also too long for a comment).

What you're doing here is good for learning purposes, but you're reinventing the wheel here. The 3 major web-based languages (PHP, Java and .NET) all aready have flexible and proven systems for user management.

Given that this is ASP.NET, I STRONGLY urge you to use the default ASP.NET Membership provider. The membership provider abstracts away all the user management, so you don't need to manually configure SQL commands. For example, I can see 2 glaring flaws in your code:

You're not salting your password hash;
You're using an outdated hashing algorithm in SHA. It should be bcrypt, Scrypt, PBKDF2 or the new Argon2 function.

Both of these issues are solved if you use the ASP.NET Membership provider.

edited Feb 28 '16 at 17:48

answered Feb 28 '16 at 16:39

Nzall

3,439
5
29
59

It is parameterized and therefore not vulnerable to an SQL injection exploit – paparazzo Feb 28 '16 at 17:01
@Frisbee My other points still stand though. He's not hashing properly for saving passwords and he's not salting his hashes. Both of these are handled by the Membership provider. The Membership provider also gives other benefits. – Nzall Feb 28 '16 at 17:12
And I did not comment on the other points. Why not just correct your answer? – paparazzo Feb 28 '16 at 17:14
@Frisbee Done. I was pressed for time and didn't have a good way to edit the answer. I wasn't sure if parameters used in this fashion are enough to fully prevent SQLi. All the guides I found on the matter also mention using SqlDbType and a max length. – Nzall Feb 28 '16 at 17:52
well i need the designs and customize my application so. i'm up for adventure and more learning to c# AND asp.net thanks for advice! – Pauline Gail Mallari Chan Feb 29 '16 at 15:00

paparazzo · Accepted Answer · 2016-02-29T17:21:28.827

This is just messed up on so many levels

dr.HasRows is false
it jumps to the else
no kidding you still have a DataReader open
you then repeat cmd.Connection = con;
dr.HasRows is true
you close the connection
then it jumps past the else
then you try a cmd.ExecuteNonQuery(); on a connection you have closed and a select statement that is not even a valid ExecuteNonQuery

There is no purpose to the DataReader in the first place
Just do count "SELECT count(*) FROM Registration WHERE Email=@Email"
Check the count and if 0 do the insert
Then close the connection

    cmd.CommandText = "SELECT count(*) FROM Registration WHERE Email=@Email";
    cmd.Parameters.AddWithValue("@Email", txtEmail.Text);
    if ((Int32)cmd.ExecuteScalar() > 0)
    {
        error.Visible = true;
        emailavail.Visible = false;
        success.Visible = false;
    }
    else
    {
        cmd.CommandText = "INSERT INTO Users VALUES (@TypeID, @Email, @Password, " +
            "@FirstName, @LastName, @CompanyName, @Street, @Municipality, @City, " +
            "@CompanyPhone, @Mobile, @Status, @DateAdded, @DateModified)";
        cmd.Parameters.AddWithValue("@TypeID", "2");
        cmd.Parameters.AddWithValue("@Email", txtEmail.Text);
        cmd.Parameters.AddWithValue("@Password", Helper.CreateSHAHash(txtPassword.Text));
        cmd.Parameters.AddWithValue("@FirstName", txtFN.Text);
        cmd.Parameters.AddWithValue("@LastName", txtLN.Text);
        cmd.Parameters.AddWithValue("@CompanyName", txtCName.Text);
        cmd.Parameters.AddWithValue("@Street", txtStreet.Text);
        cmd.Parameters.AddWithValue("@Municipality", txtMunicipality.Text);
        cmd.Parameters.AddWithValue("@City", txtCity.Text);
        cmd.Parameters.AddWithValue("@CompanyPhone", txtCPhone.Text);
        cmd.Parameters.AddWithValue("@Mobile", txtMobile.Text);
        cmd.Parameters.AddWithValue("@Status", "Pending");
        cmd.Parameters.AddWithValue("@DateAdded", DateTime.Now);
        cmd.Parameters.AddWithValue("@DateModified", DBNull.Value);
        cmd.ExecuteNonQuery();
    }
    con.Close();

could also do it in one trip but I am not positive that would work with parameters

IF NOT EXISTS(SELECT ...); 
 BEGIN 
  INSERT INTO ...;
 END;

thank you! i did it but i just decided to separate them. – Pauline Gail Mallari Chan Feb 29 '16 at 15:07 — Pauline Gail Mallari Chan, Feb 29 '16 at 15:07

score 0 · Answer 4 · answered Feb 29 '16 at 15:09

thanks for the help and kind consideration @Soner @Frisbee

what i did was separate them as i have thought about the answers i got and easier for me to understand when i get back to the same scenario.

 protected void btnRegister_Click(object sender, EventArgs e)
    {
        con.Open();
        SqlCommand cmd = new SqlCommand();
        cmd.Connection = con;

        cmd.CommandText = "SELECT Email FROM Users WHERE Email=@Email";
        cmd.Parameters.AddWithValue("@Email", txtEmail.Text);
        SqlDataReader dr = cmd.ExecuteReader();

        if (dr.HasRows)
        {
                error.Visible = true;
                emailavail.Visible = false;
                success.Visible = false;
                con.Close(); 
        }

        else
        {

            success.Visible = false;
            con.Close();
            RegisterUser();
        }



    }

    void RegisterUser()
    {
        con.Open();
        SqlCommand cmd = new SqlCommand();
        cmd.Connection = con;
        cmd.CommandText = "INSERT INTO Users VALUES (@TypeID, @Email, @Password, " +
            "@FirstName, @LastName, @CompanyName, @Street, @Municipality, @City, @CompanyPhone, @Mobile, " +
            "@Status, @DateAdded, @DateModified)";
        cmd.Parameters.AddWithValue("@TypeID", "2");
        cmd.Parameters.AddWithValue("@Email", txtEmail.Text);
        cmd.Parameters.AddWithValue("@Password", Helper.CreateSHAHash(txtPassword.Text));
        cmd.Parameters.AddWithValue("@FirstName", txtFN.Text);
        cmd.Parameters.AddWithValue("@LastName", txtLN.Text);
        cmd.Parameters.AddWithValue("@CompanyName", txtCName.Text);
        cmd.Parameters.AddWithValue("@Street", txtStreet.Text);
        cmd.Parameters.AddWithValue("@Municipality", txtMunicipality.Text);
        cmd.Parameters.AddWithValue("@City", txtCity.Text);
        cmd.Parameters.AddWithValue("@CompanyPhone", txtCPhone.Text);
        cmd.Parameters.AddWithValue("@Mobile", txtMobile.Text);
        cmd.Parameters.AddWithValue("@Status", "Pending");
        cmd.Parameters.AddWithValue("@DateAdded", DateTime.Now);
        cmd.Parameters.AddWithValue("@DateModified", DBNull.Value);
        cmd.ExecuteNonQuery();
        con.Close();

    }

thank you again guys

Already has an open DataReader

4 Answers4