r91460 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r91459‎ | r91460 | r91461 >
Date:15:05, 5 July 2011
Author:ialex
Status:resolved (Comments)
Tags:
Comment:
* Don't create a WebRequest obhject in CLI mode but a FauxRequest; avoids some useless notices about headers already sent (I know this is more a PHP silliness, but anyway)
* Added HTTP response code parsing (sending a "HTTP/1.x code" header was throwing a NOTICE about undefined index on the result of the explode() call) and storage; added FauxResponse::getStatusCode() to retrieve it
Modified paths:
  • /trunk/phase3/includes/Setup.php (modified) (history)
  • /trunk/phase3/includes/WebResponse.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Setup.php
@@ -364,14 +364,16 @@
365365 $wgLocalTZoffset = date( 'Z' ) / 60;
366366 }
367367
368 -# Can't stub this one, it sets up $_GET and $_REQUEST in its constructor
369 -$wgRequest = new WebRequest;
370 -
371368 # Useful debug output
372369 global $wgCommandLineMode;
373370 if ( $wgCommandLineMode ) {
 371+ $wgRequest = new FauxRequest( array() );
 372+
374373 wfDebug( "\n\nStart command line script $self\n" );
375374 } else {
 375+ # Can't stub this one, it sets up $_GET and $_REQUEST in its constructor
 376+ $wgRequest = new WebRequest;
 377+
376378 $debug = "Start request\n\n{$_SERVER['REQUEST_METHOD']} {$wgRequest->getRequestURL()}";
377379
378380 if ( $wgDebugPrintHttpHeaders ) {
Index: trunk/phase3/includes/WebResponse.php
@@ -77,6 +77,7 @@
7878 class FauxResponse extends WebResponse {
7979 private $headers;
8080 private $cookies;
 81+ private $code;
8182
8283 /**
8384 * Stores a HTTP header
@@ -85,11 +86,20 @@
8687 * @param $http_response_code null|int Forces the HTTP response code to the specified value.
8788 */
8889 public function header( $string, $replace = true, $http_response_code = null ) {
89 - list( $key, $val ) = explode( ":", $string, 2 );
 90+ $match = array();
 91+ if ( preg_match( '~^HTTP/1.\d (\d+)\D*$~', $string, $match ) ) {
 92+ $this->code = intval( $match[1] );
 93+ } else {
 94+ list( $key, $val ) = explode( ":", $string, 2 );
9095
91 - if( $replace || !isset( $this->headers[$key] ) ) {
92 - $this->headers[$key] = $val;
 96+ if( $replace || !isset( $this->headers[$key] ) ) {
 97+ $this->headers[$key] = $val;
 98+ }
9399 }
 100+
 101+ if ( $http_response_code !== null ) {
 102+ $this->code = intval( $http_response_code );
 103+ }
94104 }
95105
96106 /**
@@ -101,6 +111,15 @@
102112 }
103113
104114 /**
 115+ * Get the HTTP response code, null if not set
 116+ *
 117+ * @return Int or null
 118+ */
 119+ public function getStatusCode() {
 120+ return $this->code;
 121+ }
 122+
 123+ /**
105124 * @param $name String: name of cookie
106125 * @param $value String: value to give cookie
107126 * @param $expire Int: number of seconds til cookie expires

Follow-up revisions

RevisionCommit summaryAuthorDate
r91463Per Brion, fix for r91460: make this a bit more robustialex16:40, 5 July 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   16:25, 5 July 2011
~^HTTP/1.\d (\d+)\D*$~

^ this worries me a bit; for one thing it won't match 'HTTP/1.x' which we have used at times in the past (we might have replaced those, not sure offhand); the \D* also means that if the message that comes with happens to include a digit, it won't match. Shouldn't usually have a digit, but it's worth checking.

Probably should just check if the line starts with 'HTTP/'

#Comment by IAlex (talk | contribs)   16:40, 5 July 2011

Done in r91463.

Status & tagging log