Linux and PHP web application support and development (Bromsgrove, UK)

Xerte Online Toolkits

September 28, 2011, palepurple, development, , 0

Xerte Online Toolkits is an open source Flash+Ajax+PHP web application which allows tutors to create rich multimedia presentations through a web browser.

We’ve been hosting a couple of Xerte based sites (for Techdis and Warwick University) for a couple of years now. Being an open source project – we initially added support to the Techdis version so that it automatically cleans itself monthly and so on (therefore creating an ideal sandpit).

In the past we’d encountered problems with XOT which seemed to only affect our hosting environment – and we’d spend a few hours tracking them down, patch the code and then commit our fixes to our local subversion repository and post a patch on the mailing list. A couple of weeks ago we noticed toolkits 1.7 had been released, and asked one customer if they’d like to be upgraded. Not surprisingly, they did.

So we set up a demo site for the 1.7 code base, using a copy of a customer’s live database – hoping it would suffice to illustrate the new code work(s) before upgrading their live version. Unfortunately it didn’t run – the database schema had significantly changed between versions – and there remains no obvious guide/help as to how to upgrade it.

Next up, we tried installing from scratch – but then found the various bugs we’d squashed out before had reappeared (or hadn’t been fixed in upstream).

At this point it became obvious that we needed to push our changes upstream to save ourselves future work.

So, over the last couple of days, we’ve been fairly busy, and thankfully the Xerte project have given us commit access – so hopefully our changes won’t be lost. This is just a short summary of a few changes we’ve made to the Subversion trunk of toolkits:

The existing codebase is vulnerable to SQL injection, and needs PHP’s magic_quotes functionality enabled. For various reasons (beyond the scope of this short blog post) this isn’t ideal.

Toolkit’s often issues database queries like the below :

 

include $site->php_library_path . "database_library.php";
$mysql_id=database_connect("index.php database connect success",
                                              "index.php database connect fail");
$query_response = mysql_query("SELECT x From y WHERE z = '{$_POST['template_id']}');
if($query_response === false) {
     receive_message($_SESSION['login_ldap'], "admin",
                                "critical", "something is wrong", "something is wrong");
} else {
    $row = mysql_fetch_array($query_response);
    echo $row['blahblah'];
}

 

Some problems with which are :

  1. Insecure – relying on magic_quotes to protect against SQL injection
  2. It’s not escaping the output
  3. Verbose – it’s easy enough to shrink the above into something more concise (you’ll see effectively the same 8-10 lines pasted throughout the code base with minor changes)
  4. Poor error handling – what if we fail to connect to the database? What if the query string is invalid or no records are returned? Will we as developers have any way of finding out what error occurred yesterday? Or will it just silently fail for Miss O’Reilly?
  5. We’re obviously tied to always using MySQL

The code base also ran with PHP’s error_reporting effectively turned off – so undefined variable warnings and so on were not being reported

So, some fixes we’ve made are for instance :

  1. Add a db_query($sql, $params) function which performs a crude emulation of prepared statements – so providing some protection from SQL Injection. e.g.$rows = db_query("select * from foo where id = ?", array($id));
  2. Create convenience method(s) for recording debug messages (_debug($string)), so if something breaks some sort of audit trail exists.
  3. Through the config.php file, support a ‘development’ flag, which if enabled will set the error_reporting and _debug() function to work/do stuff
  4. Create a db_query_one($sql, $params) function – this returns a single row or null – ideal for use if you are only expecting one row back – for example, retrieval by primary key.

So the above code ends up being shrunk to :

 

require_once("config.php");
$row = db_query_one("SELECT x From y WHERE z = ?", array($_POST['template_id']));
if($row === null) {
     receive_message($_SESSION['login_id'], "admin",
                                "critical", "something is wrong", "something is wrong");
} else {
    echo $row['x'];
}

 

config.php always loads the database_library.php file, and starts a PHP session – before these were handled in a slightly haphazard manner – and in reality as it’s not possible for the someone to use the app and not require the functionality from either.

Other fixes/changes made include :

  1. Stopping some obvious Javascript errors from occurring with newer browsers – such as Chrome.
  2. Fixing some Javascript related bugs – for example, popups being created but having empty messages
  3. Merging in 5-6 patches others had contributed to the project from the Google Code bug tracker
  4. Addition of some documentation covering the setup of LDAP for the application’s authentication
  5. Addition of a simple logging function (_debug($string)) which should help others when installing the application in the future).
  6. Re-indentation of some of the code base – so it more closely follows more popular PHP coding styles (e.g. 4 space indentation and so on).
  7. Tracking down of why XOT wasn’t correctly creating a new Learning Object – this appears to have been related to a copy function which did a depth first copy (and then returned in the wrong place – so new templates were always left blank if .svn directories were present as the contents of the .svn directory would be copied, but not the “real” files.). We fixed this through refactoring to use a ‘cp -r’ (copy_r) style function (more readable, ignores .svn directories and so on).
  8. Simplification of input validation and error reporting in the login script

There is plenty of work remaining to be done – for instance :

  1. Removal of all ‘manual’ MySQL queries to be replaced with db_query*(), and therefore remove the chance of SQL injection.
  2. Removal of the legacy Javascript, and changing to use something like jQuery which should provide for greater cross browser compatibility
  3. Return of data to Javascript (from PHP) to be via the JSON format, rather than inline (and often) unstructured strings.
  4. Adding some sort of upgrade capability (either documentation or hopefully an automated script which just ‘does it’

Still, at least a start has been made.

, ,

‹ ›

Leave a Reply

Your email address will not be published. Required fields are marked *

Share

Search

Customer Quotes

  • We’ve been using Pale Purple for over 5 years now. There’s a lot of benefits to be gained from building a close relationship with a small but very stable and capable team. Paul Hiscoe
    TransparencyData.co.uk

Copyright Pale Purple Ltd 2006 - 2013. Pale Purple is a trading name of Pale Purple Limited, Registered in England and Wales (Registered No. 5580814)