r89405 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r89404‎ | r89405 | r89406 >
Date:10:41, 3 June 2011
Author:ialex
Status:resolved (Comments)
Tags:
Comment:
Deprecated wfGetAgent(), use $wgRequest->getHeader( 'User-Agent' ) instead
Modified paths:
  • /trunk/extensions/CheckUser/CheckUser.php (modified) (history)
  • /trunk/extensions/Collection/Collection.body.php (modified) (history)
  • /trunk/extensions/SignDocument/SignDocumentHelpers.php (modified) (history)
  • /trunk/phase3/includes/ProxyTools.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ProxyTools.php
@@ -41,9 +41,12 @@
4242 /**
4343 * Returns the browser/OS data from the request header
4444 * Note: headers are spoofable
 45+ *
 46+ * @deprecated in 1.19; use $wgRequest->getHeader( 'User-Agent' ) instead.
4547 * @return string
4648 */
4749 function wfGetAgent() {
 50+ wfDeprecated( __FUNCTION__ );
4851 if( function_exists( 'apache_request_headers' ) ) {
4952 // More reliable than $_SERVER due to case and -/_ folding
5053 $set = array();
Index: trunk/extensions/CheckUser/CheckUser.php
@@ -85,6 +85,8 @@
8686 * Saves user data into the cu_changes table
8787 */
8888 function efUpdateCheckUserData( $rc ) {
 89+ global $wgRequest;
 90+
8991 // Extract params
9092 extract( $rc->mAttribs );
9193 // Get IP
@@ -95,7 +97,7 @@
9698 // Our squid XFFs can flood this up sometimes
9799 $isSquidOnly = efXFFChainIsSquid( $xff );
98100 // Get agent
99 - $agent = wfGetAgent();
 101+ $agent = $wgRequest->getHeader( 'User-Agent' );
100102 // Store the log action text for log events
101103 // $rc_comment should just be the log_comment
102104 // BC: check if log_type and log_action exists
@@ -154,13 +156,15 @@
155157 * Saves user data into the cu_changes table
156158 */
157159 function efUpdateCUPasswordResetData( $user, $ip, $account ) {
 160+ global $wgRequest;
 161+
158162 // Get XFF header
159163 $xff = wfGetForwardedFor();
160164 list( $xff_ip, $trusted ) = efGetClientIPfromXFF( $xff );
161165 // Our squid XFFs can flood this up sometimes
162166 $isSquidOnly = efXFFChainIsSquid( $xff );
163167 // Get agent
164 - $agent = wfGetAgent();
 168+ $agent = $wgRequest->getHeader( 'User-Agent' );
165169 $dbw = wfGetDB( DB_MASTER );
166170 $cuc_id = $dbw->nextSequenceValue( 'cu_changes_cu_id_seq' );
167171 $rcRow = array(
@@ -192,7 +196,7 @@
193197 * Saves user data into the cu_changes table
194198 */
195199 function efUpdateCUEmailData( $to, $from, $subject, $text ) {
196 - global $wgSecretKey;
 200+ global $wgSecretKey, $wgRequest;
197201 if ( !$wgSecretKey || $from->name == $to->name ) {
198202 return true;
199203 }
@@ -207,7 +211,7 @@
208212 // Our squid XFFs can flood this up sometimes
209213 $isSquidOnly = efXFFChainIsSquid( $xff );
210214 // Get agent
211 - $agent = wfGetAgent();
 215+ $agent = $wgRequest->getHeader( 'User-Agent' );
212216 $dbw = wfGetDB( DB_MASTER );
213217 $cuc_id = $dbw->nextSequenceValue( 'cu_changes_cu_id_seq' );
214218 $rcRow = array(
@@ -252,6 +256,8 @@
253257 * @return bool
254258 */
255259 function efLogUserAccountCreation( $user, $actiontext ) {
 260+ global $wgRequest;
 261+
256262 // Get IP
257263 $ip = wfGetIP();
258264 // Get XFF header
@@ -260,7 +266,7 @@
261267 // Our squid XFFs can flood this up sometimes
262268 $isSquidOnly = efXFFChainIsSquid( $xff );
263269 // Get agent
264 - $agent = wfGetAgent();
 270+ $agent = $wgRequest->getHeader( 'User-Agent' );
265271 $dbw = wfGetDB( DB_MASTER );
266272 $cuc_id = $dbw->nextSequenceValue( 'cu_changes_cu_id_seq' );
267273 $rcRow = array(
Index: trunk/extensions/Collection/Collection.body.php
@@ -1267,7 +1267,7 @@
12681268
12691269 static function curlreq( $method, $url, $postFields, &$errorMessage, &$info,
12701270 $timeout = true, $toFile = null ) {
1271 - global $wgHTTPTimeout, $wgHTTPProxy, $wgTitle, $wgVersion;
 1271+ global $wgHTTPTimeout, $wgHTTPProxy, $wgTitle, $wgRequest, $wgVersion;
12721272 global $wgCollectionMWServeCert;
12731273 global $wgCollectionVersion;
12741274
@@ -1276,7 +1276,7 @@
12771277 }
12781278 $c = curl_init( $url );
12791279 curl_setopt( $c, CURLOPT_PROXY, $wgHTTPProxy );
1280 - $userAgent = wfGetAgent();
 1280+ $userAgent = $wgRequest->getHeader( 'User-Agent' );
12811281 if ( !$userAgent ) {
12821282 $userAgent = "Unknown user agent";
12831283 }
Index: trunk/extensions/SignDocument/SignDocumentHelpers.php
@@ -375,7 +375,7 @@
376376 $f->mEmail = $wgRequest->getVal( 'email', '' );
377377
378378 $f->mIp = wfGetIp();
379 - $f->mAgent = wfGetAgent();
 379+ $f->mAgent = $wgRequest->getHeader( 'User-Agent' );
380380
381381 if ( $wgRequest->getVal( 'anonymous' ) ) $f->mHiddenFields[] = 'realname';
382382 if ( $wgRequest->getVal( 'hideaddress' ) ) $f->mHiddenFields[] = 'address';

Follow-up revisions

RevisionCommit summaryAuthorDate
r101858Per Aaron, fix for r89405: introduced DerivativeRequest to allow to override ...ialex19:14, 3 November 2011

Comments

#Comment by Aaron Schulz (talk | contribs)   04:40, 31 October 2011

Actually, this doesn't work for API edit (causing bug 32027). This is yet another bug caused by the API using FauxRequest and overriding $wgRequest...

I see $wgRequest->getHeader( 'X-Forwarded-For' ) in the /trunk code, which will suffer similarly. FauxRequest lacks getIP() for the moment, so it would at least fatal if anyone tried to update CU to use that.

#Comment by IAlex (talk | contribs)   19:26, 3 November 2011

Fixed in r101858, r101859 and r101861 (yes three commits).

Note that FauxRequest extends WebRequest, thus getIP() will fall back to WebRequest's implementation (with the getRawIP() change from FauxResponse)

#Comment by Aaron Schulz (talk | contribs)   19:29, 3 November 2011

Yeah, I forgot the extends relationship there.

Status & tagging log