Merge pull request #49 from php-http/options-handling

use options resolver to handle curl options and report problems

Author: joelwurtz

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Allow cURL options to overwrite our default spec-compliant default configuration
 
 ## 1.7.1 - 2018-03-36
 

composer.json

@@ -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",

src/Client.php

@@ -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.
@@ -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}
@@ -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);
     }
 
     /**
@@ -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);
@@ -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);
@@ -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
@@ -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());

tests/ClientTest.php

@@ -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;
 
 /**
@@ -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.
      */