Thread:Extension talk:Semantic Need/Code review


 * General:
 * Don't use closing ?>, ever.
 * $wgExtensionMessagesFiles is supported since 1.16, this check makes no sense.
 * Don't use SMW-style localisation, it makes no sense in current MW versions.
 * Don't use comparison operators with $wgVersion, it's unreliable. Use version_compare.
 * Don't use global objects like $wgArticle, instead use RequestContext (for example, from $out).
 * Don't use superglobals like $_POST, there's WebRequest for that.
 * Don't create tables with hardcoded SQL, instead use LoadExtensionSchemaUpdates hook.
 * While I don't see an SQL injection after a brief glance, all DB code is extremely clumsy and looks like there's a lack of understanding what Database::tableName is for: it's for building table names with prefixes (using $wgDBprefix and $wgSharedTables), not for constructing filed names. All this code will fail with non-empty table prefix.

Double register_globals/XSS vulnerability, as it actually tries to substitute $IP.
 * SNE.php:
 * What Semantics from the people means?
 * Use 'descriptionmsg' with l10n instead of 'description'.
 * No need in global $wgVersion;


 * SNEBox.php:
 * Where is $sneBox defined? Looks like a register_globals vulnerability.


 * SNEHooks.php:
 * Don't include scripts and styles directly, use ResourceLoader.