CODEWITHSUNDEEP
c#asp.netdatabase
MNAH
MNAH

Reputation: 71

why data is not saving to database?

I have tried the following code to save to a database. The condition is are, I have a value in a dropdown list and the values are New= 1, and old=2. If the user selects 1 or new then it will save data to database or if they select old then it will show the exist data.

Now this time my label shows data inserted but the data is not saved to the table (But doesn't show any error).

 protected void btnsave_Click(object sender, EventArgs e)
        {
            if (ddl.Text=="1")
            {
                cs.Open();
                string query = "insert into resig (@id,@name,@email) values('"+txtgn.Text+"','"+txtgname.Text+"','"+txtsg.Text+"')";
                SqlCommand cmd = new SqlCommand(query,cs);
                lbdmsg.Text = "Data Inserted";

                //txtgname.Text = ddl.SelectedItem.ToString();

            }
            else
            {
                cs.Open();
                string query = "select name, email from resig where id='" + txtgn + "'";
                SqlCommand cmd= new SqlCommand(query,cs);
                dr =cmd.ExecuteReader();
                while(dr.Read())
                {
                    string name= txtgname.Text;
                    string email=txtsg.Text;
                }
                cs.Close();
            }
        }

Upvotes: 0

Views: 112

Answers (3)

Mark McGinty
Mark McGinty

Reputation: 756

Your SQL is both wrong, and very dangerous/susceptible to SQL injection. The first list in parenthesis must be a column list, and the values list should be parameters to avoid SQL injection:

string query = "insert into resig (id, name, email) values(@id, @name, @email)";
SqlCommand cmd = new SqlCommand(query, cs);
cmd.Parameters.Add(new SqlParameter("@id", txtgn.Text));
cmd.Parameters.Add(new SqlParameter("@name", txtgname.Text));
cmd.Parameters.Add(new SqlParameter("@email", txtsg.Text));
cmd.ExecuteNonQuery();

You should parameterize the select statement as well. Why is this important? Consider the resulting SQL if the user entered this for id and selected old:

'; delete resig; --

Building SQL by concatenating user input opens your database to the whim of users with bad intentions, and in this day and age should never be used. Countless web sites have been defaced and had their data corrupted -- it was ill-considered back in the day, but now we know better, and there's no excuse.

Upvotes: 1

Soner Gönül
Soner Gönül

Reputation: 98858

I see 2 things;

  • You are try to parameterize your column names, not your values.
  • You are not executing your insert command with ExecuteNonQuery().

You should use parameterized queries. This kind of string concatenations are open for SQL Injection attacks.

For example;

if (ddl.Text == "1")
{
    string query = "insert into resig (id,name,email) values(@id, @name, @email)";
    SqlCommand cmd = new SqlCommand(query,cs);
    cmd.Parameters.AddWithValue("@id", txtgn.Text);
    cmd.Parameters.AddWithValue("@name", txtgname.Text);
    cmd.Parameters.AddWithValue("@email", txtsg.Text);
    cs.Open();
    cmd.ExecuteNonQuery();
}

Upvotes: 3

Pavel Sem
Pavel Sem

Reputation: 1753

Call cmd.ExecuteNonQuery() to run the command on your db

Upvotes: 1

Related Questions