3

Is there an easier way to use the name and phone variables inside the below insert SQL Command?

String Interpolation is a way but I don't know how to implement this.

String name = textBox1.Text;
String phone = textBox2.Text;  
     
var query = "insert into Customer_info(Customer_Name,Customer_Phone) " +
            "values('" + name + "','" + phone + "');";
SqlCommand com = new SqlCommand(query,con);

try {
    con.Open();
    com.ExecuteNonQuery();
    con.Close();
}

catch (Exception Ex) {
    con.Close();
}
Improve this question
1
  • Instead of using concatenated string or string interpolation to build query string from user input, use parameters instead - see Little Bobby Tables. Commented Jan 8, 2019 at 9:14

3 Answers 3

Reset to default
6

What you really should do is use a parameterised query, so your query would look like this:

var query = "insert into Customer_info(Customer_Name,Customer_Phone)" +
"values(@name, @phone);";

You'd then use a SQLCommand object to pass the parameters to the query:

using (var command = new SqlCommand(query, connection))
{
    command.Parameters.AddWithValue("@name", name);
    command.Parameters.AddWithValue("@phone", phone);

    command.ExecuteNonQuery();
}

The reason for this is that it avoids the risk of SQL Injection (which is one of the OWASP Top 10). Consider for a moment your current query if the name passed in contained some SQL, for example if it contained:

'; DROP TABLE [Customer_info]; --

This would mean that your constructed SQL (if phone was blank) would look like this:

insert into Customer_info(Customer_Name,Customer_Phone) values ('';
DROP TABLE [Customer_Info];
-- ','');

This may well result in your Customer_Info table being dropped if the user that the code is connecting to SQL as has sufficient rights to do so.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

@GragasIncoming, no, it's really not how you're doing it... you're concatenating the values into query, I'm passing them in as parameters. Those are fundamentally different ways of constructing the query that is passed to SQL Server. Please for the sake of anyone who's a customer of the company this code is written for, take half an hour to get a handle on why concatenation is bad. Here's a page from OWASP that has a few more examples that may help: owasp.org/index.php/SQL_Injection
i think it requires connection.Open(); because it gives an error.
5

Don't do it! Seriously, just don't. String interpolation is not suitable for building SQL. Just use parameters:

var query = @"
insert into Customer_info(Customer_Name,Customer_Phone)
values(@name,@phone);";
//...
cmd.Parameters.AddWithValue("name", name);
cmd.Parameters.AddWithValue("phone", phone);
cmd.ExecuteNonQuery();

Or use a library like dapper (which removes all the messy ADO.NET code for you, like commands, parameters, and readers):

conn.Execute(query, new { name, phone });
Improve this answer

5 Comments

where are the try catch methods?
@GragasIncoming what am I catching and why? frankly, using is far more useful than try/catch - Rob's answer shows placement of that.
Then how would you output an error? E.g. inside catch function ex.Message;
@GragasIncoming if you mean re the try/catch in the question: they weren't in the question when I answered; also: your catch swallows the exception; also, it is pretty rare that you need to Close() a connection - it is far more often to use using for lifetime here. Note that re the dapper version: dapper will actually handle connection lifetime correctly anyway - if the connection isn't open when you call Execute, it'll open it, execute, and close it etc (it doesn't close it if it was open initially, obviously)
@GragasIncoming re "Then how would you output an error" - I'd probably catch that much closer to the code that is going to display the error; if your data access code and your UI code are intermixed, that's not great separation of concerns
4

To use string interpolation you need to write:

var query = $"insert into Customer_info(Customer_Name,Customer_Phone) values('{name}','{phone}');";

But, of course you are prone to SQL Injection, which you should avoid!

Use SqlCommand.Parameters collection to add parameter and be safe from that thread.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.