r100165 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r100164‎ | r100165 | r100166 >
Date:21:37, 18 October 2011
Author:johnduhart
Status:resolved (Comments)
Tags:api, miscextensions, todo 
Comment:
Adds API Module by cryptocoryne

Bug 31723
Modified paths:
  • /trunk/extensions/CheckUser/CheckUser.i18n.php (modified) (history)
  • /trunk/extensions/CheckUser/CheckUser.php (modified) (history)
  • /trunk/extensions/CheckUser/CheckUser_body.php (modified) (history)
  • /trunk/extensions/CheckUser/api (added) (history)
  • /trunk/extensions/CheckUser/api/ApiQueryCheckUser.php (added) (history)
  • /trunk/extensions/CheckUser/api/ApiQueryCheckUserLog.php (added) (history)

Diff [purge]

Index: trunk/extensions/CheckUser/CheckUser_body.php
——@@ -1284,7 +1284,7 @@
12851285 * @param string $xfor
12861286 * @return mixed array/false conditions
12871287 */
1288 - protected function getIpConds( $db, $ip, $xfor = false ) {
 1288+ public static function getIpConds( $db, $ip, $xfor = false ) {
12891289 $type = ( $xfor ) ? 'xff' : 'ip';
12901290 // IPv4 CIDR, 16-32 bits
12911291 $matches = array();
——@@ -1452,7 +1452,7 @@
14531453 Xml::monthSelector( $encMonth, - 1 );
14541454 }
14551455
1456 - protected function addLogEntry( $logType, $targetType, $target, $reason, $targetID = 0 ) {
 1456+ public static function addLogEntry( $logType, $targetType, $target, $reason, $targetID = 0 ) {
14571457 global $wgUser;
14581458
14591459 if ( $targetType == 'ip' ) {
Index: trunk/extensions/CheckUser/CheckUser.i18n.php
——@@ -27,6 +27,7 @@
2828 'right-checkuser-log' => 'View the checkuser log',
2929 'grouppage-checkuser' => '{{ns:project}}:Check user',
3030 'checkuser-reason' => 'Reason:',
 31+ 'checkuser-reason-api' => 'API:',
3132 'checkuser-showlog' => 'Show log',
3233 'checkuser-log' => 'CheckUser log',
3334 'checkuser-query' => 'Query recent changes',
——@@ -136,6 +137,7 @@
137138 'checkuser-reason' => "Field name on CheckUser Special page. See screenshot '[http://www.mediawiki.org/wiki/Extension:CheckUser#Basic_interface Basic CheckUser interface]'.
138139
139140 {{Identical|Reason}}",
 141+ 'checkuser-reason-api' => 'Prefixes check user query reasons that are made through the API',
140142 'checkuser-showlog' => "Label for link on CheckUser Special page. See screenshot '[http://www.mediawiki.org/wiki/Extension:CheckUser#Basic_interface Basic CheckUser interface]'.",
141143 'checkuser-query' => "Fieldset label. See [http://www.mediawiki.org/wiki/Extension:CheckUser#Basic_interface screenshot titled 'Basic CheckUser interface'].",
142144 'checkuser-target' => '{{Identical|IP address or username}}',
Index: trunk/extensions/CheckUser/CheckUser.php
——@@ -84,3 +84,9 @@
8585
8686 $wgAutoloadClasses['CheckUser'] = dirname( __FILE__ ) . '/CheckUser_body.php';
8787 $wgAutoloadClasses['CheckUserHooks'] = dirname( __FILE__ ) . '/CheckUser.hooks.php';
 88+
 89+// API modules
 90+$wgAutoloadClasses['ApiQueryCheckUser'] = "$dir/api/ApiQueryCheckUser.php";
 91+$wgAPIListModules['checkuser'] = 'ApiQueryCheckUser';
 92+$wgAutoloadClasses['ApiQueryCheckUserLog'] = "$dir/api/ApiQueryCheckUserLog.php";
 93+$wgAPIListModules['checkuserlog'] = 'ApiQueryCheckUserLog';
\ No newline at end of file
Index: trunk/extensions/CheckUser/api/ApiQueryCheckUser.php
——@@ -0,0 +1,271 @@
 2+<?php
 3+/**
 4+ * CheckUser API Query Module
 5+ */
 6+
 7+class ApiQueryCheckUser extends ApiQueryBase {
 8+ public function __construct( $query, $moduleName ) {
 9+ parent::__construct( $query, $moduleName, 'cu' );
 10+ }
 11+
 12+ public function execute() {
 13+ global $wgUser, $wgCheckUserForceSummary;
 14+
 15+ $db = $this->getDB( DB_SLAVE );
 16+ $params = $this->extractRequestParams();
 17+
 18+ if( !$wgUser->isAllowed( 'checkuser' ) ) {
 19+ $this->dieUsage( 'You need the checkuser right', 'permissionerror' );
 20+ }
 21+
 22+ if( $wgCheckUserForceSummary && is_null($params['reason']) ) {
 23+ $this->dieUsage( 'You need define reason for check', 'missingdata' );
 24+ }
 25+
 26+ $limit = $params['limit'];
 27+ $target = $params['target'];
 28+ $reason = wfMsgForContent( 'checkuser-reason-api' ) . ' ' . $params['reason'];
 29+ $time = wfTimestamp( TS_MW, strtotime('now') - ( strtotime($params['timecond'] ? $params['timecond'] : '2 weeks') - strtotime('now')));
 30+ if( !$time ) {
 31+ $this->dieUsage( 'You need use correct time limit (like "2 weeks")', 'invalidtime' );
 32+ }
 33+
 34+ $this->addTables( 'cu_changes' );
 35+ $this->addOption( 'LIMIT', $limit + 1 );
 36+ $this->addOption( 'ORDER BY', 'cuc_timestamp DESC' );
 37+ $this->addWhere( "cuc_timestamp > $time" );
 38+
 39+ switch($params['request']) {
 40+ case 'userips':
 41+ $user_id = User::idFromName( $target );
 42+ if( !$user_id ) {
 43+ $this->dieUsage( 'Target user does not exist', 'nosuchuser' );
 44+ }
 45+
 46+ $this->addFields( array('cuc_timestamp', 'cuc_ip', 'cuc_xff') );
 47+ $this->addWhere( "cuc_user_text = '$target'" );
 48+ $res = $this->select( __METHOD__ );
 49+ $result = $this->getResult();
 50+
 51+ $ips = array();
 52+ foreach( $res as $row ) {
 53+ $timestamp = $row->cuc_timestamp;
 54+ $ip = strval($row->cuc_ip);
 55+ $xff = $row->cuc_xff;
 56+
 57+ if( !isset( $ips[$ip] ) ) {
 58+ $ips[$ip]['end'] = $timestamp;
 59+ $ips[$ip]['editcount'] = 1;
 60+ } else {
 61+ $ips[$ip]['start'] = $timestamp;
 62+ $ips[$ip]['editcount']++;
 63+ }
 64+ }
 65+
 66+ $count = 0;
 67+ foreach( array_keys($ips) as $ip ) {
 68+ $ips[$count] = $ips[$ip];
 69+ $ips[$count]['address'] = $ip;
 70+ unset($ips[$ip]);
 71+ $count++;
 72+ }
 73+
 74+ CheckUser::addLogEntry('userips', 'user', $target, $reason, $user_id);
 75+ $result->addValue( array( 'query', $this->getModuleName() ), 'userips', $ips );
 76+ $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'userips' ), 'ip' );
 77+ break;
 78+
 79+ case 'edits':
 80+ if( IP::isIPAddress($target) && isset($params['xff']) ) {
 81+ $cond = CheckUser::getIpConds($db, $target, true);
 82+ if( !$cond ) {
 83+ $this->dieUsage( 'IP or range is invalid', 'invalidip' );
 84+ }
 85+ $this->addWhere( "$cond" );
 86+ $log_type = array('ipedits-xff', 'ip');
 87+ } elseif ( IP::isIPAddress($target) ) {
 88+ $cond = CheckUser::getIpConds($db, $target);
 89+ if( !$cond ) {
 90+ $this->dieUsage( 'IP or range is invalid', 'invalidip' );
 91+ }
 92+ $this->addWhere( "$cond" );
 93+ $log_type = array('ipedits', 'ip');
 94+ } else {
 95+ $user_id = User::idFromName( $target );
 96+ if( !$user_id ) {
 97+ $this->dieUsage( 'Target user is not exists', 'nosuchuser' );
 98+ }
 99+ $this->addWhere( "cuc_user_text = '$target'" );
 100+ $log_type = array('useredits', 'user');
 101+ }
 102+
 103+ $this->addFields( array('cuc_namespace', 'cuc_title', 'cuc_user_text', 'cuc_actiontext', 'cuc_comment', 'cuc_minor', 'cuc_timestamp', 'cuc_ip', 'cuc_xff', 'cuc_agent') );
 104+
 105+ $res = $this->select( __METHOD__ );
 106+ $result = $this->getResult();
 107+
 108+ $edits = array();
 109+ $count = 0;
 110+ foreach( $res as $row ) {
 111+ $edits[$count]['timestamp'] = $row->cuc_timestamp;
 112+ $edits[$count]['ns'] = $row->cuc_namespace;
 113+ $edits[$count]['title'] = $row->cuc_title;
 114+ $edits[$count]['user'] = $row->cuc_user_text;
 115+ if( $row->cuc_actiontext ) {
 116+ $edits[$count]['summary'] = $row->cuc_actiontext;
 117+ } elseif( $row->cuc_comment ) {
 118+ $edits[$count]['summary'] = $row->cuc_comment;
 119+ }
 120+ if( $row->cuc_minor ) {
 121+ $edits[$count]['minor'] = 'm';
 122+ }
 123+ $edits[$count]['ip'] = $row->cuc_ip;
 124+ if( $row->cuc_xff ) {
 125+ $edits[$count]['xff'] = $row->cuc_xff;
 126+ }
 127+ $edits[$count]['agent'] = $row->cuc_agent;
 128+ $count++;
 129+ }
 130+
 131+ CheckUser::addLogEntry($log_type[0], $log_type[1], $target, $reason, $user_id ? $user_id : '0');
 132+ $result->addValue( array( 'query', $this->getModuleName() ), 'edits', $edits );
 133+ $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'edits' ), 'action' );
 134+ break;
 135+
 136+ case 'ipusers':
 137+ if( IP::isIPAddress($target) && isset($params['xff']) ) {
 138+ $cond = CheckUser::getIpConds($db, $target, true);
 139+ $this->addWhere( $cond );
 140+ } elseif ( IP::isIPAddress($target) ) {
 141+ $cond = CheckUser::getIpConds($db, $target);
 142+ $this->addWhere( $cond );
 143+ $log_type = 'ipusers';
 144+ } else {
 145+ $this->dieUsage( 'IP or range is invalid', 'invalidip' );
 146+ }
 147+
 148+ $this->addFields( array('cuc_user_text', 'cuc_timestamp', 'cuc_ip', 'cuc_agent') );
 149+
 150+ $res = $this->select( __METHOD__ );
 151+ $result = $this->getResult();
 152+
 153+ $users = array();
 154+ foreach( $res as $row ) {
 155+ $user = $row->cuc_user_text;
 156+ $ip = $row->cuc_ip;
 157+ $agent = $row->cuc_agent;
 158+
 159+ if( !isset($users[$user]) ) {
 160+ $users[$user]['end'] = $row->cuc_timestamp;
 161+ $users[$user]['editcount'] = 1;
 162+ $users[$user]['ips'][] = $ip;
 163+ $users[$user]['agents'][] = $agent;
 164+ } else {
 165+ $users[$user]['start'] = $row->cuc_timestamp;
 166+ $users[$user]['editcount']++;
 167+ if( !in_array( $ip, $users[$user]['ips'] ) ) $users[$user]['ips'][] = $ip;
 168+ if( !in_array( $agent, $users[$user]['agents'] ) ) $users[$user]['agents'][] = $agent;
 169+ }
 170+ }
 171+
 172+ $count = 0;
 173+ foreach( array_keys($users) as $user ) {
 174+ $users[$count] = $users[$user];
 175+ $users[$count]['name'] = $user;
 176+ unset($users[$user]);
 177+
 178+ $result->setIndexedTagName( $users[$count]['ips'], 'ip' );
 179+ $result->setIndexedTagName( $users[$count]['agents'], 'agent' );
 180+
 181+ $count++;
 182+ }
 183+
 184+ CheckUser::addLogEntry($log_type, 'ip', $target, $reason);
 185+ $result->addValue( array( 'query', $this->getModuleName() ), 'ipusers', $users );
 186+ $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'ipusers' ), 'user' );
 187+ break;
 188+
 189+ default:
 190+ $this->dieUsage( 'Invalid request mode', 'invalidmode' );
 191+ }
 192+ }
 193+
 194+ public function mustBePosted() {
 195+ return true;
 196+ }
 197+
 198+ public function isWriteMode() {
 199+ return true;
 200+ }
 201+
 202+ public function getAllowedParams() {
 203+ return array(
 204+ 'request' => array(
 205+ ApiBase::PARAM_REQUIRED => false,
 206+ ApiBase::PARAM_TYPE => array(
 207+ 'userips',
 208+ 'edits',
 209+ 'ipusers'
 210+ )
 211+ ),
 212+ 'target' => array(
 213+ ApiBase::PARAM_REQUIRED => false
 214+ ),
 215+ 'reason' => null,
 216+ 'limit' => array(
 217+ ApiBase::PARAM_DFLT => 1000,
 218+ ApiBase::PARAM_TYPE => 'limit',
 219+ ApiBase::PARAM_MIN => 1,
 220+ ApiBase::PARAM_MAX => 5000,
 221+ ApiBase::PARAM_MAX2 => 5000
 222+ ),
 223+ 'timecond' => null
 224+ );
 225+ }
 226+
 227+ public function getParamDescription() {
 228+ return array(
 229+ 'request' => array(
 230+ 'Type of CheckUser request',
 231+ ' userips - get IP of target user',
 232+ ' edits - get changes from target IP or range',
 233+ ' ipusers - get users from target IP or range',
 234+ ),
 235+ 'target' => "Username or IP-address/range to perform check",
 236+ 'reason' => 'Reason to check',
 237+ 'limit' => 'Limit of rows',
 238+ 'timecond' => 'Time limit of user data (like "2 weeks")'
 239+ );
 240+ }
 241+
 242+ public function getDescription() {
 243+ return 'Allows check which IPs are used by a given username and which usernames are used by a given IP';
 244+ }
 245+
 246+ public function getPossibleErrors() {
 247+ return array_merge( parent::getPossibleErrors(),
 248+ array(
 249+ array( 'nosuchuser' ),
 250+ array( 'invalidip' ),
 251+ array( 'permissionerror' ),
 252+ array( 'invalidmode' ),
 253+ array( 'missingdata' )
 254+ )
 255+ );
 256+ }
 257+
 258+ public function getExamples() {
 259+ return array(
 260+ 'api.php?action=query&list=checkuser&curequest=userips&cutarget=Jimbo_Wales',
 261+ 'api.php?action=query&list=checkuser&curequest=edits&cutarget=127.0.0.1/16/xff&cureason=Some_check'
 262+ );
 263+ }
 264+
 265+ public function getHelpUrls() {
 266+ return 'http://www.mediawiki.org/wiki/Extension:CheckUser#API';
 267+ }
 268+
 269+ public function getVersion() {
 270+ return __CLASS__ . ': $Id$';
 271+ }
 272+}
