having trouble adding data to table

Question

0.00/5 (No votes)

See more:

hey guys , i get a error when adding data, - i have 2 textboxes and the entered data inside the textboxes shud be inserted into a sqls table, right, so i get a message on the 2nd text box \\

************* Exception Text **************
System.Data.SqlClient.SqlException: Incorrect syntax near '12'.
   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj)
   at System.Data.SqlClient.TdsParser.Run(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj)
   at System.Data.SqlClient.SqlCommand.RunExecuteNonQueryTds(String methodName, Boolean async)
   at System.Data.SqlClient.SqlCommand.InternalExecuteNonQuery(DbAsyncResult result, String methodName, Boolean sendToPipe)
   at System.Data.SqlClient.SqlCommand.ExecuteNonQuery()
   at login_form.add_user.button1_Click(Object sender, EventArgs e) in C:\Users\james\New folder\POS\pos\login form\add user.cs:line 25
   at System.Windows.Forms.Control.OnClick(EventArgs e)
   at System.Windows.Forms.Button.OnMouseUp(MouseEventArgs mevent)
   at System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons button, Int32 clicks)
   at System.Windows.Forms.Control.WndProc(Message& m)
   at System.Windows.Forms.ButtonBase.WndProc(Message& m)
   at System.Windows.Forms.Button.WndProc(Message& m)
   at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m)
   at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

heres my code :

C#

SqlConnection con = new SqlConnection("Data Source=JAMES-PC\\SQLEXPRESS;Initial Catalog=login1;Integrated Security=True");
con.Open();
SqlCommand cmd = new SqlCommand("INSERT INTO user1 (username, password) VALUES ('" + textBox1.Text + "','" + textBox2.Text + "'", con);
cmd.ExecuteNonQuery();
con.Close();

2ndly i know my code aint very sql injection proof , reading up on it :)

Posted 31-Jul-15 1:22am

jamesmc1535

Updated 31-Jul-15 1:39am

Suvendu Shekhar Giri

v3

Add a Solution

Comments

Richard Deeming 31-Jul-15 7:50am

Your code is vulnerable to SQL Injection[^].

NEVER use string concatenation to build a SQL query. ALWAYS use a parameterized query.

jamesmc1535 31-Jul-15 7:52am

thanks for the advice, im stil reading up on parameterized .

Richard Deeming 31-Jul-15 7:51am

You're also storing passwords in plain text. That's an extremely bad idea. You should only ever store a salted hash of the user's password.

Secure Password Authentication Explained Simply[^]
Salted Password Hashing - Doing it Right[^]

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Suvendu Shekhar Giri · Accepted Answer · 2015-07-31T01:38:00

Solution 1

You have forgetten to put a close bracket after VALUES ends.
Check this-

C#

SqlCommand cmd = new SqlCommand("INSERT INTO user1 (username, password) VALUES ('" + textBox1.Text + "','" + textBox2.Text + "')", con);

Hope, it helps :)

UPDATE:
As you know this is vulnerable to SQL Injection, spend little time (not more) to convert this to a parameterized query.

C#

SqlCommand cmd = new SqlCommand("INSERT INTO user1 ([username], [password]) VALUES (@username,@password)", con);
cmd.Parameters.AddWithValue(@username,textBox1.Text);
cmd.Parameters.AddWithValue(@password,textBox2.Text);

Reference: Using Parameterized queries to prevent SQL Injection Attacks in SQL Server[^]

Hope, you'll consider this update :)

Posted 31-Jul-15 1:38am

Suvendu Shekhar Giri

Updated 31-Jul-15 2:00am

v4

Comments

jamesmc1535 31-Jul-15 7:50am

JEEZ lotta help thanks man
something as simple as that :| and it works

Suvendu Shekhar Giri 31-Jul-15 7:53am

Glad that it helped :)

Richard Deeming 31-Jul-15 7:52am

You have copied the SQL Injection[^] vulnerability from the question.

NEVER use string concatenation to build a SQL query. ALWAYS use a parameterized query.

Suvendu Shekhar Giri 31-Jul-15 7:59am

Noted. Updated the solution.

Richard Deeming 31-Jul-15 8:01am

Thanks. Updated my vote. :)

jamesmc1535 31-Jul-15 8:08am

hey bud i just used the updated ,cmd.Parameters.AddWithValue(@username, textBox1.Text);
cmd.Parameters.AddWithValue(@password, textBox2.Text); but i get eror on username and password - does not exist

Suvendu Shekhar Giri 31-Jul-15 8:30am

Make sure that you have added the parameters before this line
cmd.ExecuteNonQuery();