[GH-ISSUE #2] CurlClient fail to exec HTTPS with empty HTTPOptions #2

Closed
opened 2026-02-28 14:22:45 +03:00 by kerem · 5 comments
Owner

Originally created by @eclipxe13 on GitHub (Mar 22, 2019).
Original GitHub issue: https://github.com/chillerlan/php-httpinterface/issues/2

Hello, thanks for your great work!

Was trying to use CurlClient without any (default) HTTPOptions but it fail with a curl error:

chillerlan\HTTP\Psr18\RequestException : error setting certificate verify locations:
  CAfile: 
  CApath: /etc/ssl/certs

wich is weird because making a plain curl connection didn't fail:

    public function testCurlIsWorking()
    {
        $curl = curl_init();
        curl_setopt($curl, \CURLOPT_URL, 'https://example.com/');
        curl_setopt($curl, \CURLOPT_SSL_VERIFYHOST, 2);
        curl_setopt($curl, \CURLOPT_SSL_VERIFYPEER, true);

        \curl_exec($curl);

        $errno = \curl_errno($curl);
        if ($errno !== \CURLE_OK) {
            $error = \curl_error($curl);
            $this->fail($error);
        }
        $this->assertTrue(true, 'test complete');
    }

So I take a look into your package and I see that in CurlHandle::init() you are setting $this->options->curl_options[\CURLOPT_CAINFO] with a NULL value.

If you modify the test and add this line will fail in the same way.

curl_setopt($curl, \CURLOPT_CAINFO, null);

I think that the problem is in CurlHandle::initCurlOptions() that returns an array with CURLOPT_CAINFO => $this->options->ca_info even when $this->options->ca_info === NULL. I tested including CURLOPT_CAINFO only when $this->options->ca_info is not falsy and it work just fine.

By the way, the same would happens with CURLOPT_CAPATH if you provide a way to work with.

Looks like CURLOPT_CAINFO or CURLOPT_CAPATH cannot be removed using null or empty strings, and it will fail. Also, if both are set then both must succeed.

Could you consider to fix this? Do you want me to make a PR? I'm not familiar with your test suite. The only this I was thinking is to simply remove 'ca_info' => __DIR__.'/../cacert.pem' from the initiation parameters on CurlClientTest.php.

Originally created by @eclipxe13 on GitHub (Mar 22, 2019). Original GitHub issue: https://github.com/chillerlan/php-httpinterface/issues/2 Hello, thanks for your great work! Was trying to use `CurlClient` without any (default) `HTTPOptions` but it fail with a curl error: ``` chillerlan\HTTP\Psr18\RequestException : error setting certificate verify locations: CAfile: CApath: /etc/ssl/certs ``` wich is weird because making a plain curl connection didn't fail: ```php public function testCurlIsWorking() { $curl = curl_init(); curl_setopt($curl, \CURLOPT_URL, 'https://example.com/'); curl_setopt($curl, \CURLOPT_SSL_VERIFYHOST, 2); curl_setopt($curl, \CURLOPT_SSL_VERIFYPEER, true); \curl_exec($curl); $errno = \curl_errno($curl); if ($errno !== \CURLE_OK) { $error = \curl_error($curl); $this->fail($error); } $this->assertTrue(true, 'test complete'); } ``` So I take a look into your package and I see that in `CurlHandle::init()` you are setting `$this->options->curl_options[\CURLOPT_CAINFO]` with a `NULL` value. If you modify the test and add this line will fail in the same way. ``` curl_setopt($curl, \CURLOPT_CAINFO, null); ``` I think that the problem is in `CurlHandle::initCurlOptions()` that returns an array with `CURLOPT_CAINFO => $this->options->ca_info` even when `$this->options->ca_info === NULL`. I tested including `CURLOPT_CAINFO` only when `$this->options->ca_info` is not *falsy* and it work just fine. By the way, the same would happens with `CURLOPT_CAPATH` if you provide a way to work with. Looks like `CURLOPT_CAINFO` or `CURLOPT_CAPATH` cannot be removed using null or empty strings, and it will fail. Also, if both are set then both must succeed. Could you consider to fix this? Do you want me to make a PR? I'm not familiar with your test suite. The only this I was thinking is to simply remove `'ca_info' => __DIR__.'/../cacert.pem'` from the initiation parameters on `CurlClientTest.php`.
kerem closed this issue 2026-02-28 14:22:46 +03:00
Author
Owner

@eclipxe13 commented on GitHub (Mar 22, 2019):

I just did #3 that also covers StreamClient with the same issue.

<!-- gh-comment-id:475482776 --> @eclipxe13 commented on GitHub (Mar 22, 2019): I just did #3 that also covers `StreamClient` with the same issue.
Author
Owner

@codemasher commented on GitHub (Mar 22, 2019):

Hey, thank you for the heads-up! I think #3 is a bit too invasive. The concept is to decouple the "settings" logic as far as possible from the application logic. So the whole process of determining the CA info went into HTTPOptions over here (see also #1): github.com/chillerlan/php-httpinterface@a7805699cd/src/HTTPOptionsTrait.php (L83-L189)
The cURL options are applied here before firing the request:
github.com/chillerlan/php-httpinterface@a7805699cd/src/Psr18/CurlHandle.php (L249-L250)
So if there's a problem finding the correct CA on your system, it might be that i'm probably missing a default path or something. The tests currently don't include much testing for default paths. So if you're looking to fix that, the best would be in HTTPOptionsTrait in first place. Also, i should point out, that the StreamClient is in "experimental" status at best.

<!-- gh-comment-id:475711771 --> @codemasher commented on GitHub (Mar 22, 2019): Hey, thank you for the heads-up! I think #3 is a bit too invasive. The concept is to decouple the "settings" logic as far as possible from the application logic. So the whole process of determining the CA info went into `HTTPOptions` over here (see also #1): https://github.com/chillerlan/php-httpinterface/blob/a7805699cd50dd0c34deb6a5066beba20c2ba708/src/HTTPOptionsTrait.php#L83-L189 The cURL options are applied here before firing the request: https://github.com/chillerlan/php-httpinterface/blob/a7805699cd50dd0c34deb6a5066beba20c2ba708/src/Psr18/CurlHandle.php#L249-L250 So if there's a problem finding the correct CA on your system, it might be that i'm probably missing a default path or something. [The tests](https://github.com/chillerlan/php-httpinterface/blob/a7805699cd50dd0c34deb6a5066beba20c2ba708/tests/HTTPOptionsTest.php) currently don't include much testing for default paths. So if you're looking to fix that, the best would be in `HTTPOptionsTrait` in first place. Also, i should point out, that the `StreamClient` is in "experimental" status at best.
Author
Owner

@eclipxe13 commented on GitHub (Mar 22, 2019):

As you mention, on php-httpinterface/src/Psr18/CurlHandle.php:250 is where a null value for CURLOPT_CAINFO is set.

My first thought was to check if CURLOPT_CAINFO exists (not with isset, but using array_key_exists), and if it is falsy then remove it from the options to apply.

Then I think that would be better to check the place where this is set and found that this null value comes from CurlHandle::initCurlOptions not include CURLOPT_CAINFO.

Anyhow, please, play around with the test I post in the issue.
It is not correct to set CURLOPT_CAINFO or CURLOPT_CAPATH to null, false or "" empty strings.

<!-- gh-comment-id:475720079 --> @eclipxe13 commented on GitHub (Mar 22, 2019): As you mention, on `php-httpinterface/src/Psr18/CurlHandle.php:250` is where a `null` value for `CURLOPT_CAINFO` is set. My first thought was to check if `CURLOPT_CAINFO` exists (not with `isset`, but using `array_key_exists`), and if it is *falsy* then remove it from the options to apply. Then I think that would be better to check the place where this is set and found that this `null` value comes from `CurlHandle::initCurlOptions` not include `CURLOPT_CAINFO`. Anyhow, please, play around with the test I post in the issue. It is not correct to set CURLOPT_CAINFO or CURLOPT_CAPATH to `null`, `false` or `""` empty strings.
Author
Owner

@codemasher commented on GitHub (Mar 22, 2019):

Ok, i investigated a bit further and found an error on my end over here: github.com/chillerlan/php-httpinterface@a7805699cd/src/Psr18/CurlHandle.php (L249-L250)
The PHP documentation says:

The + operator returns the right-hand array appended to the left-hand array; for keys that exist in both arrays, the elements from the left-hand array will be used, and the matching elements from the right-hand array will be ignored.

So it would be simply:

curl_setopt_array($this->curl, $this->options->curl_options + $options);

To circumvent any future bugs, preserve the numerical array keys (array_merge() doesn't) and dump the + operator i just changed it to a quick loop.
github.com/chillerlan/php-httpinterface@5afc32711d/src/Psr18/CurlHandle.php (L249-L252)

As for the fix in StreamClient, i've added this line to make sure a CA is set in the options: github.com/chillerlan/php-httpinterface@c28ada36a5/src/HTTPOptionsTrait.php (L174)

Anything else should throw an error now. Please let me know if this helps.

<!-- gh-comment-id:475806384 --> @codemasher commented on GitHub (Mar 22, 2019): Ok, i investigated a bit further and found an error on my end over here: https://github.com/chillerlan/php-httpinterface/blob/a7805699cd50dd0c34deb6a5066beba20c2ba708/src/Psr18/CurlHandle.php#L249-L250 The [PHP documentation](https://secure.php.net/manual/language.operators.array.php) says: > The + operator returns the right-hand array appended to the left-hand array; for keys that exist in both arrays, the elements from the left-hand array will be used, and the matching elements from the right-hand array will be ignored. So it would be simply: ```php curl_setopt_array($this->curl, $this->options->curl_options + $options); ``` To circumvent any future bugs, preserve the numerical array keys (`array_merge()` doesn't) and dump the `+` operator i just changed it to a quick loop. https://github.com/chillerlan/php-httpinterface/blob/5afc32711d90a3aea62e419394a7995aca91c5fa/src/Psr18/CurlHandle.php#L249-L252 As for the fix in `StreamClient`, i've added this line to make sure a CA is set in the options: https://github.com/chillerlan/php-httpinterface/blob/c28ada36a5bb062a4374329327825ec84ca0443e/src/HTTPOptionsTrait.php#L174 Anything else should throw an error now. Please let me know if this helps.
Author
Owner

@eclipxe13 commented on GitHub (Mar 23, 2019):

Thank you.

<!-- gh-comment-id:475827213 --> @eclipxe13 commented on GitHub (Mar 23, 2019): Thank you.
Sign in to join this conversation.
No labels
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/php-httpinterface#2
No description provided.