SqlException was unhandled by user code incorrect syntax near ','

Question

using System;
using System.Collections;
using System.Configuration;
using System.Data;
using System.Web;
using System.Web.Security;
using System.Web.UI;
using System.Web.UI.HtmlControls;
using System.Web.UI.WebControls;
using System.Web.UI.WebControls.WebParts;
using System.Data.SqlClient;


public partial class Student_InsertStudentDeta : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        lblMsg.Visible = false;
    }

    protected void btnSave_Click(object sender, EventArgs e)
    {
        btnDelete.Enabled = false;
        btnFindValuse.Enabled = false;
        btnUpdate.Enabled = false;

        SqlConnection connection = new SqlConnection();
        connection.ConnectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
        connection.Open();

        SqlCommand cmd = new SqlCommand("Insert into MoHE_Student values (N'" + txtName.Text + "',N'" + txtSureName.Text + "',N'" + txtFatherName.Text + "',N'" + txtGFatherName.Text + "',N'" + txtBirthYear.Text + "',N'" + txtNIDC.Text + "',N'" + txtTabdily.Text + "',N'" + txtTitalMonugraf.Text + "',N'" + ddlJeldBook.SelectedItem.Text + "',N'" + txtDescription.Text + "',N'" + ddlUniversity.SelectedItem.Value + "',N'" + ddlFaculty.SelectedItem.Value + "',N'" + ddlDepartment.SelectedItem.Value + "'," + txtStudentRegBook.Text + "," + txtPageBook.Text + "," + ddlReciveBook.SelectedItem.Text + "," + txtGraduateYear.Text + "," + txtRegYear.Text + ",N'" + ddlKoncurExam.SelectedItem.Text + "',N'" + ddlDefaMonugraf.SelectedItem.Text + "',N'" + ddlYearDefa.SelectedItem.Text + "',N'" + ddlMonthDefa.SelectedItem.Text + "',N'" + ddlDayDefa.SelectedItem.Text + "',N'" + ddlTakeDiplom.SelectedItem.Text + "',N'" + txtPhonNum.Text + "',N'" + ddlGender.SelectedItem.Text + "',N'" + ddlDarajaTahsili.SelectedItem.Text + "',N'" + User.Identity.Name + "')", connection);

        cmd.ExecuteNonQuery(); // Error happens here
    }
}

[SQL Injection alert](http://msdn.microsoft.com/en-us/library/ms161953%28v=sql.105%29.aspx) - you should **not** concatenate together your SQL statements - use **parametrized queries** instead to avoid SQL injection — marc_s, Oct 23 '17 at 10:42
@badruddin: please tag this with an appropriate language, and remove the `html` tag. — halfer, Oct 23 '17 at 10:43
Things like `" + txtPageBook.Text + "` should be `'" + txtPageBook.Text + "'`, so a single quote is required to wrap concatenated string parameters, with N or without N. Use parameterized queries or stored procedures instead of this approach. — Amit Kumar Singh, Oct 23 '17 at 11:43
The error is pretty clear, and so is the reason. Whatever `txtName.Text` contains, it has at least one `'` and one `,`. Imagine what would happen if it contained `'; DROP TABLE MoHE_Student;--`. That's why you should *not* concatenate strings to create a sql statement. Use parameterized queries` — Panagiotis Kanavos, Oct 23 '17 at 11:49
This is precisely what [Bobby Tables](https://xkcd.com/327/) refers to. — Panagiotis Kanavos, Oct 23 '17 at 11:51
Possible duplicate of [Why do we always prefer using parameters in SQL statements?](https://stackoverflow.com/questions/7505808/why-do-we-always-prefer-using-parameters-in-sql-statements) — Panagiotis Kanavos, Oct 23 '17 at 11:52
To add to what is written in other comments, you should always use the `using` statement when dealing with instances that implements the `IDisposable` interface - in your case that's `SqlConnection` and `SqlCommand`. — Zohar Peled, Oct 23 '17 at 12:02

score 0 · Answer 1 · answered Oct 23 '17 at 12:15

The duplicate explains what's wrong, how serious this is and how to avoid it. I won't repeat the explanation here, simply because I started to, then my fingers started tingling. The various answers to the duplicate are adequate and even contain Bobby Tables.

This just shows how this particular code can be fixed.

First, the command can be created once and reused:

SqlCommand _insertCmd;

void InitCommands()
{ 

    _connectionString = 
          ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;

    var query = "Insert into MoHE_Student values (@name,@surname,...";
    var cmd = new SqlCommand(query);
    cmd.Parameters.Add("@name",SqlDbType.NVarChar,30);
    cmd.Parameters.Add("@surname",SqlDbType.NVarChar,30);
    ...
    _insertCmd=cmd;

}

When the time comes to execute it, set the parameter values and open a connection inside a using block to ensure the connection is closed even if an error occurs :

_insertCmd["@name"].Value=txtName.Text;
...

using(var connection = new SqlConnection(_connectionString))
{
      _insertCmd.Connection=connection;  
      connection.Open();
      _insertCmd.ExecuteNonQuery(); 
}

SqlException was unhandled by user code incorrect syntax near ','

1 Answers1