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:
- add_option('option_name', 'option_value', '', 'no'); // 'no' = not autoload
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:
- add_action('init', 'myplugin_init');
- function myplugin_init() {
- global $wpdb;
- call_user_func($_GET['action']);
- $wpdb->query('DELETE from sometable WHERE somefield = '. $_GET['value']);
- }
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:
- $url = "site.com/?action=something";
- $action = "myplugin-update";
- $link = wp_nonce_url( $url, $action );
- 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:
- if ( isset( $_GET['action'] ) && $_GET['action'] == 'something' ) {
- check_admin_referer( 'myplugin-update' ); // die if invalid or missing nonce
- // rest of the code ...
- }
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:
- $calvin = "6 years";
- $hobbes = "stuffed";
- // "Prepare" the query
- $sql = $wpdb->prepare( "INSERT INTO $wpdb->joeplugin_table( id, field1, field2 ) VALUES ( %d, %s, %s )", $_POST['id'], $calvin, $hobbes );
- // Run it
- $wpdb->query( $sql );
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:
- add_action('init', 'myplugin_load_translation_file');
- function myplugin_load_translation_file() {
- // relative path to WP_PLUGIN_DIR where the translation files will sit:
- $plugin_path = plugin_basename( dirname( __FILE__ ) .'/translations' );
- load_plugin_textdomain( 'myplugin', '', $plugin_path );
- }
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 :)
Shorter URL
Want to share or tweet this post? Please use this short URL: http://ozh.in/ok
Excellent, excellent post. Only one point of disagreement:
"If you consider yourself a semi experienced coder or better, be sure to skip this article"
It's the plugins developers who think they are semi-decent who really need to read this. It will take them up to excellent, and we could all use more excellent plugins. ;)
A very nice article! Wraps up pretty much every possible mistake while developing (WordPress) plugins, and shows that you're the right one as a judge in this year's WordPress Plugin Competition. :-)
Oliver Schlöbe » Well that's pretty much what I want to avoid: give the impression I think I know everything etc… It's a lot much easier to criticize code after release than do code :)
Heh, true. But it seems that you thought about the process of reviewing code BEFORE reviewing, and that makes a good judge.
Comic appears to be from OSNews. :)
Fantastic article, as I can say that I've personally seen all of these things in plugins before.
Great article even though I have no idea what most of it means. In fact, it's Blabla to me, just like these comments :)
Proof positive that you definitely serve the judge position very well. As for myself, I judged on usability, ease of use, and usefulness of the plugin.
This is a great article for those who don't normally do plugin development like me. There's a ton of good info here!
Most problems comes from bad documentation of WordPress, bad design choices of WordPress and horrible design choices in PHP =). Most of the code WTF actually originates from lack of WP doc and PHP =)
For example the use of array in options table. You can't use arrays and also use the options.php. I have never seen it done anyway.
The use of arrays and options ain't even in the documentation.
The whole options table is horrible in its construction.
Just because of that I saw the other post on here about arrays and options.php lol.
Still think the whole options thing is ugly though.
Andreas Nurbo » Option managing is pretty neat. You pass array or anything to option functions and let WP manage it. And, *clearly*, blaming lack of WP doc is just stating you can't search, either in the codex or in the source itself.
I have used arrays and options but never with options.php. I did it the old way.
You should not have to search in the code to learn stuff.
It is totally stupid. Statements like check in the code only shows the lack of documentation that WordPress suffers from.
And be my guest and try to find use of arrays and options in the codex. I didn't learn it there, I found it in a blog post.
If the documentation gives example of one "option" per add. That is what people will use. You can blame the plugin authors all you want but the fault is with WordPress and its doc.
Same goes with the cleanup process of removing options when a plugin is removed. This should have been automatic if the optionsmanagement were designed correctly.
Thanks for listing things down. Even though some of them are trivial, listing them down makes it a nice checklist.
I am guilty of fifty point (No uninstall function). Will have to modify my Plugins to include the uninstall function, when I get some time.
[…] I was reading a post by Ozh on common coding mistakes for WP plugins and I got to this section: I've seen code comments mentioning stuff like […]
A few useful links relating to points in this post. (Ohz, I'm a bit surprised you didn't link these! ;-) )
Use Classes in your WordPress plugins to increase code portability and reduce name conflicts
Consolidate Options with Arrays in your WordPress Plugins
Stephen R » Actually I'm linking the post about arrays & options like 20 times in my (yet to be published) 43 plugin reviews :) I missed the one about classes though, it's handy.
It's *very* disappointing to hear that basic PHP programming mistakes (like allowing raw $_GET data into SQL queries) are making their way into WP plugins, especially those submitted to the competition.
Let's just hope that this is only representative of the minority of plugins in this year's competition and that there are some outstanding entries too (I've not had time to take a look myself).
johnbillion » Yeah, part of me thinks it's a shame that so noob mistakes slip in. But the other part of me thinks it shows that WP's plugin architecture is simple enough for a beginner coder to play with, and hopefully improve and make better plugins and PHP code. I mean, I practically began coding PHP with WP, my first plugins must look terrible now.
[…] Top 10 Most Common Coding Mistakes in WordPress Plugins […]
Thanks for the rundown on plugin mistakes Ozh :)
I'm guessing that 87 option one is aimed at me. I haven't counted them all but I'm probably approaching that sort of number. I actually wrote a script to serialise them then just before the plugin competition realised that there was a built in WordPress way to do it and so ripped out that functionality until I got around to doing it properly in another release.
Here's the forum topic where Andrew Rickman and Will Anderson were providing advice on how to serialise arrays into the database in case anyone is interested in learning how to do it themselves:
http://www.wptavern.com/forum/plugins-hacks/466-multiple-get_option-array.html
Interestingly, I was actually told by others repeatedly that it was more appropriate to store them all separately as it made it more clear where the data was being stored in the database. In the end I got ticked off at all the crap in my own database caused by my own plugin … which ironically I still have't fixed this problem on :p
I think that's why I wasn't sure what the appropriate way to do it was originally. I generally try to follow the 'WordPress way' of doing things and since the codex page I would have read only suggested using a new option for each piece of info. then that is what I did. Having said that, it became fairly obvious that it wasn't a smart way to do things once I got past a dozen options. It clearly makes things messy and so it makes sense to look for a smarter way of doing it.
Here, LMGTFY (WTFs per minute, 3rd link):
http://www.osnews.com/story/19266/WTFs_m
So you can credit the artist.
Ross » Thanks for the link :)
wow, thank you so much, man, the commentary plus the related links, make this post a fantastique reference lookup I am going to share it around.
Cheers, homie!
hehe… no body's perfect, but thx for share. i think Matt Mullenweg will improve soon…
Excellent article!
Regarding point 8/9, I have also seen developers using query string like parameters to call some WP functions, for example
As we have seen in the past, this kind of code may allow to override private WP variables.
I agree so much; and as a developer, I laughed and shook my head sadly at the leading image to this article (my WTF/minute rate has climbed to alarmingly high levels at times. I believe reviewing the code for an older version of CubeCart had me spasmodically reaching for the Vicodin).
Matt, as far as naming conventions go… if we're using a plugin whose initials might be shortened to something that could *still cause* conflicts (e.g., "cf_" -> "Contact Form"?), would it still be wiser to write it out as "contactform_" in case another plugin would be using the same prefix? I imagine it'd get awfully verbose; and developers are going to crack the shits at the added typing every three or four lines…
Very informative and useful post! I will definitely have to refer to this to improve my plugins ASAP. Thanks!
Absolutely best information I have found in a while. I am a newbie to wordpress.I found your site recently and I am addicted.The information you give to people is just plain and simple Fantastic!.I am going to pass your link around to people that visit my pages.Thet have got to read your information. Please keep up the great work,very much appreciated.
Great list, I've actually found something to work on! Thanks for bringing this to the public and one of your previous posts about handling options is fantastic, definitely a must-read! Thanks so much!
~ @kovshenin
Great cartoon at the beginning! Over all I found the article made tremendous sense to myself! Awesome!
Skadoosh!
All so true, however – the wordpress core code isn't free from the generically named functions problem. On those rare occasions you need the WP API to coexist with something else, having a functiion named image_resize() (media.php) pretty much guarentees a conflict!
asking for a code review of my plugin !
http://wordpress.org/extend/plugins/mailpress/stats/
I've been hoping that there would be some official vetting/review of theme and plugin security. The plugin and theme repositories are filled with outdated, abandoned and terrible code mixed in with the most useful plugins and themes. It's tough to go through and dissect all those plugins that would be useful for clients at my particular skill level.
and while we're at it…this isn't a coding mistake, but am I the only one aggravated by the lack of screenshots, changelog, or live demo of a plugin on a developer's site? It seems to be a common oversight, but one that keeps me looking for a different option when doing research for new plugin options.
Superb! I forgot where this list was. I'm rewriting one of my plugins right now, and I'll be sure to use this list as a guideline.
I've got some learning to do as well. I like using different option values… because I can see them in options.php!
Maybe now that I've started using FirePHP I'll find a better way to deal with it.
Thanks again for writing this up and posting it.
Heyo, thanks for the post. Bookmarking for later reading. We have an early plugin candidate (it's done with an iframe at the moment).
Would love feedback from blog hosters, it's a contextual advertising tool that's powered by social media (twitter at the moment), and Zemanta. We're hoping to build user assistant tools to function as 2-way search and relevant, opt in ads.
Here's a link to it.
Would love feedback, interest, ideas on how to make it more appealing to hosts and users alike.
(gotta work on our landing site victusmedia some more it's an eyesore at the moment)
OK, there's a lot of good sense in this, but half of what it shows is just how hard it is to find good advice on coding for WordPerfect. If there's no best practice guide prominent in the WordPress development documentation, you shouldn't be surprised that new plugin authors don't use best practice. If the codex examples are bad practice, well of course you're going to see bad practice in the submitted widgets and plugins.
If the attitude is really 'read the code' to find out how to write your own extras, then I shan't bother to attempt to write anything from scratch and give it back, but will just tinker with existing snippets to make them do what I want – and only exactly what I want.
ooops obviously that should read WordPress, not WordPerfect
If I want to add json_encode and decode functions, I first check if they are present, if not I include a class file of json and make the functions then and there ;)
i want to know about way of creating the my own plugins
I'm just about to start out writting my first wordpress plugin and I found this article very useful and will no doubt define how I go about building my plugin especially as it allows me to identify some of the potential pitfalls before I am knee deep in code :)
Many thanks for a great and informative article.
Nice post to remind a WP developer what things need to know when to launch a plug-in or create one by maintaining security and best practices. Thanks for the post.
Awesome guys. Noted and bookmarked.
we'll follow it.
F.
Great article!
Thanks!
plugin_basename() is marked with "@access private". Is it safe to use?
Offtopic: country detection sucks: I am not from Poland.
A great article, but #8's not-so-broken example needs a fix.
GET requests should not alter server state, see rfc.
nbr » the RFC is just stupid here. GET is completely the same as POST except for the browser client behavior. It's vital that people stop thinking that POST is more secure.
36 years of programming and I learned a few things (and had a few chuckles). I may never write a WP plugin (there are so many great ones out there already), but this page is definitely bookmarked. Too much to assimilate in a single reading. Too many links. I'll be coming back to learn, over and over, until it sinks in.
Thank you for sharing.
Ozh: I am aware that in many ways GET is as secure or insecure as POST. But it's not just about security.
The decision to use GET requests for eg. writing stuff to database is also bad wrt. to crawlers. Let's assume you have a site with links such as "/my-precious-objects?id=42&action=delete" and you try to use a tool (with authentication support) to make an offline mirror of the site…
GET and POST have other differences, too. They behave differently for caching, reloading, redirecting, especially with proxies.