Skip to content

Commit 38d7eb1

Browse files
ElGigiElGigi
committed
Missing curl_close() call
Missing `CURLOPT_NOSIGNAL` option to TRUE to prevent timeout Fix `CurlAdapter::clearOptions()` method
1 parent 5ed212d commit 38d7eb1

File tree

3 files changed

+132
-74
lines changed

3 files changed

+132
-74
lines changed

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. This projec
44
to [Semantic Versioning] (http://semver.org/). For change log format,
55
use [Keep a Changelog] (http://keepachangelog.com/).
66

7+
## [2.4.1] - 2025-06-17
8+
9+
### Fixed
10+
11+
- Missing `curl_close()` call
12+
- Missing `CURLOPT_NOSIGNAL` option to TRUE to prevent timeout
13+
- `CurlAdapter::clearOptions()`
14+
715
## [2.4.0] - 2025-05-12
816

917
### Added

‎src/Adapter/CurlAdapter.php‎

Lines changed: 85 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -67,38 +67,42 @@ public function getName(): string
6767
/**
6868
* Clear CURL options.
6969
*
70-
* Warning: you can't specify some CURL options :
70+
* Warning: you can't specify some CURL options:
71+
* - CURLINFO_HEADER_OUT
7172
* - CURLOPT_HTTP_VERSION
7273
* - CURLOPT_CUSTOMREQUEST
73-
* - CURLOPT_URL
74+
* - CURLOPT_FOLLOWLOCATION
7475
* - CURLOPT_HEADER
75-
* - CURLINFO_HEADER_OUT
76+
* - CURLOPT_HEADERFUNCTION
7677
* - CURLOPT_HTTPHEADER
77-
* - CURLOPT_FOLLOWLOCATION
78+
* - CURLOPT_NOSIGNAL
7879
* - CURLOPT_RETURNTRANSFER
7980
* - CURLOPT_POST
8081
* - CURLOPT_POSTFIELDS
82+
* - CURLOPT_URL
83+
* - CURLOPT_WRITEFUNCTION
8184
* They are reserved for good work of service.
8285
*/
8386
protected function clearOptions(): void
8487
{
8588
// Remove reserved CURL options
8689
$reservedOptions = [
90+
CURLINFO_HEADER_OUT,
8791
CURLOPT_HTTP_VERSION,
8892
CURLOPT_CUSTOMREQUEST,
89-
CURLOPT_URL,
93+
CURLOPT_FOLLOWLOCATION,
9094
CURLOPT_HEADER,
9195
CURLOPT_HEADERFUNCTION,
92-
CURLINFO_HEADER_OUT,
9396
CURLOPT_HTTPHEADER,
97+
CURLOPT_NOSIGNAL,
9498
CURLOPT_RETURNTRANSFER,
9599
CURLOPT_POST,
96100
CURLOPT_POSTFIELDS,
97-
CURLOPT_FOLLOWLOCATION,
101+
CURLOPT_URL,
98102
CURLOPT_WRITEFUNCTION,
99103
];
100104

101-
$this->options = array_diff($this->options, $reservedOptions);
105+
$this->options = array_diff_key($this->options, array_fill_keys($reservedOptions, null));
102106
}
103107

104108
/**
@@ -123,78 +127,82 @@ public function sendRequest(RequestInterface $request, ?HttpContext $context = n
123127
$context,
124128
);
125129

126-
// Execute CURL request
127-
$dateTime = new DateTimeImmutable();
128-
curl_exec($ch);
130+
try {
131+
// Execute CURL request
132+
$dateTime = new DateTimeImmutable();
133+
curl_exec($ch);
129134

130-
// CURL error?
131-
switch (curl_errno($ch)) {
132-
case CURLE_OK:
133-
break;
134-
case CURLE_URL_MALFORMAT:
135-
case CURLE_URL_MALFORMAT_USER:
136-
case CURLE_MALFORMAT_USER:
137-
case CURLE_BAD_PASSWORD_ENTERED:
138-
throw new RequestException(
139-
sprintf(
140-
'CURL error: %s (%s)',
141-
curl_error($ch),
142-
$request->getUri()
143-
),
144-
$request
145-
);
146-
default:
147-
throw new NetworkException(
148-
sprintf(
149-
'CURL error: %s (%s)',
150-
curl_error($ch),
151-
$request->getUri()
152-
),
153-
$request
154-
);
155-
}
135+
// CURL error?
136+
switch (curl_errno($ch)) {
137+
case CURLE_OK:
138+
break;
139+
case CURLE_URL_MALFORMAT:
140+
case CURLE_URL_MALFORMAT_USER:
141+
case CURLE_MALFORMAT_USER:
142+
case CURLE_BAD_PASSWORD_ENTERED:
143+
throw new RequestException(
144+
sprintf(
145+
'CURL error: %s (%s)',
146+
curl_error($ch),
147+
$request->getUri()
148+
),
149+
$request
150+
);
151+
default:
152+
throw new NetworkException(
153+
sprintf(
154+
'CURL error: %s (%s)',
155+
curl_error($ch),
156+
$request->getUri()
157+
),
158+
$request
159+
);
160+
}
156161

157-
// Timings
158-
$this->timings = new Timings(
159-
dateTime: $dateTime,
160-
send: (float)((curl_getinfo($ch, CURLINFO_PRETRANSFER_TIME_T)
161-
- curl_getinfo($ch, CURLINFO_APPCONNECT_TIME_T)) / 1000),
162-
wait: (float)((curl_getinfo($ch, CURLINFO_STARTTRANSFER_TIME_T)
163-
- curl_getinfo($ch, CURLINFO_PRETRANSFER_TIME_T)) / 1000),
164-
receive: (float)((curl_getinfo($ch, CURLINFO_TOTAL_TIME_T)
165-
- curl_getinfo($ch, CURLINFO_STARTTRANSFER_TIME_T)) / 1000),
166-
total: (float)(curl_getinfo($ch, CURLINFO_TOTAL_TIME_T) / 1000),
167-
blocked: -1,
168-
dns: (float)(curl_getinfo($ch, CURLINFO_NAMELOOKUP_TIME_T) / 1000),
169-
connect: (float)((curl_getinfo($ch, CURLINFO_CONNECT_TIME_T)
170-
- curl_getinfo($ch, CURLINFO_NAMELOOKUP_TIME_T)) / 1000),
171-
ssl: (float)((curl_getinfo($ch, CURLINFO_APPCONNECT_TIME_T)
172-
- curl_getinfo($ch, CURLINFO_CONNECT_TIME_T)) / 1000),
173-
);
162+
// Timings
163+
$this->timings = new Timings(
164+
dateTime: $dateTime,
165+
send: (float)((curl_getinfo($ch, CURLINFO_PRETRANSFER_TIME_T)
166+
- curl_getinfo($ch, CURLINFO_APPCONNECT_TIME_T)) / 1000),
167+
wait: (float)((curl_getinfo($ch, CURLINFO_STARTTRANSFER_TIME_T)
168+
- curl_getinfo($ch, CURLINFO_PRETRANSFER_TIME_T)) / 1000),
169+
receive: (float)((curl_getinfo($ch, CURLINFO_TOTAL_TIME_T)
170+
- curl_getinfo($ch, CURLINFO_STARTTRANSFER_TIME_T)) / 1000),
171+
total: (float)(curl_getinfo($ch, CURLINFO_TOTAL_TIME_T) / 1000),
172+
blocked: -1,
173+
dns: (float)(curl_getinfo($ch, CURLINFO_NAMELOOKUP_TIME_T) / 1000),
174+
connect: (float)((curl_getinfo($ch, CURLINFO_CONNECT_TIME_T)
175+
- curl_getinfo($ch, CURLINFO_NAMELOOKUP_TIME_T)) / 1000),
176+
ssl: (float)((curl_getinfo($ch, CURLINFO_APPCONNECT_TIME_T)
177+
- curl_getinfo($ch, CURLINFO_CONNECT_TIME_T)) / 1000),
178+
);
174179

175-
// Response
176-
$protocolVersion = $reasonPhrase = null;
177-
$bodyStream->seek(0);
178-
$headers = $this->parseHeaders(
179-
$headersStream->getContents(),
180-
protocolVersion: $protocolVersion,
181-
reasonPhrase: $reasonPhrase
182-
);
180+
// Response
181+
$protocolVersion = $reasonPhrase = null;
182+
$bodyStream->seek(0);
183+
$headers = $this->parseHeaders(
184+
$headersStream->getContents(),
185+
protocolVersion: $protocolVersion,
186+
reasonPhrase: $reasonPhrase
187+
);
183188

184-
// Replace location header with redirect_url parameter
185-
if (!empty($redirectUrl = curl_getinfo($ch, CURLINFO_REDIRECT_URL))) {
186-
$headers['Location'] = [$redirectUrl];
187-
}
189+
// Replace location header with redirect_url parameter
190+
if (!empty($redirectUrl = curl_getinfo($ch, CURLINFO_REDIRECT_URL))) {
191+
$headers['Location'] = [$redirectUrl];
192+
}
188193

189-
// Create response
190-
$response = new Response(
191-
$this->createStream($bodyStream, $headers['Content-Encoding'] ?? null),
192-
curl_getinfo($ch, CURLINFO_RESPONSE_CODE),
193-
$headers,
194-
$reasonPhrase ?? ''
195-
);
194+
// Create response
195+
$response = new Response(
196+
$this->createStream($bodyStream, $headers['Content-Encoding'] ?? null),
197+
curl_getinfo($ch, CURLINFO_RESPONSE_CODE),
198+
$headers,
199+
$reasonPhrase ?? ''
200+
);
196201

197-
return $response->withProtocolVersion($protocolVersion);
202+
return $response->withProtocolVersion($protocolVersion);
203+
} finally {
204+
curl_close($ch);
205+
}
198206
}
199207

200208
/**
@@ -232,6 +240,9 @@ protected function initCurl(
232240
$curlOpts[CURLOPT_CUSTOMREQUEST] = $request->getMethod();
233241
$curlOpts[CURLOPT_URL] = $request->getUri();
234242

243+
// Timeouts
244+
$curlOpts[CURLOPT_NOSIGNAL] = true;
245+
235246
// Headers
236247
{
237248
$curlOpts[CURLOPT_HEADER] = false;

‎tests/Adapter/CurlAdapterTest.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
/*
3+
* This file is part of Berlioz framework.
4+
*
5+
* @license https://opensource.org/licenses/MIT MIT License
6+
* @copyright 2025 Ronan GIRON
7+
* @author Ronan GIRON <https://github.com/ElGigi>
8+
*
9+
* For the full copyright and license information, please view the LICENSE
10+
* file that was distributed with this source code, to the root.
11+
*/
12+
13+
namespace Berlioz\Http\Client\Tests\Adapter;
14+
15+
use Berlioz\Http\Client\Adapter\CurlAdapter;
16+
use PHPUnit\Framework\TestCase;
17+
use ReflectionClass;
18+
19+
class CurlAdapterTest extends TestCase
20+
{
21+
public function testOptions()
22+
{
23+
$adapter = new CurlAdapter([
24+
CURLOPT_CONNECTTIMEOUT => 10,
25+
CURLOPT_TIMEOUT => 20,
26+
CURLOPT_URL => 'https://getberlioz.com/',
27+
]);
28+
$reflection = new ReflectionClass($adapter);
29+
$adapterOptions = $reflection->getProperty('options')->getValue($adapter);
30+
31+
$this->assertSame(
32+
[
33+
CURLOPT_CONNECTTIMEOUT => 10,
34+
CURLOPT_TIMEOUT => 20,
35+
],
36+
$adapterOptions
37+
);
38+
}
39+
}

0 commit comments

Comments
 (0)