Skip to content

Commit

Permalink
use options resolver to handle curl options and report problems
Browse files Browse the repository at this point in the history
  • Loading branch information
dbu committed Nov 29, 2018
1 parent 9f7923e commit 01f9c98
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- Allow cURL options to overwrite our default spec-compliant default configuration

## 1.7.1 - 2018-03-36

Expand Down
3 changes: 2 additions & 1 deletion composer.json
Expand Up @@ -21,7 +21,8 @@
"php-http/httplug": "^2.0",
"php-http/message-factory": "^1.0.2",
"php-http/message": "^1.2",
"php-http/discovery": "^1.0"
"php-http/discovery": "^1.0",
"symfony/options-resolver": "^3.4 || ^4.0"
},
"require-dev": {
"guzzlehttp/psr7": "^1.0",
Expand Down
34 changes: 21 additions & 13 deletions src/Client.php
Expand Up @@ -12,6 +12,7 @@
use Http\Promise\Promise;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

/**
* PSR-7 compatible cURL based HTTP client.
Expand Down Expand Up @@ -62,8 +63,6 @@ class Client implements HttpClient, HttpAsyncClient
private $multiRunner = null;

/**
* Create new client.
*
* @param MessageFactory|null $messageFactory HTTP Message factory
* @param StreamFactory|null $streamFactory HTTP Stream factory
* @param array $options cURL options {@link http://php.net/curl_setopt}
Expand All @@ -79,7 +78,20 @@ public function __construct(
) {
$this->messageFactory = $messageFactory ?: MessageFactoryDiscovery::find();
$this->streamFactory = $streamFactory ?: StreamFactoryDiscovery::find();
$this->options = $options;
$resolver = new OptionsResolver();
$resolver->setDefaults([
CURLOPT_HEADER => false,
CURLOPT_RETURNTRANSFER => false,
CURLOPT_FOLLOWLOCATION => false,
]);
$resolver->setAllowedValues(CURLOPT_HEADER, [false]); // our parsing will fail if this is set to true
$resolver->setAllowedValues(CURLOPT_RETURNTRANSFER, [false]); // our parsing will fail if this is set to true

// We do not know what everything curl supports and might support in the future.
// Make sure that we accept everything that is in the options.
$resolver->setDefined(array_keys($options));

$this->options = $resolver->resolve($options);
}

/**
Expand Down Expand Up @@ -111,15 +123,15 @@ public function __destruct()
public function sendRequest(RequestInterface $request): ResponseInterface
{
$responseBuilder = $this->createResponseBuilder();
$options = $this->createCurlOptions($request, $responseBuilder);
$requestOptions = $this->prepareRequestOptions($request, $responseBuilder);

if (is_resource($this->handle)) {
curl_reset($this->handle);
} else {
$this->handle = curl_init();
}

curl_setopt_array($this->handle, $options);
curl_setopt_array($this->handle, $requestOptions);
curl_exec($this->handle);

$errno = curl_errno($this->handle);
Expand Down Expand Up @@ -165,8 +177,8 @@ public function sendAsyncRequest(RequestInterface $request)

$handle = curl_init();
$responseBuilder = $this->createResponseBuilder();
$options = $this->createCurlOptions($request, $responseBuilder);
curl_setopt_array($handle, $options);
$requestOptions = $this->prepareRequestOptions($request, $responseBuilder);
curl_setopt_array($handle, $requestOptions);

$core = new PromiseCore($request, $handle, $responseBuilder);
$promise = new CurlPromise($core, $this->multiRunner);
Expand All @@ -176,7 +188,7 @@ public function sendAsyncRequest(RequestInterface $request)
}

/**
* Generates cURL options.
* Update cURL options for this request and hook in the response builder.
*
* @param RequestInterface $request
* @param ResponseBuilder $responseBuilder
Expand All @@ -187,14 +199,10 @@ public function sendAsyncRequest(RequestInterface $request)
*
* @return array
*/
private function createCurlOptions(RequestInterface $request, ResponseBuilder $responseBuilder)
private function prepareRequestOptions(RequestInterface $request, ResponseBuilder $responseBuilder)
{
$options = $this->options;

$options[CURLOPT_HEADER] = false;
$options[CURLOPT_RETURNTRANSFER] = false;
$options[CURLOPT_FOLLOWLOCATION] = false;

try {
$options[CURLOPT_HTTP_VERSION]
= $this->getProtocolVersion($request->getProtocolVersion());
Expand Down
15 changes: 15 additions & 0 deletions tests/ClientTest.php
Expand Up @@ -5,7 +5,10 @@
namespace Http\Client\Curl\Tests;

use Http\Client\Curl\Client;
use Http\Message\MessageFactory;
use Http\Message\StreamFactory;
use PHPUnit\Framework\TestCase;
use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
use Zend\Diactoros\Request;

/**
Expand Down Expand Up @@ -73,6 +76,18 @@ public function testRewindLargeStream()
static::assertTrue(false !== strstr($options[CURLOPT_READFUNCTION](null, null, $length), 'abcdef'), 'Steam was not rewinded');
}

public function testInvalidCurlOptions()
{
$this->expectException(InvalidOptionsException::class);
new Client(
$this->createMock(MessageFactory::class),
$this->createMock(StreamFactory::class),
[
CURLOPT_HEADER => true, // this won't work with our client
]
);
}

/**
* Discovery should be used if no factory given.
*/
Expand Down

0 comments on commit 01f9c98

Please sign in to comment.