Skip to content

Commit

Permalink
Assert that cURL handles are correctly closed
Browse files Browse the repository at this point in the history
  • Loading branch information
schlessera committed Nov 10, 2021
1 parent 43b291e commit 3d91340
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/Transport/Curl.php
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,9 @@ public function request($url, $headers = array(), $data = array(), $options = ar
$response = $this->response_data;
}

if (true === $options['blocking']) {
if (! isset($options['blocking']) || $options['blocking'] !== false) {
// Need to remove the $this reference from the curl handle.
// Otherwise \WpOrg\Requests\Transport\Curl wont be garbage collected and the curl_close() will never be called.
// Otherwise \WpOrg\Requests\Transport\Curl won't be garbage collected and the curl_close() will never be called.
curl_setopt($this->handle, CURLOPT_HEADERFUNCTION, null);
curl_setopt($this->handle, CURLOPT_WRITEFUNCTION, null);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Transport/BaseTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ abstract class BaseTestCase extends TestCase {

protected $skip_https = false;

public function set_up() {
protected function set_up() {
// Intermediary variable $test_method. This can be simplified (removed) once the minimum supported PHP version is 7.0 or higher.
$test_method = array($this->transport, 'test');

Expand Down
82 changes: 82 additions & 0 deletions tests/Transport/CurlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,63 @@
namespace WpOrg\Requests\Tests\Transport;

use WpOrg\Requests\Exception;
use WpOrg\Requests\Hooks;
use WpOrg\Requests\Requests;
use WpOrg\Requests\Tests\Transport\BaseTestCase;
use WpOrg\Requests\Transport\Curl;

final class CurlTest extends BaseTestCase {
protected $transport = Curl::class;

/**
* Temporary storage of the cURL handle to assert against.
*
* @var null|resource|\CurlHandle
*/
protected $curl_handle;

/**
* Get the options to use for the cURL tests.
*
* This adds a hook on curl.before_request to store the cURL handle. This is
* needed for asserting after the test scenarios that the cURL handle was
* correctly closed.
*
* @param array $other
* @return array
*/
protected function getOptions($other = array()) {
$options = parent::getOptions($other);

$this->curl_handle = null;

$hooks = new Hooks();
$hooks->register(
'curl.before_request',
function ($handle) {
$this->curl_handle = $handle;
}
);

$options['hooks'] = $hooks;

return $options;
}

/**
* Post condition asserts to run after each scenario.
*
* This is used for asserting that cURL handles are not leaking memory.
*/
protected function assert_post_conditions( ) {
if ($this->curl_handle === null) {
// No cURL handle was used during this particular test scenario.
return;
}

$this->assertCurlHandleIsClosed($this->curl_handle);
}

public function testBadIP() {
$this->expectException(Exception::class);
$this->expectExceptionMessage('t resolve host');
Expand Down Expand Up @@ -136,4 +186,36 @@ public function testSetsEmptyExpectHeaderIfBodySmallerThan1Mb() {

$this->assertFalse(isset($result['headers']['Expect']));
}

/**
* Assert that a provided cURL handle was properly closed.
*
* For PHP 8.0+, the cURL handle is not a resource that needs to be closed,
* but rather a \CurlHandle object. The assertion just skips these.
*
* @param resource|\CurlHandle $handle CURL handle to check.
*/
private function assertCurlHandleIsClosed($handle) {
if ($handle instanceof \CurlHandle) {
// CURL handles have been changed from resources into CurlHandle
// objects starting with PHP 8.0, which don;t need to be closed.
return;
}

self::assertThat(
in_array(
gettype($handle),
array('resource', 'resource (closed)'),
true
),
self::isTrue(),
'Failed asserting that stored cURL handle was a resource.'
);

self::assertThat(
is_resource($handle) === false,
self::isTrue(),
'Failed asserting that stored cURL handle was properly closed.'
);
}
}

0 comments on commit 3d91340

Please sign in to comment.