diff --git a/src/Transport/Curl.php b/src/Transport/Curl.php index 5547ec670..aba1bf5da 100644 --- a/src/Transport/Curl.php +++ b/src/Transport/Curl.php @@ -201,9 +201,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); } diff --git a/tests/Transport/BaseTestCase.php b/tests/Transport/BaseTestCase.php index c9f923f88..8868efe04 100644 --- a/tests/Transport/BaseTestCase.php +++ b/tests/Transport/BaseTestCase.php @@ -16,7 +16,7 @@ abstract class BaseTestCase extends TestCase { protected $skip_https = false; - public function set_up() { + protected function set_up() { $callback = array($this->transport, 'test'); $supported = call_user_func($callback); diff --git a/tests/Transport/CurlTest.php b/tests/Transport/CurlTest.php index 4f5ace51f..c5f811280 100644 --- a/tests/Transport/CurlTest.php +++ b/tests/Transport/CurlTest.php @@ -3,6 +3,7 @@ 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; @@ -10,6 +11,55 @@ 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'); @@ -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.' + ); + } }