Dataset with optional addition to where statement

I am trying to generate a dataset from a query that has a where statement. If the user passes a value additional things need to be added to the where statement. Is this the best way to go about this?

``
class TableAdapters
{
public class FindScriptByTargetVDN
{
public static DataSet Fill(SqlConnection connection, string parameter)
{
string conditional = string.Empty;
if (parameter.Length > 0)
{
conditional = "and svce.PeripheralNumber= " + parameter + " ";
}
string query = "select svce.PeripheralNumber AS Service, "
+ "ms.EnterpriseName AS [Script Name], "
+ "p.EnterpriseName AS Peripheral, "
+ "sg.PeripheralNumber AS [Skill Mapping], "
+ "s.Version AS [Latest Version], "
+ "s.DateTime AS Created, "
+ "s.Author "
+ "from Script_Cross_Reference scr "
+ "LEFT OUTER JOIN Script s on s.ScriptID = scr.ScriptID "
+ "LEFT OUTER JOIN Master_Script ms on s.MasterScriptID = ms.MasterScriptID "
+ "LEFT OUTER JOIN Service svce on svce.SkillTargetID = scr.ForeignKey "
+ "LEFT OUTER JOIN Peripheral p on svce.PeripheralID = p.PeripheralID "
+ "LEFT OUTER JOIN Service_Member sm on svce.SkillTargetID = sm.ServiceSkillTargetID "
+ "LEFT OUTER JOIN Skill_Group sg on sg.SkillTargetID = sm.SkillGroupSkillTargetID "
+ "where s.Version = ms.CurrentVersion and scr.TargetType = 1 "
+ conditional
+ "order by svce.PeripheralNumber, ms.EnterpriseName, p.EnterpriseName";
SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
DataSet dataSet = new DataSet();
dataSet.Tables.Add(new DataTable());
adapter.Fill(dataSet.Tables[0]);
return dataSet;
}
}
}


This:

+ "where s.Version = ms.CurrentVersion and scr.TargetType = 1 "
+ conditional
+ "order by svce.PeripheralNumber, ms.EnterpriseName, p.EnterpriseName";

Is poison.

conditional came from a string concatenation that involves a parameter string that the method cannot trust:

conditional = "and svce.PeripheralNumber= " + parameter.ToString() + " ";

..Uh, the ToString() call is superfluous here, parameter is already a string! ;)

Looking at the code in your other question, this is where the parameter came from:

string value = this.ValueTextBox.Text;

That is what you shouldn't be doing: concatenating user input into an SQL statement is asking for trouble, whether it's a web or a desktop application doesn't matter, SQL Injection is SQL injection. Not to mention that you just put the calling code in charge of knowing the types for all columns that might be used, whether or not the values should be enclosed in single quotes, etc.

You need to use a parameterized query instead.

StackExchange Code Review Q#55893, answer score: 8