Incorrect syntax near ']'. Unclosed quotation mark after the character string '')'

Question

I've been stuck on this issue for a few hours and I can't seem to get over what is causing this issue to populate. When running my application in debug mode, it will run the foreach two times successfully returning "Query Executed!".

However, when it goes for the third time, I get this error:

Incorrect syntax near ']'.Unclosed quotation mark after the character string '')'.

My method that will perform the insert to the SQL Server table Logs:

static String connectionString = "Data Source=.\\SQLExpress;Database=ElasticSearchService;Trusted_Connection=True;";

        public static async Task<int> InsertLogData()
        {
            SqlConnection connection = null;
            SqlCommand command = null;
            int numrows = 0;

            try
            {
                var response = await _elasticClient.SearchAsync<EsSource>(s => s
                   .Size(3000)
                   .Source(src => src.Includes(i => i
                                     .Fields(f => f.timestamp,
                                             fields => fields.messageTemplate,
                                             fields => fields.message)))
                   .Index("customer-simulation-es-app-logs*")
                   .Query(q => +q
                       .DateRange(dr => dr
                           .Field("@timestamp")
                               .GreaterThanOrEquals("2021-06-07T17:13:54.414-05:00")
                               .LessThanOrEquals(DateTime.Now))));

                // var json = _elasticClient.RequestResponseSerializer.SerializeToString(response);

                connection = new SqlConnection(connectionString);

                Console.WriteLine("\nOpening connection...");

                connection.Open();

                Console.WriteLine("\nConnection successful!");

                foreach (var item in response.Hits)
                {
                    var dateCreated = item.Source.timestamp;
                    var messageTemplate = item.Source.messageTemplate;
                    var message = item.Source.message;


                    command = new SqlCommand("insert into Logs (DateCreated, MessageTemplate, Message) values ('" + dateCreated + "', '" + messageTemplate + "', '" + message + "')", connection); 

                    numrows = command.ExecuteNonQuery();

                    Console.WriteLine("\nQuery Executed!");
                }
                connection.Close();
                Console.WriteLine("\nConnection Closed....");
            }
            catch (Exception ex)
            {
                Debug.WriteLine(ex.Message);
            }
            finally
            {
                command.Dispose();
                connection.Dispose();
            }
            return numrows;
        }

Is there something that is causing this that I am not seeing? Is my command = new SqlCommand() incorrect that is causing it to come with that error of:

Incorrect syntax near ']'.Unclosed quotation mark after the character string '')'.

[Please do not upload images of code/errors when asking a question.](//meta.stackoverflow.com/q/285551) — Thom A, Jul 11 '21 at 18:35
Also please, *please, **please***, learn to **parametrise**. It's 2021;SQL Injection should have died out like the dinosaurs by now. — Thom A, Jul 11 '21 at 18:36
Most likely due to the content of message string and how you do insert. The string content may have quotations closing off your insert statement. If you used parameters this wouldn't be an issue and safe from injection attack. — penleychan, Jul 11 '21 at 18:38
@Larnu Sorry, I placed it in case someone wanted a visual but I will remove it. — NoviceCoder, Jul 11 '21 at 18:38
I was looking into SQL parameters however I was a bit lost when it came to the values portion of it. Is there some type of configuration I need to do in SSMS in order to reference each column when I do the value() portion of the insert? I.e. my insert is `"insert into Logs (DateCreated, MessageTemplate, Message) values()`. Can I just do the name of the column and place an @ infront of it i.e. `values(@DateCreated, @MessageTemplate, @Message)` or is there a unique name it should have? If so how would I go about configuring that? @penleychan — NoviceCoder, Jul 11 '21 at 18:48
Here's a link to read on how to use SQL Parameters: https://www.sqlnethub.com/blog/using-the-csharp-sqlparameter-object-writing-more-secure-code/ — penleychan, Jul 11 '21 at 18:49
So if I understand the article you sent, then I would need to create an sql parameter for each of the columns. So in my case, I would need to create 3 parameters? — NoviceCoder, Jul 11 '21 at 18:54

Caius Jard · Accepted Answer · 2021-07-11T20:02:56.450

For starters:

command = new SqlCommand("insert into Logs (DateCreated, MessageTemplate, Message) values (@dt, @mt, @m)";

command.Parameters.AddWithValue("@dt", dateCreated);
command.Parameters.AddWithValue("@mt", messageTemplate);
command.Parameters.AddWithValue("@m", message);

Then execute it

After you get comfortable with that, take a read of this blog - AddWithValue is convenient but it can cause performance issues in some contexts. I don't care about it so much for something like an insert statement but I'd recommend you read the blog to see why you should move away from making a habit of it in SQL Server, once you're down with how parameterizing works. Also worth noting that not every database has issues with AWV, and to some great extent it is preferable to use it and suffer occasional performance issues than not use parameterizing at all and get hacked

SQL in the way you've got it (and the way I've got it) adding parameters etc is actually fairly tedious. Take a look at dapper, a library that makes it a lot easier to do this sort of stuff and more, such as running a sql query and turning the result setinto a list of c# objects, a bit like parsing Json

Other things to consider:

use of ExecuteNonQueryAsync
create your command outside of the foreach loop and add dummy parameters of the right types using AddWithValue (or take the fine opportunity to branch out into adding carefully constructed parameters of the right sql db type), then open the connection and run the loop, changing the parameter values on every pass of the loop. In summary: setup once, change the values and execute 1000 times, rather than setting up 1000 times
use using to declare your command, that way it gets disposed later

Incorrect syntax near ']'. Unclosed quotation mark after the character string '')'

1 Answers1