Is your code held together with bits of string?

Posted at 20:00 on 26 September 2007

Meh.

I hate naive code that sends data to a database by concatenating it into a SQL string.

Unfortunately, there is far too much of it knocking around -- no doubt because of the proliferation of rubbishy tutorials that teach beginners that that is the way to do database access.

Take this C# example:

public int InsertEvent(DateTime date, string description)
{
    using (SqlConnection cn = new SqlConnection(connectionString)) {
        cn.Open();
        SqlCommand cmd = new SqlCommand(cn,
            "insert into Events (Date, Description) values "" +
            date.ToString() + "", "" +
            description + "";select @@IDENTITY");
        return (int)cmd.ExecuteScalar();
    }
}

It's not just the SQL injection vulnerability that makes this code stink like a sewer: it has localisation problems as well.

Here in the UK we write dates as day/month/year, so today would be 26/09/2007. However, on the other side of the pond, they write dates as month/day/year, so today would be 09/26/2007. So if the locale of your ASP.NET application is different from the locale of your database login, you will get either the wrong date or a data conversion error.

On your development computer and your production server you will probably have your locales set up so that it all works correctly. However, it causes problems when you have to set up the application on a new box -- for instance, when another developer starts to work on the project. Especially if the other developer is in another country.

Please stop doing this!!!!

Any decent, modern programming language will let you use parametrised queries to keep your SQL and data separate. These also allow you to send dates and times to the database in native, unambiguous datetime format, avoiding any thorny localisation issues, and they all but eliminate SQL injection vulnerabilities.

The only excuse for using string concatenation in this way is that you still have to support PHP 4 which does not give you the option of parametrised SQL queries for MySQL. Even then, if at all possible, you should be upgrading to PHP 5, which does.