CodeSOD: Query Elegance

It's generally hard to do worse than a SQL injection vulnerability. Data access is fundamental to pretty much every application, and every programming environment has some set of rich tools that make it easy to write powerful, flexible queries without leaving yourself open to SQL injection attacks.

And yet, and yet, they're practically a standard feature of bad code. I suppose that's what makes it bad code.

Gidget W inherited a PHP application which, unsurprisingly, is rife with SQL injection vulnerabilities. But, unusually, it doesn't leverage string concatenation to get there. Gidget's predecessor threw a little twist on it.

$fields = "t1.id, t1.name, UNIX_TIMESTAMP(t1.date) as stamp, ";$fields .= "t2.idT1, t2.otherDate, t2.otherId";$join = "table1 as t1 join table2 as t2 on t1.id=t2.idT1";$where = "where t1.lastModified > $val && t2.lastModified = '$val2'";$query = "select $field from $join $where";

This pattern appears all through the code. Because it leverages string interpolation, the same core structure shows up again and again, almost copy/pasted, with one line repeated each time.

$query = "select $field from $join $where";

What goes into $field and $join and $where may change each time, but "select $field from $join $where" is eternal, unchanging, and omnipresent. Every database query is constructed this way.

It's downright elegant, in its badness. It simultaneously shows an understanding of how to break up a pattern into reusable code, but also no understanding of why all of this is a bad idea.

But we shouldn't let that distract us from the little nuances of the specific query that highlight more WTFs.

t1.lastModified > $val && t2.lastModified = '$val2'

lastModified in both of these tables is a date, as one would expect. Which raises the question: why does one of these conditions get quotes and why does the other one not? It implies that $val probably has the quotes baked in?

Gidget also asks: Why is the WHERE keyword part of the $where variable instead of inline in the query, but that isn't the case for SELECT or FROM?"

That, at least, I can answer. Not every query has a filter condition. Since you can't have WHERE followed by nothing, just make the $where variable contain that.

See? Elegant in its badness.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!