CODEWITHSUNDEEP
c#sqlsql-server
mutiemule
mutiemule

Reputation: 2539

Parameterized dynamic sql query

I have a list of keywords that i store in a list.

To fetch records from a table, am using the following query:

sqlBuilder.Append("SELECT name, memberid FROM members WHERE");
StringBuilder sqlBuilder = new StringBuilder();
foreach (string item in keywords)
            {
            sqlBuilder.AppendFormat(" LOWER(Name) LIKE '%{0}%' AND", item); 
            }
string sql = sqlBuilder.ToString();

As you might have noticed, my query is vulnerable to sql injection, thus i want to use parameters using SqlCommand(). I have tried the following but still doesn't work:

foreach (string item in keywords)
            {    
                sqlBuilder.AppendFormat(" LOWER(Name) LIKE '%' + @searchitem + '%' AND", item);
                SqlCommand cmd = new SqlCommand(sqlBuilder.ToString());
                cmd.Parameters.AddWithValue("@searchitem",item);
             }

Where could i be making the mistake, or rather, how should i got about it?

Upvotes: 7

Views: 18706

Answers (2)

Heinzi
Heinzi

Reputation: 172220

You are doing a few things wrong here:

  • You give all your parameters the same name @searchitem. That won't work. The parameters need unique names.
  • You create a new SqlCommand for each item. That won't work. Create the SqlCommand once at the beginning of the loop and then set CommandText once you are done creating the SQL.
  • Your SQL ends with AND, which is not valid syntax.

Improvement suggestions (not wrong per se, but not best practice either):

  • As Frederik suggested, the usual way is to put the % tokens in the parameter, rather than doing string concatenation inside the SQL.
  • Unless you explicitly use a case-sensitive collation for your database, comparisons should be case-insensitive. Thus, you might not need the LOWER.

Code example:

SqlCommand cmd = new SqlCommand();
StringBuilder sqlBuilder = new StringBuilder();
sqlBuilder.Append("SELECT name, memberid FROM members ");

var i = 1;
foreach (string item in keywords)
{
    sqlBuilder.Append(i == 1 ? " WHERE " : " AND ");
    var paramName = "@searchitem" + i.ToString();
    sqlBuilder.AppendFormat(" Name LIKE {0} ", paramName); 
    cmd.Parameters.AddWithValue(paramName, "%" + item + "%");

    i++;
}
cmd.CommandText = sqlBuilder.ToString();

Upvotes: 17

Frederik Gheysels
Frederik Gheysels

Reputation: 56934

Do not put the wildcard characters in your querystring, but add them to your parameter-value:

sql = "SELECT name FROM members WHERE Name LIKE @p_name";
...
cmd.Parameters.AddWithValue("@p_name", "%" + item + "%");

When you add the wildcard characters inside your query-string, the parameter will be escaped, but the wildcard chars will not; that will result in a query that is sent to the DB that looks like this:

SELECT name FROM members WHERE Name LIKE %'somename'%

which is obviously not correct.

Next to that, you're creating a SqlCommand in a loop which is not necessary. Also, you're creating parameters with a non-unique name, since you're adding them in a loop, and the parameter always has the same name. You also need to remove the last AND keyword, when you exit the loop.

Upvotes: 4

Related Questions