From ed9dfc1bb4724cbc0f663568129d07e59af0272e Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Sat, 27 Apr 2019 15:30:11 -0300 Subject: [PATCH] Add some protection for `EasyHandle` --- src/Handler/CurlFactory.php | 4 +- src/Handler/EasyHandle.php | 119 ++++++++++++++------ tests/Handler/EasyHandleTest.php | 186 +++++++++++++++++++++++-------- 3 files changed, 230 insertions(+), 79 deletions(-) diff --git a/src/Handler/CurlFactory.php b/src/Handler/CurlFactory.php index 431ed78ba..b9012f17c 100644 --- a/src/Handler/CurlFactory.php +++ b/src/Handler/CurlFactory.php @@ -147,7 +147,7 @@ public static function finish(callable $handler, EasyHandle $easy, CurlFactoryIn self::invokeStats($easy); } - if (!$easy->response || $easy->errno) { + if (!$easy->response || \CURLE_OK !== $easy->errno) { return self::finishError($handler, $easy, $factory); } @@ -192,7 +192,7 @@ private static function finishError(callable $handler, EasyHandle $easy, CurlFac $factory->release($easy); // Retry when nothing is present or when curl failed to rewind. - if (empty($easy->options['_err_message']) && (!$easy->errno || $easy->errno == 65)) { + if (empty($easy->options['_err_message']) && (!$easy->errno || $easy->errno == /* \CURLE_SEND_FAIL_REWIND */ 65)) { return self::retryFailedRewind($handler, $easy, $ctx); } diff --git a/src/Handler/EasyHandle.php b/src/Handler/EasyHandle.php index 31b661217..09bf73286 100644 --- a/src/Handler/EasyHandle.php +++ b/src/Handler/EasyHandle.php @@ -11,6 +11,16 @@ /** * Represents a cURL easy handle and the data it populates. * + * @property resource|\CurlHandle $handle resource cURL resource + * @property StreamInterface $sink Where data is being written + * @property array $headers Received HTTP headers so far + * @property ResponseInterface|null $response Received response (if any) + * @property RequestInterface $request Request being sent + * @property array $options Request options + * @property int $errno int cURL error number + * @property \Throwable|null $onHeadersException Exception during on_headers (if any) + * @property \Throwable|null $createResponseException Exception during createResponse (if any) + * * @internal */ final class EasyHandle @@ -18,47 +28,52 @@ final class EasyHandle /** * @var resource|\CurlHandle cURL resource */ - public $handle; + private $handle; /** * @var StreamInterface Where data is being written */ - public $sink; + private $sink; /** * @var RequestInterface Request being sent */ - public $request; + private $request; /** * @var array Request options */ - public $options = []; + private $options = []; /** - * @var \Throwable|null Exception during on_headers (if any) + * @var int cURL error number (if any) */ - public $onHeadersException; + private $errno = \CURLE_OK; /** - * @var \Exception|null Exception during createResponse (if any) + * @var array Received HTTP headers so far */ - public $createResponseException; + private $headers = []; /** - * @var int cURL error number (if any) + * @var ResponseInterface|null Received response (if any) */ - private $errno = CURLE_OK; + private $response; /** - * @var array Received HTTP headers so far + * @var \Throwable|null Exception during on_headers (if any) */ - private $headers = []; + private $onHeadersException; /** - * @var ResponseInterface|null Received response (if any) + * @var \Throwable|null Exception during createResponse (if any) */ - private $response; + private $createResponseException; + + /** + * @var bool Tells if the EasyHandle has been initialized + */ + private $initialized = false; /** * Attach a response to the easy handle based on the received headers. @@ -98,42 +113,84 @@ public function createResponse(): void } /** - * @param string $name - * - * @return void + * @return mixed * * @throws \BadMethodCallException */ - public function &__get($name) + public function &__get(string $name) { - if (in_array($name, ['errno', 'headers', 'onHeadersException', 'response'], true)) { + if (('handle' !== $name && property_exists($this, $name)) || $this->initialized && isset($this->handle)) { return $this->{$name}; } $msg = $name === 'handle' - ? 'The EasyHandle has been released' + ? 'The EasyHandle '.($this->initialized ? 'has been released' : 'is not initialized') : sprintf('Undefined property: %s::$%s', __CLASS__, $name); throw new \BadMethodCallException($msg); } - public function __set($name, $value) + /** + * @param mixed $value + * + * @throws \UnexpectedValueException|\LogicException + */ + public function __set(string $name, $value): void { - if (in_array($name, ['errno', 'headers', 'onHeadersException', 'response'], true)) { - if ('response' === $name) { - // BC: Change to `\Error` when bumping PHP version to ^7.0 - throw new \LogicException(sprintf('Cannot set private property %s::$%s', __CLASS__, $name)); - } + if ($this->initialized && !isset($this->handle)) { + throw new \UnexpectedValueException('The EasyHandle has been released, please use a new EasyHandle instead.'); + } - if (!isset($this->handle) || !is_resource($this->handle) || 'curl' !== get_resource_type($this->handle)) { - throw new \LogicException(sprintf('Property %s::$%s could not be set when there isn\'t a valid handle', __CLASS__, $name)); - } + if (in_array($name, ['response', 'initialized'], true)) { + throw new \LogicException(sprintf('Cannot set private property %s::$%s.', __CLASS__, $name)); + } + + if (in_array($name, ['errno', 'handle', 'headers', 'onHeadersException', 'createResponseException'], true)) { + if ('handle' === $name) { + if (isset($this->handle)) { + throw new \UnexpectedValueException(sprintf('Property %s::$%s is already set, please use a new EasyHandle instead.', __CLASS__, $name)); + } + + if (\PHP_VERSION_ID >= 80000) { + if (!$value instanceof \CurlHandle) { + throw new \TypeError(sprintf('Property %s::$%s can only accept an object of type "%s".', __CLASS__, $name, \CurlHandle::class)); + } + } elseif (!is_resource($value) || 'curl' !== get_resource_type($value)) { + throw new \TypeError(sprintf('Property %s::$%s can only accept a resource of type "curl".', __CLASS__, $name)); + } + + $this->initialized = true; + } else { + if (!isset($this->handle) || !$this->handle instanceof \CurlHandle || !is_resource($this->handle) || 'curl' !== get_resource_type($this->handle)) { + throw new \UnexpectedValueException(sprintf('Property %s::$%s could not be set when there isn\'t a valid handle.', __CLASS__, $name)); + } - if ('errno' === $name && CURLE_OK !== ($handleErrno = curl_errno($this->handle)) && $value !== $handleErrno) { - throw new \LogicException(sprintf('Property %s::$errno could not be set with %u since the handle is reporting error %u', __CLASS__, $value, $handleErrno)); + if ('errno' === $name) { + if (!is_int($value)) { + throw new \TypeError(sprintf('Property %s::$errno can only accept a value of type int, %s given.', __CLASS__, gettype($value))); + } + + $handleErrno = curl_errno($this->handle); + + if (\CURLE_OK !== $handleErrno && $value !== $handleErrno) { + throw new \UnexpectedValueException(sprintf('Property %s::$errno could not be set with %u since the handle is reporting error %u.', __CLASS__, $value, $handleErrno)); + } + } } } $this->{$name} = $value; } + + /** + * @throws \LogicException + */ + public function __unset(string $name): void + { + if ('handle' !== $name) { + throw new \Error(sprintf('Cannot unset private property %s::$%s.', __CLASS__, $name)); + } + + unset($this->{$name}); + } } diff --git a/tests/Handler/EasyHandleTest.php b/tests/Handler/EasyHandleTest.php index 8932c3172..1d195179f 100644 --- a/tests/Handler/EasyHandleTest.php +++ b/tests/Handler/EasyHandleTest.php @@ -3,6 +3,7 @@ namespace GuzzleHttp\Test\Handler; use GuzzleHttp\Handler\EasyHandle; +use GuzzleHttp\Psr7; use PHPUnit\Framework\TestCase; /** @@ -13,112 +14,205 @@ class EasyHandleTest extends TestCase public function testEnsuresHandleExists() { $easy = new EasyHandle(); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('The EasyHandle is not initialized'); + + $easy->handle; + } + + public function testGetReleasedHandle(): void + { + $easy = new EasyHandle(); + $easy->handle = curl_init(); + curl_close($easy->handle); unset($easy->handle); $this->expectException(\BadMethodCallException::class); $this->expectExceptionMessage('The EasyHandle has been released'); + $easy->handle; } - public function testSettingProperties() + public function testGetPropertyAfterHandleRelease(): void + { + $easy = new EasyHandle(); + $easy->handle = curl_init(); + curl_close($easy->handle); + unset($easy->handle); + + self::assertSame([], $easy->options); + } + + /** + * @requires PHP < 8 + */ + public function testSettingBadHandlePrePhp8(): void + { + $easy = new EasyHandle(); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Property GuzzleHttp\Handler\EasyHandle::$handle can only accept a resource of type "curl"'); + + $easy->handle = null; + } + + /** + * @requires PHP >= 8 + */ + public function testSettingBadHandle(): void + { + $easy = new EasyHandle(); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Property GuzzleHttp\Handler\EasyHandle::$handle can only accept an object of type "CurlHandle"'); + + $easy->handle = null; + } + + public function testSettingPropertyOnReleasedHandle(): void + { + $easy = new EasyHandle(); + $easy->handle = curl_init(); + curl_close($easy->handle); + unset($easy->handle); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('The EasyHandle has been released, please use a new EasyHandle instead'); + + $easy->options = []; + } + + public function testSettingHandleTwice(): void + { + $easy = new EasyHandle(); + $easy->handle = curl_init(); + curl_close($easy->handle); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Property GuzzleHttp\Handler\EasyHandle::$handle is already set, please use a new EasyHandle instead'); + + $easy->handle = curl_init(); + } + + public function testSettingProperties(): void { $handle = curl_init(); $stream = new Psr7\Stream(fopen('php://temp', 'r')); $headers = []; $request = new Psr7\Request('HEAD', '/'); $options = []; - $easy = new EasyHandle; + $easy = new EasyHandle(); $easy->handle = $handle; $easy->sink = $stream; $easy->headers = $headers; $easy->request = $request; $easy->options = $options; - $this->assertSame($handle, $easy->handle); - $this->assertSame($stream, $easy->sink); - $this->assertSame($headers, $easy->headers); - $this->assertSame($request, $easy->request); - $this->assertSame($options, $easy->options); + self::assertSame($handle, $easy->handle); + self::assertSame($stream, $easy->sink); + self::assertSame($headers, $easy->headers); + self::assertSame($request, $easy->request); + self::assertSame($options, $easy->options); curl_close($easy->handle); unset($handle, $stream, $easy->handle); } - /** - * @expectedException \LogicException - * @expectedExceptionMessage Property GuzzleHttp\Handler\EasyHandle::$headers could not be set when there isn't a valid handle - */ - public function testSettingHeadersWithoutHandle() + public function testSettingHeadersWithoutHandle(): void { - $easy = new EasyHandle; + $easy = new EasyHandle(); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Property GuzzleHttp\Handler\EasyHandle::$headers could not be set when there isn\'t a valid handle'); + $easy->headers = []; } - public function testSettingErrnoWithHandle() + public function testSettingErrnoWithHandle(): void { - $easy = new EasyHandle; + $easy = new EasyHandle(); $easy->handle = curl_init(); - $easy->errno = CURLE_OK; + $easy->errno = \CURLE_OK; - $this->assertSame(CURLE_OK, $easy->errno); + self::assertSame(\CURLE_OK, $easy->errno); curl_close($easy->handle); unset($easy->handle); } - /** - * @expectedException \LogicException - * @expectedExceptionMessage Property GuzzleHttp\Handler\EasyHandle::$errno could not be set with 0 since the handle is reporting error 3 - */ - public function testChangingHandleErrno() + public function testChangingHandleErrno(): void { - $easy = new EasyHandle; + $easy = new EasyHandle(); $easy->handle = curl_init(); curl_exec($easy->handle); - $easy->errno = CURLE_OK; - $this->assertSame(CURLE_OK, $easy->errno); + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Property GuzzleHttp\Handler\EasyHandle::$errno could not be set with 0 since the handle is reporting error 3'); + + $easy->errno = \CURLE_OK; + + self::assertSame(\CURLE_OK, $easy->errno); curl_close($easy->handle); + unset($easy->handle); } - /** - * @expectedException \LogicException - * @expectedExceptionMessage Cannot set private property GuzzleHttp\Handler\EasyHandle::$response - */ - public function testSettingResponse() + public function testChangingHandleErrnoWithInvalidType(): void { - $easy = new EasyHandle; + $easy = new EasyHandle(); + $easy->handle = curl_init(); + curl_exec($easy->handle); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Property GuzzleHttp\Handler\EasyHandle::$errno can only accept a value of type int, string given'); + + $easy->errno = 'wrong_type'; + + self::assertSame(\CURLE_OK, $easy->errno); + + curl_close($easy->handle); + + unset($easy->handle); + } + + public function testSettingResponse(): void + { + $easy = new EasyHandle(); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Cannot set private property GuzzleHttp\Handler\EasyHandle::$response'); + $easy->response = new Psr7\Response(); } - /** - * @expectedException \LogicException - * @expectedExceptionMessage Property GuzzleHttp\Handler\EasyHandle::$errno could not be set when there isn't a valid handle - */ public function testSettingErrnoWithoutHandle() { - $easy = new EasyHandle; - $easy->errno = CURLE_COULDNT_RESOLVE_HOST; + $easy = new EasyHandle(); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Property GuzzleHttp\Handler\EasyHandle::$errno could not be set when there isn\'t a valid handle'); + + $easy->errno = \CURLE_COULDNT_RESOLVE_HOST; } - /** - * @expectedException \LogicException - * @expectedExceptionMessage Undefined property: GuzzleHttp\Handler\EasyHandle::$nonexistent - */ - public function testGetInvalidProperty() + public function testGetInvalidProperty(): void { - $easy = new EasyHandle; + $easy = new EasyHandle(); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Undefined property: GuzzleHttp\Handler\EasyHandle::$nonexistent'); + $easy->nonexistent; } - public function testPropertyOverload() + public function testPropertyOverload(): void { $overloadedValue = 42; - $easy = new EasyHandle; + $easy = new EasyHandle(); $easy->nonexistent = $overloadedValue; - $this->assertSame($overloadedValue, $easy->nonexistent); + self::assertSame($overloadedValue, $easy->nonexistent); } }