\ No newline at end of file
Property changes on: trunk/extensions/CheckUser/api/ApiQueryCheckUser.php
___________________________________________________________________
Added: svn:keywords
1273 + Id
Added: svn:eol-style
2274 + native
Index: trunk/extensions/CheckUser/api/ApiQueryCheckUserLog.php
——@@ -0,0 +1,117 @@
 2+<?php
 3+/**
 4+ * CheckUser API Query Module
 5+ */
 6+
 7+class ApiQueryCheckUserLog extends ApiQueryBase {
 8+ public function __construct( $query, $moduleName ) {
 9+ parent::__construct( $query, $moduleName, 'cul' );
 10+ }
 11+
 12+ public function execute() {
 13+ global $wgUser;
 14+
 15+ $params = $this->extractRequestParams();
 16+
 17+ if( !$wgUser->isAllowed( 'checkuser-log' ) ) {
 18+ $this->dieUsage( 'You need the checkuser-log right', 'permissionerror' );
 19+ }
 20+
 21+ $user = $params['user'];
 22+ $limit = $params['limit'];
 23+ $target = $params['target'];
 24+ $from = $params['from'];
 25+ $to = $params['to'];
 26+
 27+ $this->addTables( 'cu_log' );
 28+ $this->addOption( 'LIMIT', $limit + 1 );
 29+ $this->addOption( 'ORDER BY', 'cul_timestamp DESC' );
 30+
 31+ $this->addFields( array('cul_timestamp', 'cul_user_text', 'cul_reason', 'cul_type', 'cul_target_text') );
 32+
 33+ if( isset($user) ) $this->addWhere( "cul_user_text = '$user'" );
 34+ if( isset($target) ) $this->addWhere( "cul_target_text = '$target'" );
 35+ if( isset($from) && isset($to) ) {
 36+ $this->addWhere( "cul_timestamp BETWEEN '$from' AND '$to'" );
 37+ unset($from, $to);
 38+ } elseif ( isset($from) ) {
 39+ $this->addWhere( "cul_timestamp < $from" );
 40+ } elseif ( isset($to) ) {
 41+ $this->addWhere( "cul_timestamp > $to" );
 42+ }
 43+
 44+ $res = $this->select( __METHOD__ );
 45+ $result = $this->getResult();
 46+
 47+ $count = 0;
 48+ $log = array();
 49+ foreach( $res as $row ) {
 50+ $log[$count]['timestamp'] = $row->cul_timestamp;
 51+ $log[$count]['checkuser'] = $row->cul_user_text;
 52+ $log[$count]['type'] = $row->cul_type;
 53+ $log[$count]['reason'] = $row->cul_reason;
 54+ $log[$count]['target'] = $row->cul_target_text;
 55+ $count++;
 56+ }
 57+
 58+ $result->addValue( array( 'query', $this->getModuleName() ), 'entries', $log );
 59+ $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'entries' ), 'entry' );
 60+ }
 61+
 62+ public function getAllowedParams() {
 63+ return array(
 64+ 'user' => null,
 65+ 'target' => null,
 66+ 'limit' => array(
 67+ ApiBase::PARAM_DFLT => 10,
 68+ ApiBase::PARAM_TYPE => 'limit',
 69+ ApiBase::PARAM_MIN => 1,
 70+ ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1,
 71+ ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2
 72+ ),
 73+ 'from' => array(
 74+ ApiBase::PARAM_TYPE => 'timestamp'
 75+ ),
 76+ 'to' => array(
 77+ ApiBase::PARAM_TYPE => 'timestamp'
 78+ ),
 79+ );
 80+ }
 81+
 82+ public function getParamDescription() {
 83+ return array(
 84+ 'user' => 'Username of CheckUser',
 85+ 'target' => "Checked user or IP-address/range",
 86+ 'limit' => 'Limit of rows',
 87+ 'from' => 'The timestamp to start enumerating from',
 88+ 'to' => 'The timestamp to end enumerating'
 89+ );
 90+ }
 91+
 92+ public function getDescription() {
 93+ return 'Allows get entries of CheckUser log';
 94+ }
 95+
 96+ public function getPossibleErrors() {
 97+ return array_merge( parent::getPossibleErrors(),
 98+ array(
 99+ array( 'permissionerror' )
 100+ )
 101+ );
 102+ }
 103+
 104+ public function getExamples() {
 105+ return array(
 106+ 'api.php?action=query&list=checkuserlog&culuser=WikiSysop&limit=25',
 107+ 'api.php?action=query&list=checkuserlog&cultarget=127.0.0.1&culfrom=20111015230000'
 108+ );
 109+ }
 110+
 111+ public function getHelpUrls() {
 112+ return 'http://www.mediawiki.org/wiki/Extension:CheckUser#API';
 113+ }
 114+
 115+ public function getVersion() {
 116+ return __CLASS__ . ': $Id$';
 117+ }
 118+}
