0

When i try to connect to my database to Edit a datatable in MVC. When I try to acces to my view i have an error when I execute my command. the error is:

System.Data.SqlClient.SqlException: 'incorrect syntax near ('. incorrect syntax near the kewword SET.

but i can not figure out my syntax errors. I am a Beginer so i am still learning the basis. It would be really grateful for any help. Thanks!. here is my code

private void UpdateDataBase(int EmailId, string userName, string title, string Email, string description)
{
    var sqlstring = string.Format("UPDATE Email (Email, Description, UserName, Title) " +
        "SET ('{0}',  '{1}',  '{2}',  '{3}')", Email, description, userName, title +
        "WHERE ID=" + EmailId);

    var myConnection = getconection();
    SqlCommand myCommand = new SqlCommand(sqlstring, myConnection);
    myCommand.ExecuteNonQuery();

    try
    {
        myConnection.Close();
    }
    catch (Exception e)
    {
        Console.WriteLine(e.ToString());
    }
}
public ActionResult Edit (int EmailId, string userName, string title, string Email, string description)
{          
    UpdateDataBase(EmailId, userName, title, Email, description);

    return View("EmailData");
}

[HttpPost]
public ActionResult Edit (ModelTemplateEmail  EditEmailData)
{       
    if (ModelState.IsValid)
    {                
      return RedirectToAction("EmailData");
    };
    return View(EditEmailData);
}
haldo
  • 14,512
  • 5
  • 46
  • 52
Lisa
  • 31
  • 7
  • 1
    **Warning:** Your code is vulnerable to SQL Injection attacks. You should use parameterised queries and prepared statements to help prevent attackers from compromising your database by using malicious input values. http://bobby-tables.com gives an explanation of the risks, as well as some examples of how to write your queries safely using C# / ADO.NET. **Never** insert unsanitised data directly into your SQL. The way your code is written now, someone could easily steal, incorrectly change, or even delete your data. – ADyson Mar 23 '20 at 10:20
  • 1
    parameterised queries https://stackoverflow.com/questions/12142806/how-to-insert-records-in-database-using-c-sharp-language/12142974#12142974 – freedomn-m Mar 23 '20 at 10:22
  • 2
    Not directly related to the question: You're calling `UpdateDataBase` on the GET function - with MVC you generally have a GET that gives you a partial view where you enter data, then a related POST where you check `ModelState.IsValid` and, if valid, save the data - so your `UpdateDatabase` call should be in the `[HttpPost]` overload. – freedomn-m Mar 23 '20 at 10:24
  • 1
    Anyway you need to re-read your tutorials / examples or whatever, because you've muddled up the query structure for UPDATE and INSERT. – ADyson Mar 23 '20 at 10:24
  • @freedomn-m so you mean to put my update data base in my httppost? so the i need to refer in the action result the values? – Lisa Mar 23 '20 at 11:15
  • 1
    For the question, it doesn't matter as your question is about creating an UPDATE statement. For your application you want to update with the values that the user enters, so your GET action only needs the `int EmailId` to get the existing data from the DB then in the POST it does the update with what the user has entered in the form, using the values from your `ModelTemplateEmail` – freedomn-m Mar 23 '20 at 11:36

1 Answers1

2

There are a couple of problems with your code

  1. The syntax for UPDATE is incorrect. It should be UPDATE SET columnName = value...
  2. Use parameterised queries, because at the moment your code is vulnerable to SQL injection
  3. Move myCommand.ExecuteNonQuery(); inside the try block to catch any exceptions

Please see my update to your code:

var sqlstring = @"UPDATE Email SET Email = @email, Description = @description, UserName = @username, Title = @title WHERE ID = @id");

var myConnection = getconection();
SqlCommand myCommand = new SqlCommand(sqlstring, myConnection);

// add parameters
myCommand.Parameters.AddWithValue("@email", email);
myCommand.Parameters.AddWithValue("@description", description);
myCommand.Parameters.AddWithValue("@username", userName);
myCommand.Parameters.AddWithValue("@title", title);
myCommand.Parameters.AddWithValue("@id", emailId);

try
{
    // execute the command in the try block to catch any exceptions
    myCommand.ExecuteNonQuery();
    myConnection.Close();
}
catch (Exception e)
{
    Console.WriteLine(e.ToString());
}

As mentioned in the comments, you should really perform the update in the HttpPost method and validate the values before calling UpdateDataBase().

haldo
  • 14,512
  • 5
  • 46
  • 52
  • thanks! i have an error that says that SqlParameterCollectionAdd(string object)' is obsolete its ok to put instead AddWithValue("EmailId",EmailId); – Lisa Mar 23 '20 at 10:35
  • 1
    Please see my update - use `AddWithValue` instead @Maria – haldo Mar 23 '20 at 10:38
  • Sounds like you're passing a null value for one of the parameters. Is the value for `email` null? @Maria – haldo Mar 23 '20 at 10:52
  • Now its working.. but not when i save it. I will try to fix it. Thanks a lot for your help and helpful advices, someties is hard to understand when you are all alone. – Lisa Mar 23 '20 at 10:56