System.Data.SqlClient.SqlException: 'Incorrect syntax near ') in C#

Question

I can't find the error in the code but it showing me.

System.Data.SqlClient.SqlException: 'Incorrect syntax near ')

My code is here:

private void ShowChart()
{
    string UserID = "";

    for (int Counter = 0; Counter < UID.Count - 1; Counter++)
    {
        UserID += UID[Counter].ToString() + ",";
    }

    UserID = UserID.Substring(0, UserID.Length);

    string[] ListFamily = { };
    int[] ListTime = { };

    var Query = Database.Database.SqlQuery<Vw_ShowChartInfo>("Select * From Vw_ShowChartInfo Where UserID In (" + UserID + ")").ToList();

    for (int I = 0; I < Query.Count; I++)
    {
        Family.Add(Query[I].FullName.ToString());
        Time.Add(Convert.ToInt32(Query[I].TotalTime));
        ListFamily = Family.ToArray();
        ListTime = Time.ToArray();
    }

    this.Chart.Series.Clear();
    this.Chart.Palette = System.Windows.Forms.DataVisualization.Charting.ChartColorPalette.Pastel;
    this.Chart.Titles.Add("نمودار کارکرد پرسنل");

    for (int II = 0; II < ListFamily.Length; II++)
    {
        Series series = this.Chart.Series.Add(ListFamily[II] + "-" + Query[II].TotalTime);
        series.Points.Add(ListTime[II]);
    }
}

Possible duplicate of [What are good ways to prevent SQL injection?](https://stackoverflow.com/questions/14376473/what-are-good-ways-to-prevent-sql-injection), or possibly https://stackoverflow.com/questions/16085812/how-to-use-parameters-in-entity-framework-in-a-in-clause — mjwills, Jan 03 '19 at 22:32
What is the **exact** value of `UserID` (please don't guess)? — mjwills, Jan 03 '19 at 22:32
Your code is not removing the last comma added in the initial loop. You should write _UserID = UserID.Substring(0, UserID.Length-1);_ Also the loop is missing to add the last element from the UID collection, not sure if this is an error or if you really don't want the last item — Steve, Jan 03 '19 at 22:35
Also you should really read about Sql Injection. We don't know what is UID and from which source you initialize it but is a really good practice to use parameters when writing this kind of database code. The parameterization of the IN clause is problematic but still you should use parameters. [Read here about IN clause using parameters](https://stackoverflow.com/questions/337704/parameterize-an-sql-in-clause) — Steve, Jan 03 '19 at 22:40
I'd also suggest changing your second for loop to `Family.AddRange(Query.Select(z => z.FullName.ToString())); Time.Add(Query.Select(z => Convert.ToInt32(z.TotalTime))); ListFamily = Family.ToArray(); ListTime = Time.ToArray();` since it is much more concise and likely faster. — mjwills, Jan 03 '19 at 22:46
For other visitors, please **do not** follow the accepted answer here. It is likely open to SQL Injection. Please read the links that @Steve and I provided. — mjwills, Jan 03 '19 at 22:50

score 0 · Accepted Answer · answered Jan 03 '19 at 22:36

0

I think your problem might be this line:

UserID = UserID.Substring(0, UserID.Length);

The string you are generating contains a comma at the end. It looks like you are attempting to trim the comma with that line but you are simply selecting the same string again. Use this instead:

UserID = UserID.Substring(0, UserID.Length - 1);

Also, the assumption is your user ids are numeric. Otherwise, you would need to surround them with single quotes.

That being said, it's a terrible idea to concatenate strings this way as it leaves you open to SQL injection attacks. Consider a different approach.

answered Jan 03 '19 at 22:36

JuanR

7,405
1
19
30

6

It's a terrible idea to tell beginners to put SQL injection vulnerabilities in their code. – Dour High Arch Jan 03 '19 at 22:44
@DourHighArch: I think no one would disagree, but a question was asked and an answer was given. I think the votes are meant to rate the quality of the answer with regards to the question, not how well we can refactor code. Either way, I still mentioned it at the bottom of my answer. Happy New Year! – JuanR Jan 04 '19 at 14:07

System.Data.SqlClient.SqlException: 'Incorrect syntax near ') in C#

1 Answers1