Top 10 Most Common Coding Mistakes in WordPress Plugins « planetOzh

Short intro for readers who don't follow me or this blog's feed: I've been a judge in the annual WordPress Plugin Competition, and as such I have reviewed a number of plugins. Read more about this.

As promised, I'm going to share a list of the most common mistakes, errors, misunderstandings, bad habits or wrong design decisions I've encountered while reviewing all these 43 plugins. Some are highly critical stuff (I've contacted 3 plugins authors after finding serious security holes in their plugin), some are more potential annoyances than real bugs, or are just causing a waste of server resources that could be avoided, but all have something in common: they're trivial to fix.

(Image stolen from Thom Holwerda without permission)

I've classified them in two parts: 10 bad code signs, plus a bonus with design decisions that suck. If you consider yourself a semi experienced coder or better, be sure to skip this article, you're not going to learn a thing :)

10 most frequent bad code bits

What I call bad code can be: code that might work but is ugly, code that works but will fail one day, and obviously code that doesn't work at all.

1. It's not a plugin, it's a mess

I've been truly shocked by the number of coders who deliver stuff with no comment, no or random indentation in code (honestly!! just *no* indentation!! can you believe this??). As a judge I've poured my wrath on their final grade, just like as a user I would simply never install a plugin that looks like a mess, because if it looks like a mess it is obviously one.

A plugin with no comment where needed and no indentation to make code readable tells one thing: the plugin won't ever be updated or maintained, because in a couple of months the author will be simply lost and won't remember what, how and why they've done this or that.

I won't elaborate too much on this because it's just plain common sense, but if you're one of those messy guys or gals who don't really see the point here, please read the following article: Make clean and readable sources: why and how. Hopefully you'll change your mind and adhere to WordPress Coding Standards.

2. Way too generic function names

Again basics here, but about 40% of the plugins I've reviewed use a waaaaay too generic function naming scheme. The point here is to make sure your plugin will never trigger a "Fatal error: Cannot redeclare your poorly named function" error.

Function names must be two things: descriptive and unique. I've found plugins with function named as simply as pretty_title(), pages() or update_options(). Ironically enough, a coder submitted several plugins that won't be able to run on the same blog because they all use the same function declarations.

Better function names would have been for instance joeplugin_post_pretty_title(), joeplugin_list_pages() or joeplugin_update_options(). An alternative to keep function names short is to wrap them into a class (that also must have a unique and descriptive name).

3. What? 87 new rows in the option table?

If you've been reading me for a while, you know it's my favorite pet peeve: don't save each setting into a separate DB entry. Store them in an array, and save it in one DB row. One of the plugins in the competition for instance creates 87 entries in the option table.

What I don't like about adding a bunch of entries in the option table is that it makes a unnecessarilly cluttered database once you deactivate the plugin, and that the plugin performs a bunch of SQL write queries instead of just one.

There's an exception to this rule: if your plugin has a gazillion options and only a few of them are going to be used on every instance while the rest will only be used by an admin page, then it makes sense to store them into 2 or more entries and set the rather unknown autoload parameter to 'no'.

WordPress works like this: when initialized, it requires all its needed files, then does a "SELECT option_name, option_value FROM wp_options WHERE autoload = 'yes'". This loads in memory *all* the options which have autoload set to (default value) 'yes'.

To specify that an item from the DB option must not be loaded on start, simply use:

4. You create new tables for what?

WordPress comes with a several tables for all the various tasks you might think of: users, meta informations about users, posts and their meta informations, etc..

Now, of course, it's possible that your plugin needs to create one or more extra tables, but before doing so, think and be sure the existing tables just won't fit. Typically, consider using wp_options and the wp_*meta tables.

If you *really* have to create tables, give them a name that makes sense. Always begin with $wpdb->prefix, use your plugin shortname, and append a word that will describe the table usage. Something like $wpdb->prefix.'myplugintitle_custom_logs'.

5. No uninstall function

