{"id":1321,"date":"2009-09-11T19:31:17","date_gmt":"2009-09-11T17:31:17","guid":{"rendered":"http:\/\/planetozh.com\/blog\/?p=1321"},"modified":"2009-09-15T21:56:51","modified_gmt":"2009-09-15T19:56:51","slug":"top-10-most-common-coding-mistakes-in-wordpress-plugins","status":"publish","type":"post","link":"https:\/\/planetozh.com\/blog\/2009\/09\/top-10-most-common-coding-mistakes-in-wordpress-plugins\/","title":{"rendered":"Top 10 Most Common Coding Mistakes in WordPress Plugins"},"content":{"rendered":"

Short intro for readers who don't follow me<\/a> or this blog's feed<\/a>: 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<\/a>.<\/em><\/p>\n

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.<\/p>\n

\"wtf-code\"
\n(Image stolen from Thom Holwerda<\/a> without permission)<\/p>\n

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 :)<\/p>\n

<\/p>\n

10 most frequent bad code bits<\/h2>\n

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.<\/p>\n

1. It's not a plugin, it's a mess<\/h3>\n

I've been truly shocked<\/strong> 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<\/em> like a mess it is<\/em> obviously one.<\/p>\n

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.<\/p>\n

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<\/a>. Hopefully you'll change your mind and adhere to WordPress Coding Standards<\/a>.<\/p>\n

2. Way too generic function names<\/h3>\n

Again basics here, but about 40% of the plugins<\/em> 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.<\/p>\n

Function names must be two things: descriptive and unique. I've found plugins with function named as simply as pretty_title()<\/tt>, pages()<\/tt> or update_options()<\/tt>. 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.<\/p>\n

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

3. What? 87 new rows in the option table?<\/h3>\n

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

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.<\/p>\n

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<\/strong> set the rather unknown autoload<\/tt> parameter to 'no'.<\/p>\n

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'\"<\/tt>. This loads in memory *all* the options which have autoload set to (default value) 'yes'.<\/p>\n

To specify that an item from the DB option must not be loaded on start, simply use:<\/p>\n

\n\n\t\t
\n\n\t\t
\n\t\t\t\t\t\t< View plain text<\/span> ><\/a>\n\t\t\t\t\t<\/div>\n\n\t\t
php<\/div>\n\n\t\t\n\t\t
\n\n\t<\/div>\n\t\n\t
\n\t\t
  1. add_option(<\/span>'option_name'<\/span>,<\/span> 'option_value'<\/span>,<\/span> ''<\/span>,<\/span> 'no'<\/span>)<\/span>;<\/span> \/\/ 'no' = not autoload<\/span><\/div><\/li>\n<\/ol>\t<\/div>\n\n<\/div>\n\n

    4. You create new tables for what?<\/h3>\n

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

    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<\/tt> and the wp_*meta<\/tt> tables.<\/p>\n

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

    5. No uninstall function<\/h3>\n

    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<\/a>.<\/p>\n

    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.<\/p>\n

    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.<\/p>\n

    6. Custom javascript or CSS added on each and every admin pages<\/h3>\n

    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<\/tt>. Inserting your javascript to all other pages *will* eventually break another plugin interface that also uses javascript, not to mention that it's useless.<\/p>\n

    Inserting your script or CSS to your page only<\/em> is very easy<\/a>.<\/p>\n

    7. Plugin forms with no security, or nonces misunderstood<\/h3>\n

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

    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()<\/tt> or that current_user_can()<\/tt>. Basically, this means that anyone (users with no authority) can POST data to your blog and play with the plugin.<\/p>\n

    But even checking for authority can be insufficient. Imagine a plugin that would, say, delete posts<\/a>. 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.<\/p>\n

    This is the issue nonces<\/strong> 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.<\/p>\n

    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.<\/p>\n

    Read more about nonces<\/a> and how to implement them. Or even better: read this article on how to deal with options in WP 2.8<\/a>, 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.<\/p>\n

    8. Actions triggered from unchecked GET data<\/h3>\n

    I've seen similar constructs to the following block more than once, and that's a bit scary:<\/p>\n

    \n\n\t\t
    \n\n\t\t
    \n\t\t\t\t\t\t< View plain text<\/span> ><\/a>\n\t\t\t\t\t<\/div>\n\n\t\t
    php<\/div>\n\n\t\t\n\t\t
    \n\n\t<\/div>\n\t\n\t
    \n\t\t
    1. add_action(<\/span>'init'<\/span>,<\/span> 'myplugin_init'<\/span>)<\/span>;<\/span><\/div><\/li>\n
    2. function<\/span> myplugin_init(<\/span>)<\/span> {<\/span><\/div><\/li>\n
    3.     global<\/span> $wpdb<\/span>;<\/span><\/div><\/li>\n
    4.     call_user_func<\/span>(<\/span>$_GET<\/span>[<\/span>'action'<\/span>]<\/span>)<\/span>;<\/span><\/div><\/li>\n
    5.     $wpdb<\/span>-><\/span>query<\/span>(<\/span>'DELETE from sometable WHERE somefield = '<\/span>.<\/span> $_GET<\/span>[<\/span>'value'<\/span>]<\/span>)<\/span>;<\/span><\/div><\/li>\n
    6. }<\/span><\/div><\/li>\n<\/ol>\t<\/div>\n\n<\/div>\n\n

      This case is similar to the previous one: use an unchecked request (yoursite.com?action=bleh&value=wot<\/tt>) 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.<\/p>\n

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