\ No newline at end of file
Property changes on: trunk/extensions/CheckUser/api/ApiQueryCheckUserLog.php
___________________________________________________________________
Added: svn:keywords
1119 + Id
Added: svn:eol-style
2120 + native

Follow-up revisions

RevisionCommit summaryAuthorDate
r100186Followup r100165, fixing formatting per CRjohnduhart23:41, 18 October 2011
r100187Followup r100165 and r100186: Formatting, requirement change, and removing mi...johnduhart23:46, 18 October 2011
r100189Follow up r100165:...johnduhart23:56, 18 October 2011
r100306Followup r100165, fix SQL injections and conditionsjohnduhart00:45, 20 October 2011
r107844Followup r100165, properly formatted timestampsjohnduhart18:09, 2 January 2012
r108856Refactored some code (reducing duplication)reedy21:51, 13 January 2012
r108859Fix eaten hyphen from r108856...reedy21:59, 13 January 2012
r108860intval NS from r100165...reedy22:01, 13 January 2012
r110807Follow-up r100165:...aaron23:58, 6 February 2012

Comments

#Comment by Cryptocoryne (talk | contribs)   22:05, 18 October 2011

Obviously in ApiQueryCheckUser.php need change ApiBase::PARAM_REQUIRED => false to true. My misprint.