WordPress since version 2.7 has implemented functions to uninstall plugins. There are two ways of doing so: using an uninstall hook, which some might find a bit complicated (well, it's not, really), and using an uninstall file, which is a no brainer dead easy solution, as Jacob explains.

Now, having uninstall functions is not really something I consider a prerequisite or an absolute must have feature. It's just a nice touch to know that the plugin will leave no trails in the database once the user will want to get rid of it.

This said, I find it totally unacceptable when a plugin creates a collection of option entries (see point #3) or extra tables (see point #4) and provide no automatic way of cleaning things up upon removal.

6. Custom javascript or CSS added on each and every admin pages

This one is an all time classic. When you create option pages for your plugin and will need custom javascript for them, please, please, pretty please, don't just hook your jQuery bits to admin_head. Inserting your javascript to all other pages *will* eventually break another plugin interface that also uses javascript, not to mention that it's useless.

Inserting your script or CSS to your page only is very easy.

7. Plugin forms with no security, or nonces misunderstood

When an option form is submitted, a plugin should verify if the submitter has authority and intention. There's a very well explained article from super star Mark Jaquith on this subject.

The problem I've seen too many times in the plugins I've reviewed is that form data are handled and processed without even checking that is_admin() or that current_user_can(). Basically, this means that anyone (users with no authority) can POST data to your blog and play with the plugin.

But even checking for authority can be insufficient. Imagine a plugin that would, say, delete posts. It's trivial to make a webpage that sends POST data to someone else's blog plugin option page, and share with them a bitlyfied short URL of that webpage via Twitter. One click on it and you would be redirected to your own blog admin area where you do have authority, and delete your posts. You have authority, but no intention.

This is the issue nonces address. Nonce functions generate unique and temporary timestamps that are impossible to guess, and check that data submitted come from a particular page from *your* admin area, not just from anywhere on the interwebs.

A very common mistake I've seen in a lot of plugin is simply adding a nonce field to the form. This won't work, it is not enough. You need nonce fields on the form, and nonce checks where the form is processed, otherwise it's just like telling "everybody needs an ID card to get into my bar" but never check them.

Read more about nonces and how to implement them. Or even better: read this article on how to deal with options in WP 2.8, and learn how to both correctly store all your options in a single DB entry and implement security in your forms, using just one function call.

8. Actions triggered from unchecked GET data

I've seen similar constructs to the following block more than once, and that's a bit scary:

This case is similar to the previous one: use an unchecked request (yoursite.com?action=bleh&value=wot) but this time to execute code or even perform SQL queries. Omit to implement security in this case is highly critical since you don't even have to trick a blog owner into clicking on a URL that sends them to their own admin: just anyone can do it.

Once again, nonce functions are what you need. If you want to use $_GET['action'] as the switch for your action, please do this:

• Instead of sending user to site.com/?action=something, use the following code:
  < View plain text >

  php

  
  

  1. $url = "site.com/?action=something";
  2. $action = "myplugin-update";
  3. $link = wp_nonce_url( $url, $action );
  4. echo "<a href='$link'>click here</a>"; // whatever you're doing to echo the nonced link
• In the function triggered by the $_GET parameter, do:
  < View plain text >

  php

  
  

  1. if ( isset( $_GET['action'] ) && $_GET['action'] == 'something' ) {
  2.     check_admin_referer( 'myplugin-update' ); // die if invalid or missing nonce
  3.    
  4.     // rest of the code ...
  5. }

See? Nonces are simple. One function call in the form or the link, one function call in the processing part.

There are also other functions that might be required or just sensible to use here: is_admin() to make sure we're in the admin area, current_user_can() to make sureprivileges are sufficient.

9. Trust user input and pass it to SQL

This one is the most critical security hole you can craft, especially when used in conjunction with point #8, as I've seen in two plugins. Whenever you're performing SQL queries containing user input data, validate them. The risk here is SQL injection.

If you're passing a parameter that's supposed to be an integer, use intval() before storing it in the DB. If you're allowing HTML, esc_attr() it. If you're expecting a string, preg_replace('/[^a-zA-Z]/', '') it. And so on.

Once the data for your SQL query is validated, send the query string through $wpdb->prepare() before running it. Method prepare() is similar in use to sprintf(), and handles for you all the escaping, quoting and integer casting you'll need. It's as simple as:

If your plugin is going to play with MySQL, make sure you grok the whole $wpdb class.

10. Localization done wrong

This last one has to be the most frequent code error I've encountered: thinking __('some string') will be enough to make a plugin translatable.

The correct syntax to use for a plugin to support localization is __('some string', 'myplugin') where 'myplugin' is a unique identifier to the text domain, which has to be initialized with load_plugin_textdomain()

Complete example with a subdirectory 'translations/' in your plugin directory:

For a comprehensive tutorial on plugin translation, I suggest your read this one. There's also a page on the Codex about I18n for WordPress Developers that you should read.

Design decisions that suck

During my reviewing of all the plugins, I've found numerous bits of code that are not exactly bad (and can even be smart, actually) but are simply a bad idea because one day or another it will fail on someone's setup, or just because there are easier ways to go.

Unadvertised or unchecked PHP5+ functions

As of writing, WordPress has pretty loose requirements, namely PHP 4.3. I'm not sure how many people are still running PHP 4.x, but there are. It's tempting to use PHP5 functions such as json_encode(), and that's OK, but in such a case, you need to either warn the user on the plugin's page ("This plugin requires PHP5"), or make your plugin check the environment and tell the user it's not going to run as expected.

Extension required. Is it available?

A lot of plugins I've looked at require PHP's CURL extension, I've also seen one needing mbstring functions, and yet they don't check if it's present. This is similar to the previous point: you have to tell the user, or make your code check that everything is OK prior to do anything.

Wheel reinvented

Speaking of cURL, why are you still using it? It's awesome and everything, but it totally might be unavailable. What's the point in writing a 10 line code block to fetch remote content while a single call to wp_remote_get() is enough, and will work even if there's no cURL?

WordPress has a number of internal functions to make your code faster to write and more compatible with every setup. I've written for instance about making HTTP requests the easy way, managing options without mucking with $_POST, but there's also built-in Ajax functions and hooks, and more.

Whenever you're going to write a code block that seems to be quite generic and common, check first if there's a WP function that can do it for you.

Compatibility maintained with deprecated versions of WordPress

I've seen code comments mentioning stuff like "// we're doing this for people using WP 2.5". This one is more a personal choice, but I think maintaining compatibility with older versions is a terrible idea.

Terrible for you: fixing bugs and implementing new features is quite a task already, don't add to the burden with more deprecated code.

Terrible for the users: it's nice that your plugin is going to run fine on their obsolete, insecure and already hacked blog, but it really does not motivate them to upgrade, which is vital.

I know I've dropped backward compat with my plugins a long time ago, and always code for the latest release available. It makes life so much easier :)

Hardcoded paths

Since several point releases, things might not be where you think they are: wp-config.php gone up a directory, or the whole wp-content directory moved somewhere else. I've also seen users rename a plugin's directory.

In any of these edge cases, your plugin will break if you're relying on hardcoded path. Use WP_PLUGIN_DIR, WP_PLUGIN_URL, plugin_basename( __FILE__ )

All files included, even if not needed

Make your life easier: split your plugin in several smaller chunks, put files in various directories instead of dropping everything in the same folder (includes/, css/, translations/, etc…) and include only what you need. Maybe !is_admin()? Then don't require_once() all the stuffs that create the option forms.

User left alone in the dark if something goes wrong

Once you're done with your neat functions that send stuff to Twitter, ask yourself: what's going to happen if Twitter is down?

Nothing is more frustrating than users coming to your site and asking for support because the plugin did not work and they just cannot tell what went wrong. Anywhere possible, try to add diagnosis functions and messages, check results of operations and warn the user if something looks like things failed.

No download link on the plugin's page

Yeah, this last one sounds like a joke, but it's not. I've seen it last year, I've seen it this year again: people make a plugin, write a nice page about it, and don't tell how to download it. Please make sure the download link is unmissable :)

And that's it

That was a way too long article for such basic tips :)