Preserve caller expectations for behaviour of sslVerifyHost
authorTim Starling <tstarling@wikimedia.org>
Thu, 28 Feb 2013 01:58:52 +0000 (17:58 -0800)
committerTim Starling <tstarling@wikimedia.org>
Thu, 28 Feb 2013 02:03:44 +0000 (18:03 -0800)
(bug 42441) The previous patch unnecessarily broke backwards
compatibility in the Http::request() API, following cURL's broken
conventions for sslVerifyHost instead of the boolean interpretation
expected by all existing callers. This change reverts that one, and
fixes the bug in another way. See Ia6535f10.

Also don't bother wrapping the $this->sslVerifyHost access with isset()
since it's always set.

Change-Id: Ia4e1689249b6ac515b859ea2eca1dcff3e63098c

includes/HttpFunctions.php

index 6a348a6..892e0ed 100644 (file)
@@ -45,9 +45,7 @@ class Http {
         *                          Otherwise it will use $wgHTTPProxy (if set)
         *                          Otherwise it will use the environment variable "http_proxy" (if set)
         *    - noProxy             Don't use any proxy at all. Takes precedence over proxy value(s).
-        *    - sslVerifyHost       (curl only) Set to 2 to verify hostname against certificate
-        *                                  Setting to 1 (or true) will NOT verify the host name. It will
-        *                                  only check its existence. Setting to 0 (or false) disables entirely.
+        *    - sslVerifyHost       (curl only) Verify hostname against certificate
         *    - sslVerifyCert       (curl only) Verify SSL certificate
         *    - caInfo              (curl only) Provide CA information
         *    - maxRedirects        Maximum number of redirects to follow (defaults to 5)
@@ -187,15 +185,7 @@ class MWHttpRequest {
        protected $postData = null;
        protected $proxy = null;
        protected $noProxy = false;
-       /**
-        * Parameter passed to Curl that specifies whether
-        * to validate SSL certificates.
-        *
-        * Setting to 0 disables entirely. Setting to 1 checks
-        * the existence of a CN, but doesn't verify it. Setting
-        * to 2 (the default) actually verifies the host.
-        */
-       protected $sslVerifyHost = 2;
+       protected $sslVerifyHost = true;
        protected $sslVerifyCert = true;
        protected $caInfo = null;
        protected $method = "GET";
@@ -731,13 +721,8 @@ class CurlHttpRequest extends MWHttpRequest {
                }
                $this->curlOptions[CURLOPT_USERAGENT] = $this->reqHeaders['User-Agent'];
 
-               if ( isset( $this->sslVerifyHost ) ) {
-                       $this->curlOptions[CURLOPT_SSL_VERIFYHOST] = $this->sslVerifyHost;
-               }
-
-               if ( isset( $this->sslVerifyCert ) ) {
-                       $this->curlOptions[CURLOPT_SSL_VERIFYPEER] = $this->sslVerifyCert;
-               }
+               $this->curlOptions[CURLOPT_SSL_VERIFYHOST] = $this->sslVerifyHost ? 2 : 0;
+               $this->curlOptions[CURLOPT_SSL_VERIFYPEER] = $this->sslVerifyCert;
 
                if ( $this->caInfo ) {
                        $this->curlOptions[CURLOPT_CAINFO] = $this->caInfo;