1. Home
  2. Blog
  3. Showcase
  4. Contact
Blogitems
13Oct

10 Pitfalls for extension developers

Posted on October 13th 2010 at 09:00 inside Extension Development with 5 comments

While developing and reviewing TYPO3 extension, you occasionally come upon pretty funny and sometimes disturbing stuff. At some point we all have to learn new things. So we can't expect it all to go right and well from the start.

If you have some experience with extension development, you might be thinking "Is this real?". Yes, it all is (unfortunately). If you're reading these examples and you're thinking "Is this bad?" then you can learn some things. blinkert

The Goal

This article isn't about burning down people for there 'noobism'. It's actually to help future TYPO3 developers. To help them avoid these silly mistakes. We assume these examples take place inside a frontend plugin, while extending tslib_pibase.

Here we go!

1. The Rhetorical Frontend User!

The Rhetorical Frontend User! 

  1. $user = $this->pi_getRecord('fe_users',$GLOBALS['TSFE']->fe_user->user['uid']);
$user = $this->pi_getRecord('fe_users',$GLOBALS['TSFE']->fe_user->user['uid']);

Explanation: We are grabbing all data for the current frontend user. The code functions perfectly well. The problem is just that $GLOBALS['TSFE']->fe_user->user already contains this data. So we are requesting data we already have available.

2. Mythical piVars

Mythical piVars 

  1. $recordUid = $_GET['uid'];
$recordUid = $_GET['uid'];

Explanation: In this case, no piVars are used. This can be problematic in certain cases. piVars are designed to make sure extensions don't overwrite each others variables that are passed on by query-string or post-back. See if all extensions would use just 'uid' we would get problems here.

3. piWhatever!?

piWhatever!? 

  1. $recordUid = $_GET[$this->prefxId]['showUid'];
$recordUid = $_GET[$this->prefxId]['showUid'];

Explanation: Ok we are somewhere, were using piVars! But why on earth aren't we using the local storage for retrieving the info we need?

4. Homebrewn piVars

Homebrewn piVars 

  1. class tx_myext_pi1 extends tslib_pibase {
  2.         var $prefixId = 'tx_myext_pi1';
  3.  
  4.         function main($content,$conf) {
  5.                 $piVars = t3lib_div::_GP('tx_myext_pi1');
  6.         }
  7. }
class tx_myext_pi1 extends tslib_pibase {
	var $prefixId = 'tx_myext_pi1';

	function main($content,$conf) {
		$piVars = t3lib_div::_GP('tx_myext_pi1');
	}
}

Explanation: Again, the code will function perfectly well, it just has two flaws. The biggest one is that the plugin variables ($this->piVars) are by default already set inside as a class variable when extending tslib_pibase. Second of all, the ID of the extension 'tx_myext_pi1' is coded into the function, thought its by default available as a class variable.

5. Master of the SQL injection

Master of the SQL injection 

  1. $showUid = $this->piVars['showUid'];
  2. $rows = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
  3.         '*',
  4.         'fe_users',
  5.         'uid = '.$showUid
  6. );
$showUid = $this->piVars['showUid'];
$rows = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
	'*',
	'fe_users',
	'uid = '.$showUid
);

Explanation: Since the input is not verified or filtered, this is one of the most classic examples of SQL injection. If you think that by using TYPO3's wrapper function will save you, you're wrong. The only moment when the wrapper functions might escape values for is when you use either the exec_INSERTquery() or exec_UPDATEquery() wrapper.

6. I <3 MySQL

I <3 MySQL 

  1. $sql = 'SELECT * FROM fe_users WHERE 1';
  2. $res = mysql_query($sql);
$sql = 'SELECT * FROM fe_users WHERE 1';
$res = mysql_query($sql);

Explanation: TYPO3 comes shipped with a nice database API, which is globally accessible (via $GLOBALS['TYPO3_DB']). This API provides some very nice SQL wrapper functions, that will allow you to get data easily in a nicely structured way. By not using the database API, you make sure that users who don't run TYPO3 on MySQL will not be able to use your extension.

7. Power Debuggers

Power Debuggers 

  1. $productUid = intval($this->piVars[showUid]);
  2. if($productUid > 0) {
  3.         echo '1';
  4.         // Do some stuff
  5.         print_r(2);
  6. }
$productUid = intval($this->piVars[showUid]);
if($productUid > 0) {
	echo '1';
	// Do some stuff
	print_r(2);
}

Explanation: From time to time developers might quickly want to debug certain stuff. And often they forget to take out their little debug prints and echoes. If someone else worked on the code, and you are the one looking where there hell the '1' might be printed you might know how extremely annoying this is. This is because you simply don't know what someone is doing to print this. Are they using echo, print_r? Is it a hard-coded variable? Is it generated? In case you do want to debug something quickly, use t3lib_div::debug() instead. This is much easier to trace back, and can be configured to show only for specified IP-address.

8. Do-it-yourself

Do-it-yourself 

  1. $records = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
  2.         '*',
  3.         'pages',
  4.         'pid = 10 AND NOT hidden AND NOT deleted'
  5. );
