7 More Mistakes Commonly Made by PHP Developers

Back at the end of June, TopTal, the freelance marketplace, published a post about 10 Most Common Mistakes PHP Programmers Make. The list wasn’t exhaustive, but it was well written and pointed out some very interesting pitfalls one should be wary of – even if I wouldn’t personally list the mistakes as very common.

I encourage you to give it a thorough read – it has some truly valuable information you should be aware of – especially the first eight points. A couple days back, Anna Filina expanded on the list with seven new entries. While less specific and common, her points still carry weight and should be considered when developing.

7 More Mistakes PHP Developers Often Make

I was asked by someone from TopTal to take a look at their list and potentially contribute, and some of our followers on social networks expressed an interest in seeing the list continued, too, so I’d like to take this opportunity to add some of my own entries to this list that I repeatedly need to warn my team members or followers about.

1. Using the mysql extension

This news is quite old, but the number of developers oblivious to the fact is worrying. When using SQL databases, specifically MySQL, far too many developers still opt for the mysql extension. The mysql extension is officially deprecated. It’s insecure, unreliable, doesn’t support SSL and is missing some modern MySQL features. It also generates deprecation notices which don’t break your app, they just appear at the top of your app. Hilariously, what this means is that it’s also possible to simply Google for all the various sites that use this insecure setup by simply looking for this. The world of hurt those apps are exposed to due to this mess is staggering.

Instead of using mysql, opt for one of the alternatives: MySQLi, or PDO. For example, using MySQLi instead is almost as simple as adding the letter “i” to the end of the API calls:

$c = mysql_connect("host", "user", "pass");
mysql_select_db("database");
$result = mysql_query("SELECT * FROM posts LIMIT 1");
$row = mysql_fetch_assoc($result);

$mysqli = new mysqli("host", "user", "pass", "database");
$result = $mysqli->query("SELECT * FROM posts LIMIT 1");
$row = $result->fetch_assoc();

That’s all it took to make the setup immeasurably more secure.

You should opt for PDO, though. More on that in point 2.

2. Not using PDO

Don’t get me wrong, mysqli is (quite literally) generations ahead of the ancient mysql extension. It’s kept up to date, secure, reliable and fast. However, it’s mysql specific. Using PDO instead would let you use some wonderfully practical object oriented syntax, and would prepare you for tango with other SQL databases like PostgreSQL, MS SQL, and more. What’s more, PDO will let you use named parameters, a feature so useful, few people can imagine going to anything else after having taken proper advantage of it. Last but not least, there’s this: you can inject fetched data directly into a new object, which is a delightful timesaver in large projects.

3. Not rewriting URLs

Another commonly ignored and easy to fix issue. URLs like myapp.com/index.php?p=34&g=24 are just not acceptable in this day and age. Due to it being incredibly difficult to write a good URL rewriting guide that would cover every server and framework out there, almost every framework has a guide on how to set up clean URLs (Laravel, Phalcon, Symfony, Zend) and any that don’t just aren’t worth using – they obviously don’t care about modern practices.

4. Suppressing errors

I wrote about this in a previous article, but it’s worth mentioning again. Any time you find yourself using the @ operator, reconsider and approach the problem from a different angle more carefully. Take my word for it when I say that 20 lines of boilerplate cURL code around an app’s functionality is better than a single line with the @ operator in front of it.

I’ve found through personal experimentation that a good approach is the one I advocate in the original post – turn all your notices into fatal errors. Making sure nothing gets logged into the error logs because there’s literally nothing to log is better than pretending poop isn’t hitting the fan by holding @ in front of your eyes.

We recently covered some Heroku add-ons for production ready PHP apps, and one of those was the excellent Papertrail – an add-on which lets you push all your app’s errors to their backend for easier searching, grouping, and elimination later on; so even if some errors do happen, it’s better to let them be logged and get rid of them by fixing your code, than silencing them and playing dumb in front of your users.

5. Assigning in Conditions

