Merge branch 'hotfix/198' into develop

Forward port #198
zendframework · Nov 29, 2019 · 668c37c · 668c37c
2 parents 178e869 + 56e210e
commit 668c37c
Show file tree

Hide file tree

Showing 14 changed files with 239 additions and 86 deletions.
diff --git a/.ci/php5.6.ini b/.ci/php5.6.ini
@@ -0,0 +1 @@
+always_populate_raw_post_data=-1
diff --git a/.ci/proxy.conf b/.ci/proxy.conf
@@ -0,0 +1,6 @@
+<VirtualHost *:8081>
+  ProxyRequests On
+
+  ErrorLog ${APACHE_LOG_DIR}/error.log
+  CustomLog ${APACHE_LOG_DIR}/access.log combined
+</VirtualHost>
diff --git a/.ci/site.conf b/.ci/site.conf
@@ -0,0 +1,21 @@
+<VirtualHost *:80>
+  DocumentRoot %TRAVIS_BUILD_DIR%/test/Client/_files
+
+  <Directory "%TRAVIS_BUILD_DIR%/test/Client/_files/">
+    Options FollowSymLinks MultiViews ExecCGI
+    AllowOverride All
+    Require all granted
+  </Directory>
+
+  # Wire up Apache to use Travis CI's php-fpm.
+  <IfModule mod_fastcgi.c>
+    AddHandler php%PHP_VERSION%-fcgi .php
+    Action php%PHP_VERSION%-fcgi /php%PHP_VERSION%-fcgi
+    Alias /php%PHP_VERSION%-fcgi /usr/lib/cgi-bin/php%PHP_VERSION%-fcgi
+    FastCgiExternalServer /usr/lib/cgi-bin/php%PHP_VERSION%-fcgi -host 127.0.0.1:9000 -pass-header Authorization
+
+    <Directory /usr/lib/cgi-bin>
+        Require all granted
+    </Directory>
+  </IfModule>
+</VirtualHost>
diff --git a/.travis.yml b/.travis.yml
@@ -9,6 +9,9 @@ env:
     - COMPOSER_ARGS="--no-interaction"
     - COVERAGE_DEPS="php-coveralls/php-coveralls"
     - TESTS_ZEND_HTTP_CLIENT_ONLINE=true
+    - TESTS_ZEND_HTTP_CLIENT_BASEURI=http://127.0.0.1
+    - TESTS_ZEND_HTTP_CLIENT_HTTP_PROXY=127.0.0.1:8081
+    - TESTS_ZEND_HTTP_CLIENT_NOTRESPONDINGURI=http://127.1.1.0:1234
 
 matrix:
   include:
@@ -73,12 +76,40 @@ install:
   - if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry composer require --dev $COMPOSER_ARGS $COVERAGE_DEPS ; fi
   - stty cols 120 && composer show
 
+before_script:
+  # custom php.ini for PHP 5.6
+  - if [[ ${TRAVIS_PHP_VERSION:0:3} == "5.6" ]]; then phpenv config-add .ci/php5.6.ini; fi
+  # install apache
+  - sudo apt-get update -qq
+  - sudo apt-get install apache2 libapache2-mod-fastcgi
+  # enable php-fpm
+  - sudo cp ~/.phpenv/versions/$(phpenv version-name)/etc/php-fpm.conf.default ~/.phpenv/versions/$(phpenv version-name)/etc/php-fpm.conf
+  - sudo a2enmod rewrite actions fastcgi alias
+  - echo "cgi.fix_pathinfo = 1" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini
+  - sudo sed -i -e "s,www-data,travis,g" /etc/apache2/envvars
+  - sudo chown -R travis:travis /var/lib/apache2/fastcgi
+  - ~/.phpenv/versions/$(phpenv version-name)/sbin/php-fpm
+  # configure apache virtual hosts
+  - sudo cp -f .ci/site.conf /etc/apache2/sites-available/000-default.conf
+  - sudo sed -e "s?%TRAVIS_BUILD_DIR%?$(pwd)?g" --in-place /etc/apache2/sites-available/000-default.conf
+  - sudo sed -e "s?%PHP_VERSION%?$(phpenv version-name)?g" --in-place /etc/apache2/sites-available/000-default.conf
+  # enable TRACE
+  - sudo sed -e "s?TraceEnable Off?TraceEnable On?g" --in-place /etc/apache2/conf-available/security.conf
+  # configure proxy
+  - sudo a2enmod proxy proxy_http proxy_connect
+  - sudo cp -f .ci/proxy.conf /etc/apache2/sites-available/proxy.conf
+  - sudo a2ensite proxy
+  - sudo sed -i "s/Listen 80/Listen 80\nListen 8081/" /etc/apache2/ports.conf
+  - sudo service apache2 restart
+
 script:
   - if [[ $TEST_COVERAGE == 'true' ]]; then composer test-coverage ; else composer test ; fi
   - if [[ $CS_CHECK == 'true' ]]; then composer cs-check ; fi
 
 after_script:
   - if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry php vendor/bin/php-coveralls -v ; fi
