Number of query values and destination fields are not the same. where is the problem in the code?

Question

I have a sign up page that when you submit you go to here but it says: "Number of query values and destination fields are not the same." why??? heres the code:

                 string MySQL = "INSERT INTO users (email, pname, 
accountname, pid, age, passw) VALUES ('";
             MySQL = MySQL + Request.Form["email"] + "','";
             MySQL = MySQL + Request.Form["pname"] + "','";
             MySQL = MySQL + Request.Form["accountname"] + "','";
             MySQL = MySQL + Request.Form["pid"] + "','";
             MySQL = MySQL + Request.Form["age"] + "','";
             MySQL = MySQL + Request.Form["passw"] + "','";

             string strConnection = "Provider=Microsoft.ACE.OLEDB.12.0;Data 
Source=" + 
System.Web.HttpContext.Current.Server.MapPath(@"DataBase\Users.accdb");
             System.Data.OleDb.OleDbConnection con = new 
System.Data.OleDb.OleDbConnection(strConnection);
             System.Data.OleDb.OleDbCommand cmd = new 
System.Data.OleDb.OleDbCommand(MySQL, con);

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

Try rewriting the 8th line as follows: `MySQL = MySQL + Request.Form["passw"] + "')";`. But know that your code is open to [SQL injection](https://en.wikipedia.org/wiki/SQL_injection). Rewrite code using the [`SQLCommand.Parameters`](https://learn.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlcommand.parameters?view=netframework-4.8) property. — Andrei Odegov, Jun 02 '19 at 15:38

score 0 · Accepted Answer · answered Jun 02 '19 at 15:19

You have a simple syntax error in your query statement. You add a comma after the last value and forgot to close the VALUES statement. But this, while easy to fix, is not the correct way to build an sql statement.

You should always use a parameterized query and never forget to dispose the objects like the connection when you have done with them.

 string MySQL = @"INSERT INTO users 
                (email, pname, accountname, pid, age, passw) 
                 VALUES(@email, @pname,@account,@pid, @age, @passw)";
 string strConnection = "......";
 using(OleDbConnection con = new OleDbConnection(strConnection))
 using(OleDbCommand cmd = new OleDbCommand(MySQL, con))
 {
     con.Open();
     cmd.Parameters.Add("@email", OleDbType.VarWChar).Value = Request.Form["email"];
     cmd.Parameters.Add("@pname", OleDbType.VarWChar).Value = Request.Form["pname"];
     cmd.Parameters.Add("@account", OleDbType.VarWChar).Value = Request.Form["accountname"];
     cmd.Parameters.Add("@pid", OleDbType.VarWChar).Value = Request.Form["pid"];
     cmd.Parameters.Add("@age", OleDbType.VarWChar).Value = Request.Form["age"];
     cmd.Parameters.Add("@passw", OleDbType.VarWChar).Value = Request.Form["passw"];
     cmd.ExecuteNonQuery();
 }

Now this approachs while it seems more verbose is a lot more clear. No more concatenation of stringe pieces with the proper quotes around these values. No more silly syntax errors and no one could write an sql injection hack against your database using the input values. See Sql Injection

There is another problem for the secutiry point of view. You have clear password stored in the database and this is not considered a good practice.
See Best way to store passwords in a database

Number of query values and destination fields are not the same. where is the problem in the code?

1 Answers1