follow up r61357

* fix up fopen warnings — I was bedazzled by phpUnit's
  set_error_handler that turned warnings into exceptions and Tim set
  me right.
* Use Tim's read loop
* No need for private $fh member
* Fix some minor problems with PHP versions (no "timeout" param in
  pre-5.2.1, POST issue in Hardy's PHP version where timeout is null.)
* Fix how tests are run on old versions of phpUnit.

diff --git a/includes/HttpFunctions.php b/includes/HttpFunctions.php
index 8070b01..8a8dc50 100644 (file)
--- a/includes/HttpFunctions.php
+++ b/includes/HttpFunctions.php
@@ -184,7 +184,7 @@ class HttpRequest {
                case 'php':
                        if ( !wfIniGetBool( 'allow_url_fopen' ) ) {
                                throw new MWException( __METHOD__.': allow_url_fopen needs to be enabled for pure PHP http requests to work. '.
-                                       'If possible, curl should be used instead.  See http://php.net/curl.' );
+                                       'If possible, curl should be used instead.      See http://php.net/curl.' );
                        }
                        return new PhpHttpRequest( $url, $options );
                default:
@@ -365,8 +365,6 @@ class CurlHttpRequest extends HttpRequest {
 }
 
 class PhpHttpRequest extends HttpRequest {
-       private $fh;
-
        protected function urlToTcp( $url ) {
                $parsedUrl = parse_url( $url );
 
@@ -376,7 +374,7 @@ class PhpHttpRequest extends HttpRequest {
        public function execute() {
                if ( $this->parsedUrl['scheme'] != 'http' ) {
                        $this->status->fatal( 'http-invalid-scheme', $this->parsedURL['scheme'] );
-           }
+               }
 
                parent::execute();
                if ( !$this->status->isOK() ) {
@@ -399,26 +397,33 @@ class PhpHttpRequest extends HttpRequest {
                $options['method'] = $this->method;
                $options['timeout'] = $this->timeout;
                $options['header'] = implode("\r\n", $this->getHeaderList());
-               // FOR NOW: Force everyone to HTTP 1.0
-               /* if ( version_compare( "5.3.0", phpversion(), ">" ) ) { */
-                       $options['protocol_version'] = "1.0";
-               /* } else { */
-               /*      $options['protocol_version'] = "1.1"; */
-               /* } */
+               // Note that at some future point we may want to support
+               // HTTP/1.1, but we'd have to write support for chunking
+               // in version of PHP < 5.3.1
+               $options['protocol_version'] = "1.0";
 
                if ( $this->postData ) {
                        $options['content'] = $this->postData;
                }
 
+               $oldTimeout = false;
+               if ( version_compare( '5.2.1', phpversion(), '>' ) ) {
+                       $oldTimeout = ini_set('default_socket_timeout', $this->timeout);
+               }
+
                $context = stream_context_create( array( 'http' => $options ) );
-               try {
-                       $this->fh = fopen( $this->url, "r", false, $context );
-               } catch ( Exception $e ) {
-                       $this->status->fatal( $e->getMessage() ); /* need some l10n help */
+               wfSuppressWarnings();
+               $fh = fopen( $this->url, "r", false, $context );
+               wfRestoreWarnings();
+               if ( $oldTimeout !== false ) {
+                       ini_set('default_socket_timeout', $oldTimeout);
+               }
+               if ( $fh === false ) {
+                       $this->status->fatal( 'http-request-error' );
                        return $this->status;
                }
 
-               $result = stream_get_meta_data( $this->fh );
+               $result = stream_get_meta_data( $fh );
                if ( $result['timed_out'] ) {
                        $this->status->fatal( 'http-timed-out', $this->url );
                        return $this->status;
@@ -426,16 +431,17 @@ class PhpHttpRequest extends HttpRequest {
 
                $this->headers = $result['wrapper_data'];
 
-               $end = false;
-               while ( !$end ) {
-                       $contents = fread( $this->fh, 8192 );
-                       $size = 0;
-                       if ( $contents ) {
-                               $size = call_user_func_array( $this->callback, array( $this->fh, $contents ) );
+               while ( !feof( $fh ) ) {
+                       $buf = fread( $fh, 8192 );
+                       if ( $buf === false ) {
+                               $this->status->fatal( 'http-read-error' );
+                               break;
+                       }
+                       if ( strlen( $buf ) ) {
+                               call_user_func( $this->callback, $fh, $buf );
                        }
-                       $end = ( $size == 0 )  || feof( $this->fh );
                }
-               fclose( $this->fh );
+               fclose( $fh );
 
                return $this->status;
        }

diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php
index 013302e..86b04b1 100644 (file)
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -2154,7 +2154,7 @@ For optimal security, img_auth.php is disabled.',
 # HTTP errors
 'http-invalid-url'    => 'Invalid URL: $1',
 'http-invalid-scheme' => 'URLs with the "$1" scheme are not supported',
-'http-request-error'  => 'Error sending request:',
+'http-request-error'  => 'Unknown error sending request.',
 
 # Some likely curl errors. More could be added from <http://curl.haxx.se/libcurl/c/libcurl-errors.html>
 'upload-curl-error6'       => 'Could not reach URL',

diff --git a/tests/HttpTest.php b/tests/HttpTest.php
index 4d0731f..2a1037f 100644 (file)
--- a/tests/HttpTest.php
+++ b/tests/HttpTest.php
@@ -1,5 +1,9 @@
 <?php
 
+if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
+       require_once( 'bootstrap.php' );
+}
+
 class HttpTest extends PhpUnit_Framework_TestCase {
        static $content;
        static $headers;
@@ -25,7 +29,7 @@ class HttpTest extends PhpUnit_Framework_TestCase {
                self::$has_curl = function_exists( 'curl_init' );
 
                if ( !file_exists("/usr/bin/curl") ) {
-                       $this->markTestIncomplete("This test requires the curl binary at /usr/bin/curl.  If you have curl, please file a bug on this test, or, better yet, provide a patch.");
+                       $this->markTestIncomplete("This test requires the curl binary at /usr/bin/curl.  If you have curl, please file a bug on this test, or, better yet, provide a patch.");
                }
 
                $content = tempnam( sys_get_temp_dir(), "" );
@@ -65,7 +69,7 @@ class HttpTest extends PhpUnit_Framework_TestCase {
        function testInstantiation() {
                Http::$httpEngine = false;
 
-           $r = HttpRequest::factory("http://www.example.com/");
+               $r = HttpRequest::factory("http://www.example.com/");
                if ( self::$has_curl ) {
                        $this->assertThat($r, $this->isInstanceOf( 'CurlHttpRequest' ));
                } else {
@@ -120,7 +124,7 @@ class HttpTest extends PhpUnit_Framework_TestCase {
        }
 
        /* ./phase3/includes/Import.php:1108:           $data = Http::request( $method, $url ); */
-       /* ./includes/Import.php:1124:          $link = Title::newFromText( "$interwiki:Special:Export/$page" ); */
+       /* ./includes/Import.php:1124:                  $link = Title::newFromText( "$interwiki:Special:Export/$page" ); */
        /* ./includes/Import.php:1134:                  return ImportStreamSource::newFromURL( $url, "POST" ); */
        function runHTTPRequests($proxy=null) {
                $opt = array();
@@ -160,8 +164,8 @@ class HttpTest extends PhpUnit_Framework_TestCase {
        /* ./extensions/BookInformation/drivers/IsbnDb.php:24:                  if( ( $xml = Http::get( $uri ) ) !== false ) { */
        /* ./extensions/BookInformation/drivers/Amazon.php:23:                  if( ( $xml = Http::get( $uri ) ) !== false ) { */
        /* ./extensions/TitleBlacklist/TitleBlacklist.list.php:217:                     $result = Http::get( $url ); */
-       /* ./extensions/TSPoll/TSPoll.php:68:      $get_server = Http::get( 'http://toolserver.org/~jan/poll/dev/main.php?page=wiki_output&id='.$id ); */
-       /* ./extensions/TSPoll/TSPoll.php:70:      $get_server = Http::get( 'http://toolserver.org/~jan/poll/main.php?page=wiki_output&id='.$id ); */
+       /* ./extensions/TSPoll/TSPoll.php:68:      $get_server = Http::get( 'http://toolserver.org/~jan/poll/dev/main.php?page=wiki_output&id='.$id ); */
+       /* ./extensions/TSPoll/TSPoll.php:70:      $get_server = Http::get( 'http://toolserver.org/~jan/poll/main.php?page=wiki_output&id='.$id ); */
        /* ./extensions/DoubleWiki/DoubleWiki.php:56:                   $translation = Http::get( $url.$sep.'action=render' ); */
        /* ./extensions/ExternalPages/ExternalPages_body.php:177:                       $serializedText = Http::get( $this->mPageURL ); */
        /* ./extensions/Translate/utils/TranslationHelpers.php:143:             $suggestions = Http::get( $url, $timeout ); */
@@ -198,7 +202,7 @@ class HttpTest extends PhpUnit_Framework_TestCase {
        /* ./extensions/LocalisationUpdate/LocalisationUpdate.class.php:172:                            $basefilecontents = Http::get( $basefile ); */
        /* ./extensions/APC/SpecialAPC.php:245:         $rss = Http::get( 'http://pecl.php.net/feeds/pkg_apc.rss' ); */
        /* ./extensions/Interlanguage/Interlanguage.php:56:             $a = Http::get( $url ); */
-       /* ./extensions/MWSearch/MWSearch_body.php:492:         $data = Http::get( $searchUrl, $wgLuceneSearchTimeout, $httpOpts);  */
+       /* ./extensions/MWSearch/MWSearch_body.php:492:         $data = Http::get( $searchUrl, $wgLuceneSearchTimeout, $httpOpts);      */
        function runHTTPGets($proxy=null) {
                $opt = array();
 

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
index 1023f06..3f0e6be 100644 (file)
--- a/tests/bootstrap.php
+++ b/tests/bootstrap.php
@@ -13,12 +13,26 @@ $IP = dirname( dirname( __FILE__ ) );
 
 define( 'MEDIAWIKI', true );
 define( 'MW_PHPUNIT_TEST', true );
-ini_set( 'include_path', "$IP:" .ini_get( 'include_path' ) );
-
-require "$IP/includes/Defines.php";
-require "$IP/includes/AutoLoader.php";
-require "$IP/LocalSettings.php";
 
+require_once "$IP/includes/Defines.php";
+require_once "$IP/includes/AutoLoader.php";
+require_once "$IP/LocalSettings.php";
 require_once "$IP/includes/ProfilerStub.php";
 require_once "$IP/includes/GlobalFunctions.php";
 require_once "$IP/includes/Hooks.php";
+
+// for php versions before 5.2.1
+if ( !function_exists('sys_get_temp_dir')) {
+  function sys_get_temp_dir() {
+         if( $temp=getenv('TMP') )                return $temp;
+         if( $temp=getenv('TEMP') )            return $temp;
+         if( $temp=getenv('TMPDIR') )    return $temp;
+         $temp=tempnam(__FILE__,'');
+         if (file_exists($temp)) {
+                 unlink($temp);
+                 return dirname($temp);
+         }
+         return null;
+  }
+ }
+