+  - sudo cat /var/log/apache2/error.log
+  - sudo cat /var/log/apache2/access.log
 
 notifications:
   email: false
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -52,6 +52,12 @@ All notable changes to this project will be documented in this file, in reverse
 
 - [#189](https://github.com/zendframework/zend-http/pull/189) fixes `Headers::toArray` method to properly handle headers of the same type. Behaviour was different depends how header has been attached (`addHeader` or `addHeaderLine` broken before). 
 
+- [#198](https://github.com/zendframework/zend-http/pull/198) fixes merging options in Curl adapter. It was not possible to override integer-key options (constants) set via constructor with method `setOptions`. 
+
+- [#198](https://github.com/zendframework/zend-http/pull/198) fixes allowed options type in `Proxy::setOptions`. `Traversable`, `array` or `Zend\Config` object is expected.
+
+- [#198](https://github.com/zendframework/zend-http/pull/198) fixes various issues with `Proxy` adapter.
+
 ## 2.10.0 - 2019-02-19
 
 ### Added

diff --git a/src/Client/Adapter/Curl.php b/src/Client/Adapter/Curl.php
@@ -143,7 +143,7 @@ public function setOptions($options = [])
                     break;
                 default:
                     if (is_array($v) && isset($this->config[$option]) && is_array($this->config[$option])) {
-                        $v = ArrayUtils::merge($this->config[$option], $v);
+                        $v = ArrayUtils::merge($this->config[$option], $v, true);
                     }
                     $this->config[$option] = $v;
                     break;
@@ -425,16 +425,16 @@ public function write($method, $uri, $httpVersion = 1.1, $headers = [], $body =
          * Make sure POSTFIELDS is set after $curlMethod is set:
          * @link http://de2.php.net/manual/en/function.curl-setopt.php#81161
          */
-        if (in_array($method, ['POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], true)) {
-            curl_setopt($this->curl, CURLOPT_POSTFIELDS, $body);
-        } elseif ($curlMethod == CURLOPT_UPLOAD) {
+        if ($curlMethod == CURLOPT_UPLOAD) {
             // this covers a PUT by file-handle:
             // Make the setting of this options explicit (rather than setting it through the loop following a bit lower)
             // to group common functionality together.
             curl_setopt($this->curl, CURLOPT_INFILE, $this->config['curloptions'][CURLOPT_INFILE]);
             curl_setopt($this->curl, CURLOPT_INFILESIZE, $this->config['curloptions'][CURLOPT_INFILESIZE]);
             unset($this->config['curloptions'][CURLOPT_INFILE]);
             unset($this->config['curloptions'][CURLOPT_INFILESIZE]);
+        } elseif (in_array($method, ['POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], true)) {
+            curl_setopt($this->curl, CURLOPT_POSTFIELDS, $body);
         }
 
         // set additional curl options

diff --git a/src/Client/Adapter/Proxy.php b/src/Client/Adapter/Proxy.php
@@ -7,9 +7,11 @@
 
 namespace Zend\Http\Client\Adapter;
 
+use Traversable;
 use Zend\Http\Client;
 use Zend\Http\Client\Adapter\Exception as AdapterException;
 use Zend\Http\Response;
+use Zend\Stdlib\ArrayUtils;
 use Zend\Stdlib\ErrorHandler;
 
 /**
@@ -60,6 +62,15 @@ class Proxy extends Socket
      */
     public function setOptions($options = [])
     {
+        if ($options instanceof Traversable) {
+            $options = ArrayUtils::iteratorToArray($options);
+        }
+        if (! is_array($options)) {
+            throw new AdapterException\InvalidArgumentException(
+                'Array or Zend\Config object expected, got ' . gettype($options)
+            );
+        }
+
         //enforcing that the proxy keys are set in the form proxy_*
         foreach ($options as $k => $v) {
             if (preg_match('/^proxy[a-z]+/', $k)) {
@@ -93,6 +104,7 @@ public function connect($host, $port = 80, $secure = false)
         /* Url might require stream context even if proxy connection doesn't */
         if ($secure) {
             $this->config['sslusecontext'] = true;
+            $this->setSslCryptoMethod = false;
         }
 
         // Connect (a non-secure connection) to the proxy server
@@ -129,7 +141,10 @@ public function write($method, $uri, $httpVer = '1.1', $headers = [], $body = ''
         $host = $this->config['proxy_host'];
         $port = $this->config['proxy_port'];
 
-        if ($this->connectedTo[0] != sprintf('tcp://%s', $host) || $this->connectedTo[1] != $port) {
+        $isSecure = strtolower($uri->getScheme()) === 'https';
+        $connectedHost = ($isSecure ? $this->config['ssltransport'] : 'tcp') . '://' . $host;
+
+        if ($this->connectedTo[1] !== $port || $this->connectedTo[0] !== $connectedHost) {
             throw new AdapterException\RuntimeException(
                 'Trying to write but we are connected to the wrong proxy server'
             );
@@ -145,24 +160,29 @@ public function write($method, $uri, $httpVer = '1.1', $headers = [], $body = ''
         }
 
         // if we are proxying HTTPS, preform CONNECT handshake with the proxy
-        if ($uri->getScheme() == 'https' && ! $this->negotiated) {
+        if ($isSecure && ! $this->negotiated) {
             $this->connectHandshake($uri->getHost(), $uri->getPort(), $httpVer, $headers);
             $this->negotiated = true;
         }
 
         // Save request method for later
         $this->method = $method;
 
-        // Build request headers
-        if ($this->negotiated) {
-            $path = $uri->getPath();
-            $query = $uri->getQuery();
-            $path .= $query ? '?' . $query : '';
-            $request = sprintf('%s %s HTTP/%s%s', $method, $path, $httpVer, "\r\n");
-        } else {
-            $request = sprintf('%s %s HTTP/%s%s', $method, $uri, $httpVer, "\r\n");
+        if ($uri->getUserInfo()) {
+            $headers['Authorization'] = 'Basic ' . base64_encode($uri->getUserInfo());
         }
 
+        $path = $uri->getPath();
+        $query = $uri->getQuery();
+        $path .= $query ? '?' . $query : '';
+
+        if (! $this->negotiated) {
+            $path = $uri->getScheme() . '://' . $uri->getHost() . $path;
+        }
+
+        // Build request headers
+        $request = sprintf('%s %s HTTP/%s%s', $method, $path, $httpVer, "\r\n");
+
         // Add all headers to the request string
         foreach ($headers as $k => $v) {
             if (is_string($k)) {
@@ -182,7 +202,7 @@ public function write($method, $uri, $httpVer = '1.1', $headers = [], $body = ''
         ErrorHandler::start();
         $test  = fwrite($this->socket, $request);
         $error = ErrorHandler::stop();
-        if (! $test) {
+        if ($test === false) {
             throw new AdapterException\RuntimeException('Error writing request to proxy server', 0, $error);
         }
 

diff --git a/src/Client/Adapter/Socket.php b/src/Client/Adapter/Socket.php
@@ -86,6 +86,11 @@ class Socket implements HttpAdapter, StreamInterface
      */
     protected $context;
 
+    /**
+     * @var bool
+     */
+    protected $setSslCryptoMethod = true;
+
     /**
      * Adapter constructor, currently empty. Config is set using setOptions()
      *
@@ -301,47 +306,52 @@ public function connect($host, $port = 80, $secure = false)
             }
 
             if ($secure || $this->config['sslusecontext']) {
-                if ($this->config['ssltransport'] && isset(static::$sslCryptoTypes[$this->config['ssltransport']])) {
-                    $sslCryptoMethod = static::$sslCryptoTypes[$this->config['ssltransport']];
-                } else {
-                    $sslCryptoMethod = STREAM_CRYPTO_METHOD_SSLv3_CLIENT;
-                }
+                if ($this->setSslCryptoMethod) {
+                    if ($this->config['ssltransport']
+                        && isset(static::$sslCryptoTypes[$this->config['ssltransport']])
+                    ) {
+                        $sslCryptoMethod = static::$sslCryptoTypes[$this->config['ssltransport']];
+                    } else {
+                        $sslCryptoMethod = STREAM_CRYPTO_METHOD_SSLv3_CLIENT;
+                    }
 
-                ErrorHandler::start();
-                $test  = stream_socket_enable_crypto($this->socket, true, $sslCryptoMethod);
-                $error = ErrorHandler::stop();
-                if (! $test || $error) {
-                    // Error handling is kind of difficult when it comes to SSL
-                    $errorString = '';
-                    if (extension_loaded('openssl')) {
-                        while (($sslError = openssl_error_string()) != false) {
-                            $errorString .= sprintf('; SSL error: %s', $sslError);
+                    ErrorHandler::start();
+                    $test  = stream_socket_enable_crypto($this->socket, true, $sslCryptoMethod);
+                    $error = ErrorHandler::stop();
+                    if (! $test || $error) {
+                        // Error handling is kind of difficult when it comes to SSL
+                        $errorString = '';
+                        if (extension_loaded('openssl')) {
+                            while (($sslError = openssl_error_string()) != false) {
+                                $errorString .= sprintf('; SSL error: %s', $sslError);
+                            }
                         }
-                    }
-                    $this->close();
-
-                    if ((! $errorString) && $this->config['sslverifypeer']) {
-                        // There's good chance our error is due to sslcapath not being properly set
-                        if (! ($this->config['sslcafile'] || $this->config['sslcapath'])) {
-                            $errorString = 'make sure the "sslcafile" or "sslcapath" option are properly set for the '
-                                . 'environment.';
-                        } elseif ($this->config['sslcafile'] && ! is_file($this->config['sslcafile'])) {
-                            $errorString = 'make sure the "sslcafile" option points to a valid SSL certificate file';
-                        } elseif ($this->config['sslcapath'] && ! is_dir($this->config['sslcapath'])) {
-                            $errorString = 'make sure the "sslcapath" option points to a valid SSL certificate '
-                                . 'directory';
+                        $this->close();
+
+                        if ((! $errorString) && $this->config['sslverifypeer']) {
+                            // There's good chance our error is due to sslcapath not being properly set
+                            if (! ($this->config['sslcafile'] || $this->config['sslcapath'])) {
+                                $errorString = 'make sure the "sslcafile" or "sslcapath" option are properly set for '
+                                    . 'the environment.';
+                            } elseif ($this->config['sslcafile'] && ! is_file($this->config['sslcafile'])) {
+                                $errorString = 'make sure the "sslcafile" option points to a valid SSL certificate '
+                                    . 'file';
+                            } elseif ($this->config['sslcapath'] && ! is_dir($this->config['sslcapath'])) {
+                                $errorString = 'make sure the "sslcapath" option points to a valid SSL certificate '
+                                    . 'directory';
+                            }
                         }
-                    }
 
-                    if ($errorString) {
-                        $errorString = sprintf(': %s', $errorString);
-                    }
+                        if ($errorString) {
+                            $errorString = sprintf(': %s', $errorString);
+                        }
 
-                    throw new AdapterException\RuntimeException(sprintf(
-                        'Unable to enable crypto on TCP connection %s%s',
-                        $host,
-                        $errorString
-                    ), 0, $error);
+                        throw new AdapterException\RuntimeException(sprintf(
+                            'Unable to enable crypto on TCP connection %s%s',
+                            $host,
+                            $errorString
+                        ), 0, $error);
+                    }
                 }
 
                 $host = $this->config['ssltransport'] . '://' . $host;

diff --git a/src/Headers.php b/src/Headers.php
@@ -194,7 +194,7 @@ public function addHeaderLine($headerFieldNameOrLine, $fieldValue = null)
             $headerName = $headerFieldNameOrLine;
             $headerKey  = static::createKey($headerFieldNameOrLine);
             if (is_array($fieldValue)) {
-                $fieldValue = implode(', ', $fieldValue);
+                $fieldValue = implode('; ', $fieldValue);
             }
             $line = $headerFieldNameOrLine . ': ' . $fieldValue;
         }

diff --git a/src/Response.php b/src/Response.php
@@ -564,8 +564,10 @@ protected function decodeGzip($body)
             );
         }
 
-        if ($this->getHeaders()->has('content-length')
-            && 0 === (int) $this->getHeaders()->get('content-length')->getFieldValue()) {
+        if ($body === ''
+            || ($this->getHeaders()->has('content-length')
+                && (int) $this->getHeaders()->get('content-length')->getFieldValue() === 0)
+        ) {
             return '';
         }