Skip to content

Commit 2f253f9

Browse files
authored
Fix how value of cURL's CURLOPT_SSL_VERIFYHOST is determined (#46)
* Reducing code duplication by utilising an existing method for setting cURL options in JiraClient. * Adding and using a new method to determine the actual value to use when setting 'CURLOPT_SSL_VERIFYHOST' to fix PHP notices about trying to set it to an invalid value.
1 parent 0b4c489 commit 2f253f9

File tree

4 files changed

+112
-21
lines changed

4 files changed

+112
-21
lines changed

src/Configuration/AbstractConfiguration.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,15 @@ public function getCurlOptUserAgent(): ?string
161161
return $this->curlOptUserAgent;
162162
}
163163

164+
public function getCurlOptSslVerifyHostValue(): int
165+
{
166+
return [
167+
false => 0,
168+
// See https://www.php.net/manual/en/function.curl-setopt.php for information why 2 needs to be here.
169+
true => 2,
170+
][$this->isCurlOptSslVerifyHost()];
171+
}
172+
164173
public function getOAuthAccessToken(): string
165174
{
166175
return $this->oauthAccessToken;

src/Configuration/ConfigurationInterface.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ public function isCurlOptVerbose(): bool;
6565
*/
6666
public function getCurlOptUserAgent(): ?string;
6767

68+
/**
69+
* Get the actual value for Curl's CURLOPT_SSL_VERIFYHOST.
70+
*/
71+
public function getCurlOptSslVerifyHostValue(): int;
72+
6873
/**
6974
* HTTP header 'Authorization: Bearer {token}' for OAuth.
7075
*/

src/JiraClient.php

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public function curlPrepare(\CurlHandle|bool $ch, array $curl_http_headers, ?str
132132
{
133133
$this->authorization($ch, $curl_http_headers, $cookieFile);
134134

135-
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, $this->getConfiguration()->isCurlOptSslVerifyHost());
135+
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, $this->getConfiguration()->getCurlOptSslVerifyHostValue());
136136
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, $this->getConfiguration()->isCurlOptSslVerifyPeer());
137137
if ($this->getConfiguration()->isCurlOptSslCert()) {
138138
curl_setopt($ch, CURLOPT_SSLCERT, $this->getConfiguration()->isCurlOptSslCert());
@@ -327,26 +327,7 @@ private function createUploadHandle(string $url, string $upload_file, \CurlHandl
327327
$this->log->debug('using CURLFile='.var_export($attachments, true));
328328
}
329329

330-
$this->authorization($ch, $curl_http_headers);
331-
332-
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, $this->getConfiguration()->isCurlOptSslVerifyHost());
333-
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, $this->getConfiguration()->isCurlOptSslVerifyPeer());
334-
335-
if ($this->getConfiguration()->isCurlOptSslCert()) {
336-
curl_setopt($ch, CURLOPT_SSLCERT, $this->getConfiguration()->isCurlOptSslCert());
337-
}
338-
if ($this->getConfiguration()->isCurlOptSslCertPassword()) {
339-
curl_setopt($ch, CURLOPT_SSLCERTPASSWD, $this->getConfiguration()->isCurlOptSslCertPassword());
340-
}
341-
if ($this->getConfiguration()->isCurlOptSslKey()) {
342-
curl_setopt($ch, CURLOPT_SSLKEY, $this->getConfiguration()->isCurlOptSslKey());
343-
}
344-
if ($this->getConfiguration()->isCurlOptSslKeyPassword()) {
345-
curl_setopt($ch, CURLOPT_SSLKEYPASSWD, $this->getConfiguration()->isCurlOptSslKeyPassword());
346-
}
347-
if ($this->getConfiguration()->getTimeout()) {
348-
curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, $this->getConfiguration()->getTimeout());
349-
}
330+
$curl_http_headers = $this->curlPrepare($ch, $curl_http_headers);
350331

351332
$this->proxyConfigCurlHandle($ch);
352333

tests/Curl/SslVerifyHostTest.php

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php
2+
3+
namespace JiraCloud\Test\Curl;
4+
5+
use JiraCloud\Configuration\{
6+
AbstractConfiguration,
7+
ConfigurationInterface,
8+
};
9+
use PHPUnit\Framework\TestCase;
10+
11+
class SslVerifyHostTest extends TestCase
12+
{
13+
/**
14+
* @dataProvider sslVerifyHostGetterDataSource
15+
*/
16+
public function testSslVerifyHostGetter(bool $curlOptSslVerifyHost, int $expectedResult): void
17+
{
18+
$this->assertEquals(
19+
$expectedResult,
20+
$this->getTestedInstance($curlOptSslVerifyHost)->getCurlOptSslVerifyHostValue()
21+
);
22+
}
23+
24+
public function sslVerifyHostGetterDataSource(): array
25+
{
26+
return [
27+
'turned off' => [
28+
'curlOptSslVerifyHost' => false,
29+
'expectedResult' => 0,
30+
],
31+
'turned on' => [
32+
'curlOptSslVerifyHost' => true,
33+
'expectedResult' => 2,
34+
],
35+
];
36+
}
37+
38+
/**
39+
* @dataProvider setterSanityDataSource
40+
*/
41+
public function testSetterSanity(int $curlOptSslVerifyHostValue, bool $expectError): void
42+
{
43+
$errorTriggered = false;
44+
45+
$noticeHandler = function (
46+
int $number,
47+
string $message,
48+
string $file,
49+
int $line
50+
) use (&$errorTriggered): bool {
51+
$errorTriggered = true;
52+
53+
return true;
54+
};
55+
56+
set_error_handler($noticeHandler, E_NOTICE);
57+
58+
// Note that the host isn't really important here.
59+
$curlResource = curl_init("https://localhost/");
60+
curl_setopt($curlResource, CURLOPT_SSL_VERIFYHOST, $curlOptSslVerifyHostValue);
61+
curl_close($curlResource);
62+
63+
$this->assertEquals($expectError, $errorTriggered);
64+
65+
restore_error_handler();
66+
}
67+
68+
public function setterSanityDataSource(): array
69+
{
70+
return [
71+
'turned off' => [
72+
'curlOptSslVerifyHostValue' => 0,
73+
'expectError' => false,
74+
],
75+
'turned on' => [
76+
'curlOptSslVerifyHostValue' => 2,
77+
'expectError' => false,
78+
],
79+
'using old "on" value' => [
80+
'curlOptSslVerifyHostValue' => 1,
81+
'expectError' => true,
82+
],
83+
];
84+
}
85+
86+
protected function getTestedInstance(bool $curlOptSslVerifyHost): ConfigurationInterface
87+
{
88+
return new class ($curlOptSslVerifyHost) extends AbstractConfiguration
89+
{
90+
public function __construct(bool $curlOptSslVerifyHost)
91+
{
92+
$this->curlOptSslVerifyHost = $curlOptSslVerifyHost;
93+
}
94+
};
95+
}
96+
}

0 commit comments

Comments
 (0)