0

May I know what's wrong with my statement? I receive a syntax error. Been trying to find out what's wrong all day. :(

cmd.CommandText = "INSERT INTO LogIn(Username,Password) VALUES('" + AddUsernameTextBox.Text + "','" + AddPasswordTextBox.Text + "')";
11
  • 5
    FYI, your allowing this code to permit SQL injection!!! Commented Feb 13, 2011 at 22:08
  • 4
    Print out the string after you build it and see if it's what you expected. Commented Feb 13, 2011 at 22:10
  • 2
    @pacheco, why are you trying something bad? You see the problem is not only hackers and SQL injection. It's also users entering quotes in some fields without knowing and making your system crash. So why not do things the way they are supposed to be? Commented Feb 13, 2011 at 22:10
  • 1
    Does your username or password contain a single quote? Commented Feb 14, 2011 at 4:21
  • 4
    You should look at parameterized queries... see csharp-station.com/Tutorials/AdoDotNet/Lesson06.aspx Commented Feb 14, 2011 at 4:30

9 Answers 9

4
cmd.CommandText = "INSERT INTO LogIn([Username],[Password]) VALUES('" + AddUsernameTextBox.Text + "','" + AddPasswordTextBox.Text + "')";

This code will help if the error is due to reserved keywords :- username and password. Please quote the error if this is not the case .

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

4 Comments

Think about what would happen if you use this code and I enter the password '); DROP TABLE LogIn;--? Parametrized queries are your friend.
You don't have to get hostile to have problems here, just Irish. What happens with the account for Mr. O'Neill?
@Greg: probably not much--Access doesn't allow multiple SQL statements to run together. Still not a good idea, since it would be horrible to upgrade to a DBMS that does without fixing it.
@RolandTumble - I didn't know that about Access, thanks. In fact I didn't even notice it was Access; my "Danger!" siren had already gone off.
3
  command.CommandText = "INSERT INTO Login([Username],[Password]) VALUES(@Username, @Password)";

  //Not sure how you create your commands in your project
  //here I'm using the ProviderFactory to create instances of provider specific DbCommands.

  var parameter = dbProviderFactory.CreateParameter();
  parameter.DbType = System.Data.DbType.String;
  parameter.ParameterName = "@Username";
  parameter.Value = AddUsernameTextBox.Text;
  command.Parameters.Add(parameter);

  parameter = dbProviderFactory.CreateParameter();
  parameter.DbType = System.Data.DbType.String;
  parameter.ParameterName = "@Password";
  parameter.Value = AddPasswordTextBox.Text;
  command.Parameters.Add(parameter);

Below is a more complete code sample of using ConnectionStringSettings and DbProviderFactory etc. This is not going to solve your problem, but this is the way to do data access if you're using ADO.NET core as you seem to be doing in your sample.

  ConnectionStringSettings connectionStringSettings = ConfigurationManager.ConnectionStrings["SomeConnectionName"];
  if (connectionStringSettings == null)
    throw new Exception("Application config file does not contain a connectionStrings section with a connection called \"SomeConnectionName\"");
  DbProviderFactory dbProviderFactory = DbProviderFactories.GetFactory(connectionStringSettings.ProviderName);
  using (var dbConnection = dbProviderFactory.CreateConnection())
  {
    dbConnection.ConnectionString = connectionStringSettings.ConnectionString;
    dbConnection.Open();
    using (var command = dbConnection.CreateCommand())
    {
      command.CommandText = "INSERT INTO Login([Username],[Password]) VALUES(@Username, @Password)";

      var parameter = dbProviderFactory.CreateParameter();
      parameter.DbType = System.Data.DbType.String;
      parameter.ParameterName = "@Username";
      parameter.Value = AddUsernameTextBox.Text;
      command.Parameters.Add(parameter);

      parameter = dbProviderFactory.CreateParameter();
      parameter.DbType = System.Data.DbType.String;
      parameter.ParameterName = "@Password";
      parameter.Value = AddPasswordTextBox.Text;
      command.Parameters.Add(parameter);

      var dbTransaction = dbConnection.BeginTransaction();
      try
      {
        command.ExecuteNonQuery();
        dbTransaction.Commit();
      }
      catch (Exception)
      {
        dbTransaction.Rollback();
        throw;
      }
    }
  }

the app.Config file that the code above relies on would look like this the following. Of course only the connectionStrings section in the config file is important in this context

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <connectionStrings>
    <add name="SomeConnectionName" providerName="System.Data.OleDb" connectionString="Your Provider Specific Connection String" />
  </connectionStrings>
</configuration>

Comments

1

The Best way is to use Parameters: '@' By this your code will look much more clearer and easy to understand. And makes your Application more secure.

try this code:

            using (var con = new OleDbConnection(_constring))
            {
                con.Open();
                using (
                    var cmd =
                        new OleDbCommand(
"UPDATE LogIn SET Username=@Username, Password=@Password WHERE (ID = @Id)",
                            con))
                {
                    try
                    {

                        cmd.Parameters.AddWithValue("@Username",EditUsernameTextBox.Text);
                        cmd.Parameters.AddWithValue("@Password",EditPasswordTextBox.Text);
                        cmd.Parameters.AddWithValue("@Id",IDTextBox.Text);


                        cmd.ExecuteNonQuery();
                    }
                    catch (Exception ex)
                    {
                        throw;
                    }
                    finally
                    {
                        con.Close();
                    }

                }

Regards!

1 Comment

AFAIK you cannot use named parameters with Jet (the OP is tagged ms-access) and OLEDB, you must replace the parameters with a question mark and refer to them in the order in which they occur.
1

Please protect the single quotes. Also, you may need a closing semicolon in the Access SQL string.

cmd.CommandText = "INSERT INTO LogIn(Username,Password) VALUES('" + AddUsernameTextBox.Text.Replace("'","''") + "','" + AddPasswordTextBox.Text.Replace("'","''") + "');";

It is of course only 100% better to use parameterized queries; from you other questions is this C#/Visual Studio against MS Access through OLE/Jet?

2 Comments

yup it's C# in visual studio and MS Access through OLE. Tried your statement but i still receive syntax error :(
@Pac - please see comment against question
0

Is ID column an integer? If not you need to wrap values in single quotes, too. Also, try removing the parentheses.

Comments

0

Most likely, the value in IDTextBox.text is not numeric...

But like Daniel points out, this is very vulnerable to SQL inject..

What would happen if I typed:

' ; DROP TABLE login

in the EditUserNameTextBox field

7 Comments

It would fail. Access will not run more than one statement, but I am quibbling :)
THanks Remou, I didn't know that. Just wanted to give an example of how dangerous SQL injection can be
With Jet/ACE, it's not very dangerous, as no DDL or DML statements can injected via a WHERE clause. However, it is possible to change the number of rows acted upon, which can be dangerous in a case like this with a SQL UPDATE. For more on the specifics with Access see stackoverflow.com/questions/512174/non-web-sql-injection/… .
I guess ' -- would be dangerous, effectively emptying out all user name fields... Don't use access myself however
Er, how would '-- be dangerous in Access? It doesn't mean anything special at all in Jet/ACE/Access SQL, so you'd just be passing criteria that wouldn't return any rows.
|
0

Check if this works. You were missing single quotes for the value in your WHERE statement:

"UPDATE
    LogIn
SET
    Username = '" + EditUsernameTextBox.Text + "'
    ,Password = '" + EditPasswordTextBox.Text + "'
WHERE
    (ID = '" + IDTextBox.Text + "')";

Plus, make sure, as Daniel White mentioned, you take care of any SQL Injection.

4 Comments

still receiving syntax error :/ could it be because my ID DataType is AutoNumber?
Can you provide the data you're passing in? Are you passing in anything with an apostrophe (single quote) or any other possibly problematic characters?
i typed in all alphabets in the textboxes
That's at least one problem. If you're trying to compare an AutoNumber field to alphabetical characters, then you'll receive an error. Try removing the single quotes from the WHERE clause, and then just type in numerals to test it. Otherwise, your query is essentially syntactically sound.
0

You missed a pair of apostrophes, if your ID is non-numeric:

WHERE (ID ='" + IDTextBox.Text + "')";

2 Comments

syntax error still. could it be because my ID DataType is AutoNumber?
If ID is an autonumber, it is numeric and does not need quotes, it will not cause your statement to fail. Print out the statement as requested and post it.
0

Do the values of EditUsernameTextBox.Text or EditPasswordTextBox.Text themselves have quotes? This will bollix up the SQL.

If so, you'll need to escape them. or don't use string concatenation as pointed out already...

And have you printed the statement out to see what it looks like as requested...?

2 Comments

how do i print the statement? sorry newbie here
You can use the immediate window in Visual Studio, I think it is what you are using.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.