#Comment by Johnduhart (talk | contribs)   23:47, 18 October 2011

r100187

#Comment by MaxSem (talk | contribs)   22:09, 18 October 2011 
$reason = wfMsgForContent( 'checkuser-reason-api' ) . ' ' . $params['reason'];

Not suitable for some languages, set that message to "API: $1" and use it like 

$reason = wfMsgForContent( 'checkuser-reason-api', $params['reason'] );
#Comment by Johnduhart (talk | contribs)   23:57, 18 October 2011

r100189

#Comment by Duplicatebug (talk | contribs)   13:16, 14 January 2012

Why you are adding a prefix to the reason for API calls? No other action module of the api makes this. In my opinion it is not necessary. What is the benefit of that?

#Comment by Reedy (talk | contribs)   22:47, 18 October 2011

There's also mixed tabs and spaces

And it could/should have been stylized before committing

#Comment by Johnduhart (talk | contribs)   23:44, 18 October 2011

yuck, oops. Fixed in r100186

#Comment by Reedy (talk | contribs)   22:48, 18 October 2011

It would also be good practise to swap $this-> for self:: on the callers of those methods changed

#Comment by Johnduhart (talk | contribs)   23:57, 18 October 2011

r100189

#Comment by Platonides (talk | contribs)   16:14, 19 October 2011