$records = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
	'*',
	'pages',
	'pid = 10 AND NOT hidden AND NOT deleted'
);

Explanation: Although it may look like perfectly normal code, it's not the right way to do it. Again we have 2 problems, which you might easily overlook on. Both problems lie in the WHERE clause. The first one is simple, we don't need to write this code. We can make TYPO3 do it for us. Simply by using $this->cObj->enableFields('pages'). This will automatically handle any visibility like hidden, deleted and user access. By using this also enable special handling by the admin tool in the front-end. The second problem is again database system dependency. Since 'NOT' is not supported by all database systems, and '!=' is.

9. Do-it-yourself with CSV

Do-it-yourself with CSV 

  1. $record = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
  2.         '*',
  3.         'be_users',
  4.         'FIND_IN_SET(2,usergroup)'
  5. );
$record = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
	'*',
	'be_users',
	'FIND_IN_SET(2,usergroup)'
);

Explanation: The problems are the same as in the previous example. Just with a simple twist. First of all, there is a function that can do the FIND_IN_SET for you. It's also shipped with the database API. So instead of using FIND_IN_SET, we should use this: $GLOBALS['TYPO3_DB']->listQuery('usergroup',2,'be_users'). This will also kill the 2nd problem, which is database system dependency. FIND_IN_SET is as far as I know only supported by MySQL.

10. The no-cacher

The no-cacher 

  1. $GLOBALS['TSFE']->set_no_cache();
$GLOBALS['TSFE']->set_no_cache();

Explanation: In general this might not even be bad, but some developers tend to use it in the wrong way. By setting this, you will disable ALL caching. None of the content on your page will be cached. Meaning that this has to be rendered every time the page is requested. In case you want to disable caching for only your plugin, this is not the right way to do it. Especially on larger websites with many modules, plugins and requests, this can be a big performance issue. Disabling cache for your plugin should be done either via static templates (plugin.tx_myext_pi1 = USER_INT) or inside your ext_localconf.php.

Creating an uncached extension. 

  1. t3lib_extMgm::addPItoST43($_EXTKEY, 'pi1/class.tx_myext_pi1.php', '_pi1', 'CType', 0);
t3lib_extMgm::addPItoST43($_EXTKEY, 'pi1/class.tx_myext_pi1.php', '_pi1', 'CType', 0);

The last parameter in this case (the '0') disables caching (the plugin will run as USER_INT). The other way around, by setting this to '1' you will enable caching for your plugin (USER).

Some tips

  • If you try to do something, try doing it the TYPO3 way. In many occasions there will be either an API or a wrapper function available. This will make you write less code, and will make sure you are doing it in a compatible and good way.
  • If you are trying to access data from records that are already loaded or displayed, try to see if the data is already available. The object $GLOBALS['TSFE'] contains an incredible amount of data. If you want to find out what, simply try to print it! (Using the debug function of course!). Be aware that some megabytes of data might be printed, so your browser might not be to happy about it.
  • If you're not sure if something is right or wrong, or if there is a better way to do it… just ask it. There are plenty of ways to learn new stuff and ask other developers. Thinks of mailings lists, forums and IRC.
  • Validate user input. I can't say this enough honestly, many developers don't know or simply don't care about security. The impact is often underestimated. No matter what, always validate user input. This might save you your job one day..
  • In order to understand better how TYPO3 works, you should check out some of it's code every now and then. Of course, I'm not telling you to go through all of the code. Just some parts. If your curious how something might work, take a look at it. This way you will give yourself a great advantage. It also helps to look at how other extension builders approaches, they might know something you don't. And you can learn from that.

Got some great example?

Share it! Put it down in a comment and share with the rest of the world. So others can learn from these mistakes. And of course, so we all can have a laugh about it. blinkert

Tags

typo3 extension development fails beginners pitfalls bad practices

Comments (5)

Gravatar: Nitin SewtahalsingNitin Sewtahalsing2010-10-13 10:47

Great article!

I think it is a great way to show extension developers that some things are a lot faster if you use these things.
 
I also recommend to print the $GLOBALS['TSFE'] array to see what's already in it. It saves you a lot of time.

Gravatar: Benjamin SerfhosBenjamin Serfhos2010-10-13 10:47

Reminds me of the old days

Ha, some of the code reminds me of my first extension.. =)
 
For point 8; it could be possible that there is no $this->cObj. In most of the scenarios you can make your own local_cObj, using this->local_cObj = t3lib_div::makeInstance('tslib_cObj');
And for the backend, you can use the t3lib_BEfunc::BEenableFields(); function.
 
At the end, good post.

Gravatar: IngoIngo2010-10-13 11:05

shortcut

Instead of t3lib_div::debug() you can go ahead and just use debug() as a shortcut

Gravatar: SargoDaryaSargoDarya2010-10-25 10:57

Thanks

Forgot how to disable extension based caching. If I'd be in IRC it would be a definitive Prot0++ ;) Maybe I'll catch that up later.

Gravatar: Sebastiaan de JongeSebastiaan de Jonge2010-10-25 11:29

RE: Thanks

Hehe, thanks mate. Though, last time I checked Karmalicious wasn't adding too much ;-)

Your comment