Skip to content

Commit

Permalink
Merge pull request #61 from boesing/bugfix/http-version-float
Browse files Browse the repository at this point in the history
Regression: non-float HTTP version detection
  • Loading branch information
boesing authored Dec 3, 2021
2 parents f2c60df + 8b6561d commit 261f079
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 9 deletions.
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"laminas/laminas-validator": "^2.15"
},
"require-dev": {
"ext-curl": "*",
"laminas/laminas-coding-standard": "~2.2.1",
"phpunit/phpunit": "^9.5.5"
},
Expand Down
6 changes: 4 additions & 2 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions src/Client/Adapter/Curl.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
use function in_array;
use function intval;
use function is_array;
use function is_float;
use function is_numeric;
use function is_resource;
use function number_format;
use function preg_match;
use function preg_replace;
use function preg_split;
Expand Down Expand Up @@ -334,7 +336,7 @@ public function connect($host, $port = 80, $secure = false)
*
* @param string $method
* @param Uri $uri
* @param float $httpVersion
* @param float|string $httpVersion
* @param array $headers
* @param string $body
* @return string $request
Expand All @@ -344,8 +346,12 @@ public function connect($host, $port = 80, $secure = false)
* @throws AdapterException\InvalidArgumentException If $method is currently not supported.
* @throws AdapterException\TimeoutException If connection timed out.
*/
public function write($method, $uri, $httpVersion = 1.1, $headers = [], $body = '')
public function write($method, $uri, $httpVersion = '1.1', $headers = [], $body = '')
{
if (is_float($httpVersion)) {
$httpVersion = number_format($httpVersion, 1, '.', '');
}

// Make sure we're properly connected
if (! $this->curl) {
throw new AdapterException\RuntimeException('Trying to write but we are not connected');
Expand Down Expand Up @@ -442,7 +448,7 @@ public function write($method, $uri, $httpVersion = 1.1, $headers = [], $body =
}

// get http version to use
$curlHttp = $httpVersion === 1.1 ? CURL_HTTP_VERSION_1_1 : CURL_HTTP_VERSION_1_0;
$curlHttp = $httpVersion === '1.1' ? CURL_HTTP_VERSION_1_1 : CURL_HTTP_VERSION_1_0;

// mark as HTTP request and set HTTP method
curl_setopt($this->curl, CURLOPT_HTTP_VERSION, $curlHttp);
Expand Down
72 changes: 72 additions & 0 deletions test/Client/Adapter/CurlTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

declare(strict_types=1);

namespace LaminasTest\Http\Client\Adapter;

use Laminas\Http\Client\Adapter\Curl;
use Laminas\Uri\Uri;
use PHPUnit\Framework\TestCase;

use function curl_getinfo;

use const CURL_HTTP_VERSION_1_0;
use const CURL_HTTP_VERSION_1_1;
use const CURLINFO_HTTP_VERSION;

final class CurlTest extends TestCase
{
/** @var Curl */
private $adapter;

protected function setUp(): void
{
parent::setUp();
$this->adapter = new Curl();
}

/**
* @return iterable<non-empty-string,array{0:CURL_HTTP_VERSION_*,1:float}>
*/
public function floatHttpVersions(): iterable
{
yield 'HTTP 1.0' => [CURL_HTTP_VERSION_1_0, 1.0];
yield 'HTTP 1.1' => [CURL_HTTP_VERSION_1_1, 1.1];
}

/**
* @return iterable<non-empty-string,array{0:CURL_HTTP_VERSION_*,1:float}>
*/
public function httpVersions(): iterable
{
yield 'HTTP 1.0' => [CURL_HTTP_VERSION_1_0, '1.0'];
yield 'HTTP 1.1' => [CURL_HTTP_VERSION_1_1, '1.1'];
}

/**
* NOTE: This test is only needed for BC compatibility. The {@see \Laminas\Http\Client\Adapter\AdapterInterface}
* has a default for "string" but "float" was used in {@see Curl::write()} due to the lack of strict types.
*
* @dataProvider floatHttpVersions
*/
public function testWriteCanHandleFloatHttpVersion(int $expectedCurlOption, float $version): void
{
$this->adapter->connect('example.org');
$this->adapter->write('GET', new Uri('http://example.org:80/'), $version);
$handle = $this->adapter->getHandle();
self::assertNotNull($handle);
self::assertEquals($expectedCurlOption, curl_getinfo($handle, CURLINFO_HTTP_VERSION));
}

/**
* @dataProvider httpVersions
*/
public function testWriteCanHandleStringHttpVersion(int $expectedCurlOption, string $version): void
{
$this->adapter->connect('example.org');
$this->adapter->write('GET', new Uri('http://example.org:80/'), $version);
$handle = $this->adapter->getHandle();
self::assertNotNull($handle);
self::assertEquals($expectedCurlOption, curl_getinfo($handle, CURLINFO_HTTP_VERSION));
}
}
4 changes: 0 additions & 4 deletions test/Client/CurlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use function base64_encode;
use function curl_getinfo;
use function explode;
use function extension_loaded;
use function file_get_contents;
use function filesize;
use function fopen;
Expand Down Expand Up @@ -71,9 +70,6 @@ class CurlTest extends CommonHttpTests

protected function setUp(): void
{
if (! extension_loaded('curl')) {
$this->markTestSkipped('cURL is not installed, marking all Http Client Curl Adapter tests skipped.');
}
parent::setUp();
}

Expand Down

0 comments on commit 261f079

Please sign in to comment.