Even experienced developers sometimes have a slip of the finger and write if ($condition = 'value') { instead of if ($condition == 'value') {. Our hands will slip, our keyboards won’t register the keypress, we’ll end up pasting from another part of the code where the assignment actually happened – it happens, and we usually find out only when we run the app.

There are several ways to completely avoid this:

Use a decent IDE. Any good IDE (like PhpStorm, for example) will warn you of “assignment in condition” issues when it detects them.
Use “Yoda Conditions”. You’ll see these in many popular projects, even large frameworks. By inverting the comparison (as in, if ('value' = $condition) {), weaker IDEs will notice the problem, too. Some consider the Yoda syntax annoying and pointless, a lifeline where there should be none (“be more carefuly with your code, dammit”), but to each his own – if it helps someone, I’m all for it. If we were all elitists, WordPress and Zend Framework wouldn’t exist.
By simply keeping it in mind, you’ll develop an eye reflex to check for it every time you write it. All it takes is practice, but it happens even to the best devs and that’s where 1. and 2. come in handy.

6. Being Too Transparent

Saying this might stir up some controversy, but here goes anyway. Unless you have 100% confidence in the framework’s developers, or don’t operate high-profit, high-traffic business critical applications, you should always strive to obscure your back-end ways – not broadcasting which framework your app is based on can actually help in preventing attacks, should a security vulnerability of that framework be discovered. For example:

If you use Symfony2 translator and have a route with a {_locale} parameter upgrade NOW ! http://t.co/jihXHB8MzT

— Jérémy DERUSSÉ (@jderusse) July 15, 2014

In this tweet, knowledge of a serious code injection issue is being broadcast into public domain. This is great if you’re at work and can upgrade immediately without devops issues and getting the team huddled up first, but for most people and companies using Symfony, this is not the case. Even though Symfony can be upgraded via Composer (as Ryan mentioned in the comments below), it usually takes a while to get approval in large teams with multi-tier environments. All websites using this translator approach that are declared Symfony users were (are?) therefore exposed to this vulnerability until fixed.

Using Symfony in the example above was just that – an example. Similar situations have arisen with countless other software over the years. Back when I still used Zend Framework commercially, we had this happen too, and suffered an attack due to it. WordPress has had its share of security gaffes and we know how high of a percentage of websites out there they power. These things happen, and sometimes, open source and transparency aren’t the best approach when dealing with applications that carry the majority of a company’s revenue stream.

7. Not Removing Development Configurations

Last but not least, development configuration removal should be mentioned. Quite recently (and it’s an honest coincidence I’m mentioning Symfony here again), Cnet suffered an attack due to not removing their development configuration.

Uhmmm no: http://t.co/rAQis1ycWq #security #symfony

— Marco Pivetta (@Ocramius) July 15, 2014

Cnet, one of the world’s largest tech news sites, is based on Symfony. Symfony, as you might know, features two entry points to your application: app.php and app_dev.php. By pointing your browser to one, you get the production environment. By pointing to the one with the _dev suffix, you obviously get the development version, which features a debugger, sensitive data, and more. Whether this is good or bad is a subject of many discussions (again, thanks to Ryan for pointing this out), but it’s undeniable that it opens some clumsier developers to errors such as those Cnet suffered from. What’s more, any other URLs accessed when on app_dev will get redirected to other app_dev URLs. In other words, it’s not just the index page that launches in development mode, it’s the entire website – in Cnet’s case, that’s a lot of access.

If you follow the discussion on Twitter, it gets emabrrasingly sad really fast – and what’s even sadder is that it could have been avoided in a second’s work:

The devs could have removed app_dev.php from the production servers
The devs could have whitelisted IPs allowed to access app_dev.php, which is how it works by default unless you loosen those restrictions up.

Either of these approaches would have completely prevented all problems. Remember, when pushing to production, make sure your development configuration is either fully inaccessible, or accessible only to a whitelisted set of IPs.

Conclusion

How do you feel about this list? Does it cover common aspects or is it too esoteric? Do you have some more common pitfalls the three posts in total have failed to mention? Let me know in the comments below and we’ll update the post if your advice is sound!