Double quoting one variable is meaningless. Just use the variable.

The $this->addWhere( "cuc_user_text = '$target'" ); calls allow SQL injection. Use addWhereFld. Same for $user, $from, $to.

#Comment by Johnduhart (talk | contribs)   00:45, 20 October 2011

r100306

#Comment by Catrope (talk | contribs)   16:27, 21 December 2011 
+        $time = wfTimestamp( TS_MW, strtotime('now') - ( strtotime($params['timecond'] ? $params['timecond'] : '2 weeks') - strtotime('now')));

WTF?

  1. Computing $now - ($timecond - $now) doesn't make much sense to me. What are you trying to do here?
  2. You should just be setting a PARAM_DEFAULT for 'timecond' rather than using a default value of 2 weeks implicitly
+        $this->addWhere( "cuc_timestamp > $time" );

$time needs to be quoted, preferably with $this->getDB()->addQuotes() so escaping is done too. 

+                        $ips[$ip]['end'] = $timestamp;
[...]
+                    $edits[$count]['timestamp'] = $row->cuc_timestamp;

Timestamps for output should be formatted as RFC2822. There are more occurrences of this all over the query module. 

+                    $edits[$count]['ns'] = $row->cuc_namespace;
+                    $edits[$count]['title'] = $row->cuc_title;

