r89528 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r89527‎ | r89528 | r89529 >
Date:19:51, 5 June 2011
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
* (bug 22179) Internal use of API (FauxRequest) results in HTTP headers being set


Per Chad, switch API to use WebResponse::header() wrapper

Add $http_response_code to WebResponse::header()


Fix some code spacing/whitespace issues
Modified paths:
  • /trunk/phase3/includes/WebResponse.php (modified) (history)
  • /trunk/phase3/includes/api/ApiFormatBase.php (modified) (history)
  • /trunk/phase3/includes/api/ApiMain.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiFormatBase.php
@@ -145,11 +145,10 @@
146146 if ( is_null( $mime ) ) {
147147 return; // skip any initialization
148148 }
149 -
150 - if( !$this->getMain()->isInternalMode() ) {
151 - header( "Content-Type: $mime; charset=utf-8" );
152 - }
153149
 150+ global $wgRequest;
 151+ $wgRequest->response()->header( "Content-Type: $mime; charset=utf-8" );
 152+
154153 if ( $isHtml ) {
155154 ?>
156155 <!DOCTYPE HTML>
Index: trunk/phase3/includes/api/ApiMain.php
@@ -370,11 +370,13 @@
371371 // Error results should not be cached
372372 $this->setCacheMode( 'private' );
373373
 374+ global $wgRequest;
 375+ $response = $wgRequest->response();
374376 $headerStr = 'MediaWiki-API-Error: ' . $errCode;
375377 if ( $e->getCode() === 0 ) {
376 - header( $headerStr );
 378+ $response->header( $headerStr );
377379 } else {
378 - header( $headerStr, true, $e->getCode() );
 380+ $response->header( $headerStr, true, $e->getCode() );
379381 }
380382
381383 // Reset and print just the error message
@@ -397,26 +399,29 @@
398400 }
399401
400402 protected function sendCacheHeaders() {
 403+ global $wgRequest;
 404+ $response = $wgRequest->response();
 405+
401406 if ( $this->mCacheMode == 'private' ) {
402 - header( 'Cache-Control: private' );
 407+ $response->header( 'Cache-Control: private' );
403408 return;
404409 }
405410
406411 if ( $this->mCacheMode == 'anon-public-user-private' ) {
407412 global $wgUseXVO, $wgOut;
408 - header( 'Vary: Accept-Encoding, Cookie' );
 413+ $response->header( 'Vary: Accept-Encoding, Cookie' );
409414 if ( $wgUseXVO ) {
410 - header( $wgOut->getXVO() );
 415+ $response->header( $wgOut->getXVO() );
411416 if ( $wgOut->haveCacheVaryCookies() ) {
412417 // Logged in, mark this request private
413 - header( 'Cache-Control: private' );
 418+ $response->header( 'Cache-Control: private' );
414419 return;
415420 }
416421 // Logged out, send normal public headers below
417422 } elseif ( session_id() != '' ) {
418423 // Logged in or otherwise has session (e.g. anonymous users who have edited)
419424 // Mark request private
420 - header( 'Cache-Control: private' );
 425+ $response->header( 'Cache-Control: private' );
421426 return;
422427 } // else no XVO and anonymous, send public headers below
423428 }
@@ -433,7 +438,7 @@
434439 // Public cache not requested
435440 // Sending a Vary header in this case is harmless, and protects us
436441 // against conditional calls of setCacheMaxAge().
437 - header( 'Cache-Control: private' );
 442+ $response->header( 'Cache-Control: private' );
438443 return;
439444 }
440445
@@ -442,7 +447,7 @@
443448 // Send an Expires header
444449 $maxAge = min( $this->mCacheControl['s-maxage'], $this->mCacheControl['max-age'] );
445450 $expiryUnixTime = ( $maxAge == 0 ? 1 : time() + $maxAge );
446 - header( 'Expires: ' . wfTimestamp( TS_RFC2822, $expiryUnixTime ) );
 451+ $response->header( 'Expires: ' . wfTimestamp( TS_RFC2822, $expiryUnixTime ) );
447452
448453 // Construct the Cache-Control header
449454 $ccHeader = '';
@@ -459,7 +464,7 @@
460465 }
461466 }
462467
463 - header( "Cache-Control: $ccHeader" );
 468+ $response->header( "Cache-Control: $ccHeader" );
464469 }
465470
466471 /**
@@ -588,8 +593,12 @@
589594 $maxLag = $params['maxlag'];
590595 list( $host, $lag ) = wfGetLB()->getMaxLag();
591596 if ( $lag > $maxLag ) {
592 - header( 'Retry-After: ' . max( intval( $maxLag ), 5 ) );
593 - header( 'X-Database-Lag: ' . intval( $lag ) );
 597+ global $wgRequest;
 598+ $response = $wgRequest->response();
 599+
 600+ $response->header( 'Retry-After: ' . max( intval( $maxLag ), 5 ) );
 601+ $response->header( 'X-Database-Lag: ' . intval( $lag ) );
 602+
594603 if ( $wgShowHostnames ) {
595604 $this->dieUsage( "Waiting for $host: $lag seconds lagged", 'maxlag' );
596605 } else {
Index: trunk/phase3/includes/WebResponse.php
@@ -17,12 +17,14 @@
1818 * header()
1919 * @param $string String: header to output
2020 * @param $replace Bool: replace current similar header
 21+ * @param $http_response_code null|int Forces the HTTP response code to the specified value.
2122 */
22 - public function header($string, $replace=true) {
23 - header($string,$replace);
 23+ public function header( $string, $replace = true, $http_response_code = null ) {
 24+ header( $string, $replace, $http_response_code );
2425 }
2526
26 - /** Set the browser cookie
 27+ /**
 28+ * Set the browser cookie
2729 * @param $name String: name of cookie
2830 * @param $value String: value to give cookie
2931 * @param $expire Int: number of seconds til cookie expires
@@ -59,15 +61,15 @@
6062 private $headers;
6163 private $cookies;
6264
63 - public function header($string, $replace=true) {
64 - list($key, $val) = explode(":", $string, 2);
 65+ public function header( $string, $replace = true, $http_response_code = null ) {
 66+ list( $key, $val ) = explode( ":", $string, 2 );
6567
66 - if($replace || !isset($this->headers[$key])) {
 68+ if( $replace || !isset( $this->headers[$key] ) ) {
6769 $this->headers[$key] = $val;
6870 }
6971 }
7072
71 - public function getheader($key) {
 73+ public function getheader( $key ) {
7274 return $this->headers[$key];
7375 }
7476
@@ -76,7 +78,7 @@
7779 }
7880
7981 public function getcookie( $name ) {
80 - if ( isset($this->cookies[$name]) ) {
 82+ if ( isset( $this->cookies[$name] ) ) {
8183 return $this->cookies[$name];
8284 }
8385 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r89531Followup r89528, don't use $wgRequest use $this->getMain()->getRequest()->res...reedy20:29, 5 June 2011

Comments

#Comment by Bryan (talk | contribs)   20:20, 5 June 2011

Do you really need to introduce the use of the $wgRequest global? You should be able to use $this->getMain()->getRequest() always.

#Comment by Reedy (talk | contribs)   20:30, 5 June 2011

Calling $this->getMain() in a Main class is just daft ;)

Status & tagging log