Topic on Extension talk:Semantic Need

MaxSem (talkcontribs)
  • 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.
  • SNE.php:
        echo <<<EOT
To install my extension, put the following line in LocalSettings.php:
require_once( "$IP/extensions/SemanticNeed/SNE.php" );
EOT;

Double register_globals/XSS vulnerability, as it actually tries to substitute $IP.

    • 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:
Reply to "Code review"