You should run the namespace through intval() so it'll be output as an integer in JSON and in any other output formats that might differentiate between integers and strings. Also, this means that User talk:Catrope looks like namespace:3, title:'Catrope' , is that what you intended? The most common format used in the API is namespace:3, title:"User talk:Catrope" . 

+                    if( $row->cuc_actiontext ) {
[...]
+                    } elseif( $row->cuc_comment ) {

What are you really trying to test for here? Emptiness (!== '')? Null-ness (!== null)? Casting to a boolean like this may give false negatives for things like '0' . 

+                ApiBase::PARAM_MAX => 5000,
+                ApiBase::PARAM_MAX2 => 5000

You should set these to ApiBase::LIMIT_BIG1 (=500) and ApiBase::LIMIT_BIG2 (=5000) respectively. QueryCheckUserLog does do this correctly.

#Comment by Platonides (talk | contribs)   00:39, 25 December 2011

Also note that those operations with strtotime() will have extra limits in 32 bit php. DateTime might be preferable (if it's really needed).

#Comment by Reedy (talk | contribs)   22:02, 13 January 2012

Marking new... I think everything is dealt with now. I dealt with a few left over things, and checking for the others seem ok... Bar maybe the comment about $row->cuc_title

#Comment by Aaron Schulz (talk | contribs)   23:51, 26 January 2012 
-	protected function getIpConds( $db, $ip, $xfor = false ) {
+	public static function getIpConds( $db, $ip, $xfor = false ) {

I'd rather these be moved to their own static class file rather than have the API borrow some special page functions.

#Comment by Aaron Schulz (talk | contribs)   00:05, 27 January 2012

FIXME per Roan's comment. Lots of stuff still there.

#Comment by RobLa-WMF (talk | contribs)   22:32, 6 February 2012

Per Chad's comment: we don't necessarily need to block deployment for this, since we can always disable the new APIs if they are a problem

#Comment by Aaron Schulz (talk | contribs)   23:51, 6 February 2012

Making $log_type an array is somewhat ugly here.

Status & tagging log