Most programmers are familiar with a notion of technical debt. Sometimes all it takes to make or break a project is a single bad decision, questionable design solution, or even a plain old bug that doesn't get fixed early on. The hacks and workarounds keep piling up, slowly turning the project into an unmaintainable mess.

In this regard, David was already off to a head start. He has recently been assigned to maintain a meeting tracking system with – to put it lightly - a bit of history. A year before, the marketing department of his company received the first version from a subcontractor and promptly implemented it – only to find out that the data gathered were a little off. According to the reports, every single meeting lasted exactly 24 hours – from midnight to midnight.

The heads of Marketing found the sudden love for meetings hard to believe. David’s team took a quick look at the code and found out that the original developers used a date type for the columns representing the meeting’s beginning and end in the database – never bothering to store the actual time. David was a little stumped – how did this manage to pass even the most basic tests? – but sent it back for a fix and called it a day.

Fast forward a year later. The subcontracting company delivered a fixed version and promptly went belly-up, so it was up to David’s team to maintain the codebase. But while the original issue was fixed now, looking at the code made David wish it had never been the case…

Instead of converting the startTime and endTime columns to a proper type, the developer had a better idea – he added a startHour and endHour to the table. And just in case David’s office suddenly moves a few millions of timezones away, or makes contact with an alien civilization, they made the column a varchar2(64).

This odd design decision, however, had very interesting effects on other parts of the code. For example, this (single) line determines whether the meeting is happening now: 

elseif(($this->meetings[0]['startDate']meetings[0]['endDate']>date('Y-m-d') || ($this->meetings[0]['endDate']==date('Y-m-d') && $this->meetings[0]['endTime']>date('H:i')))) || (($this->meetings[0]['startDate']==date('Y-m-d') && ($this->meetings[0]['startTime']meetings[0]['endDate']>date('Y-m-d') || ($this->meetings[0]['endDate']==date('Y-m-d') && $this->meetings[0]['endTime']>date('H:i'))))))) { //…

And this (again, single-line) query, aside from introducing a nice SQL injection vector, allegedly checks whether there’s an overlapping meeting: 

'SELECT * FROM meetings WHERE id!='.$id.' AND (((startDate < \''.$start_date.'\' OR (startDate =\''.$start_date.'\' AND startHour <= \''.$start_hour.'\'))) AND (endDate > \''.$end_date.'\' OR (endDate =\''.$end_date.'\' AND endHour >= \''.$end_hour.'\'))) OR (((startDate > \''.$start_date.'\' OR (startDate =\''.$start_date.'\' AND startHour >= \''.$start_hour.'\'))) AND (endDate < \''.$end_date.'\' OR (endDate =\''.$end_date.'\' AND endHour <= \''.$end_hour.'\'))) OR (((startDate < \''.$start_date.'\' OR (startDate =\''.$start_date.'\' AND startHour < \''.$start_hour.'\')) AND ((endDate > \''.$start_date.'\' OR (endDate =\''.$start_date.'\' AND endHour > \''.$start_hour.'\')) AND (endDate < \''.$end_date.'\' OR (endDate=\''.$end_date.'\' AND endHour<\''.$end_hour.'\'))))) OR (((startDate > \''.$start_date.'\' OR (startDate = \''.$start_date.'\' AND startHour > \''.$start_hour.'\')) AND ((startDate < \''.$end_date.'\' OR (startDate = \''.$end_date.'\' AND startHour < \''.$endHour.'\'))) AND ((endDate > \''.$end_date.'\' OR (endDate = \''.$end_date.'\' AND endHour > \''.$end_hour.'\')))))';

"All I could do," says David, "was to replace the concatenation with placeholders. I still don’t know what exactly it does, other than making me tremble."

 

Photo credit: stockerre / Foter / CC BY

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!