From d9a56a8fcf2bc90ae1e657c69fa5f05b3751b167 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sun, 10 Jan 2021 23:07:38 +0300 Subject: [PATCH 1/7] Now curl writes response into memory or temporary file --- lib/Client.php | 148 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 116 insertions(+), 32 deletions(-) diff --git a/lib/Client.php b/lib/Client.php index b79c564..1b98a0f 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -68,6 +68,8 @@ class Client extends EventEmitter protected $headerLinesMap = []; + protected $responseResourcesMap = []; + /** * Initializes the client. */ @@ -222,7 +224,7 @@ public function poll(): bool unset($this->curlMultiMap[$resourceId]); $curlHandle = $status['handle']; - $curlResult = $this->parseResponse(curl_multi_getcontent($curlHandle), $curlHandle); + $curlResult = $this->parseCurlResource($curlHandle); $retry = false; if (self::STATUS_CURLERROR === $curlResult['status']) { @@ -312,17 +314,15 @@ public function addCurlSetting(int $name, $value) */ protected function doRequest(RequestInterface $request): ResponseInterface { - $settings = $this->createCurlSettingsArray($request); - if (!$this->curlHandle) { $this->curlHandle = curl_init(); } else { curl_reset($this->curlHandle); } - curl_setopt_array($this->curlHandle, $settings); - $response = $this->curlExec($this->curlHandle); - $response = $this->parseResponse($response, $this->curlHandle); + $this->prepareCurl($request, $this->curlHandle); + $this->curlExec($this->curlHandle, false); + $response = $this->parseCurlResource($this->curlHandle); if (self::STATUS_CURLERROR === $response['status']) { throw new ClientException($response['curl_errmsg'], $response['curl_errno']); } @@ -415,26 +415,57 @@ protected function createCurlSettingsArray(RequestInterface $request): array const STATUS_CURLERROR = 1; const STATUS_HTTPERROR = 2; - private function parseResponse(string $response, $curlHandle): array + /** + * Parses the result of a curl call in a format that's a bit more + * convenient to work with. + * + * The method returns an array with the following elements: + * * status - one of the 3 STATUS constants. + * * curl_errno - A curl error number. Only set if status is + * STATUS_CURLERROR. + * * curl_errmsg - A current error message. Only set if status is + * STATUS_CURLERROR. + * * response - Response object. Only set if status is STATUS_SUCCESS, or + * STATUS_HTTPERROR. + * * http_code - HTTP status code, as an int. Only set if Only set if + * status is STATUS_SUCCESS, or STATUS_HTTPERROR + * + * @param resource $curlHandle + */ + protected function parseCurlResource($curlHandle): array { - $settings = $this->curlSettings; - $separatedHeaders = isset($settings[CURLOPT_HEADERFUNCTION]) && (bool) $settings[CURLOPT_HEADERFUNCTION]; + [$curlInfo, $curlErrNo, $curlErrMsg] = $this->curlStuff($curlHandle); - if ($separatedHeaders) { - $resourceId = (int) $curlHandle; - if (isset($this->headerLinesMap[$resourceId])) { - $headers = $this->headerLinesMap[$resourceId]; - } else { - $headers = []; + if ($curlErrNo) { + return [ + 'status' => self::STATUS_CURLERROR, + 'curl_errno' => $curlErrNo, + 'curl_errmsg' => $curlErrMsg, + ]; + } + + $response = new Response(); + $response->setStatus($curlInfo['http_code']); + $response->setBody($this->responseResourcesMap[(int) $curlHandle] ?? ''); + + $headerLines = $this->headerLinesMap[(int) $curlHandle] ?? []; + foreach ($headerLines as $header) { + $parts = explode(':', $header, 2); + if (2 === count($parts)) { + $response->addHeader(trim($parts[0]), trim($parts[1])); } - $response = $this->parseCurlResponse($headers, $response, $curlHandle); - } else { - $response = $this->parseCurlResult($response, $curlHandle); } - return $response; + $httpCode = $response->getStatus(); + + return [ + 'status' => $httpCode >= 400 ? self::STATUS_HTTPERROR : self::STATUS_SUCCESS, + 'response' => $response, + 'http_code' => $httpCode, + ]; } + /** * Parses the result of a curl call in a format that's a bit more * convenient to work with. @@ -450,6 +481,8 @@ private function parseResponse(string $response, $curlHandle): array * * http_code - HTTP status code, as an int. Only set if Only set if * status is STATUS_SUCCESS, or STATUS_HTTPERROR * + * @deprecated Use parseCurlResource instead + * * @param resource $curlHandle */ protected function parseCurlResponse(array $headerLines, string $body, $curlHandle): array @@ -458,7 +491,7 @@ protected function parseCurlResponse(array $headerLines, string $body, $curlHand $curlInfo, $curlErrNo, $curlErrMsg - ) = $this->curlStuff($curlHandle); + ) = $this->curlStuff($curlHandle); if ($curlErrNo) { return [ @@ -503,7 +536,7 @@ protected function parseCurlResponse(array $headerLines, string $body, $curlHand * * http_code - HTTP status code, as an int. Only set if Only set if * status is STATUS_SUCCESS, or STATUS_HTTPERROR * - * @deprecated Use parseCurlResponse instead + * @deprecated Use parseCurlResource instead * * @param resource $curlHandle */ @@ -513,7 +546,7 @@ protected function parseCurlResult(string $response, $curlHandle): array $curlInfo, $curlErrNo, $curlErrMsg - ) = $this->curlStuff($curlHandle); + ) = $this->curlStuff($curlHandle); if ($curlErrNo) { return [ @@ -557,10 +590,7 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes $this->curlMultiHandle = curl_multi_init(); } $curl = curl_init(); - curl_setopt_array( - $curl, - $this->createCurlSettingsArray($request) - ); + $this->prepareCurl($request, $curl); curl_multi_add_handle($this->curlMultiHandle, $curl); $resourceId = (int) $curl; @@ -573,6 +603,50 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes ]; } + /** + * @param resource $curlHandle + */ + private function prepareCurl(RequestInterface $request, $curlHandle): void + { + $handleId = (int) $curlHandle; + $this->headerLinesMap[$handleId] = []; + $this->responseResourcesMap[$handleId] = \fopen('php://temp', 'rw+'); + + $options = $this->createCurlSettingsArray($request); + + $options[CURLOPT_RETURNTRANSFER] = false; + $options[CURLOPT_HEADER] = false; + $options[CURLOPT_FILE] = $this->responseResourcesMap[$handleId]; + + $userHeaderFunction = $this->curlSettings[CURLOPT_HEADERFUNCTION] ?? null; + $options[CURLOPT_HEADERFUNCTION] = function ($curlHandle, $str) use ($userHeaderFunction) { + // Call user func + if ($userHeaderFunction !== null) { + $userHeaderFunction($curlHandle, $str); + } + + return $this->parseHeaders($curlHandle, $str); + }; + + curl_setopt_array($curlHandle, $options); + } + + /** + * @param resource $curlHandle + */ + protected function parseHeaders($curlHandle, string $str): int + { + // Header parsing + $headerBlob = explode("\r\n\r\n", trim($str, "\r\n")); + // We only care about the last set of headers + $headerBlob = end($headerBlob); + $headerBlob = explode("\r\n", $headerBlob); + foreach ($headerBlob as $headerLine) { + $this->receiveCurlHeader($curlHandle, $headerLine); + } + return strlen($str); + } + // @codeCoverageIgnoreStart /** @@ -581,17 +655,27 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes * This method exists so it can easily be overridden and mocked. * * @param resource $curlHandle + * @param bool $returnString If true then returns response content string + * + * @return bool|string */ - protected function curlExec($curlHandle): string + protected function curlExec($curlHandle, bool $returnString = true) { - $this->headerLinesMap[(int) $curlHandle] = []; - $result = curl_exec($curlHandle); - if (false === $result) { - $result = ''; + + $handleId = (int) $curlHandle; + + $streamExists = isset($this->responseResourcesMap[$handleId]); + if ($streamExists) { + rewind($this->responseResourcesMap[$handleId]); } - return $result; + if (!$returnString) { + return $result; + } + return false === $result || !$streamExists + ? '' + : fread($this->responseResourcesMap[$handleId], PHP_INT_MAX); } /** From 7ba4ce90b7d130cc59117c69fc58474b754b2740 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sun, 10 Jan 2021 23:39:46 +0300 Subject: [PATCH 2/7] Update tests --- tests/HTTP/ClientTest.php | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index f94b9a1..c485816 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -434,8 +434,10 @@ public function testDoRequest() { $client = new ClientMock(); $request = new Request('GET', 'http://example.org/'); - $client->on('curlExec', function (&$return) { - $return = "HTTP/1.1 200 OK\r\nHeader1:Val1\r\n\r\nFoo"; + $client->on('curlExec', function (&$return, string &$headers, string &$body) { + $return = true; + $body = 'Foo'; + $headers = "HTTP/1.1 200 OK\r\nHeader1:Val1\r\n\r\n"; }); $client->on('curlStuff', function (&$return) { $return = [ @@ -457,8 +459,8 @@ public function testDoRequestCurlError() { $client = new ClientMock(); $request = new Request('GET', 'http://example.org/'); - $client->on('curlExec', function (&$return) { - $return = ''; + $client->on('curlPrepareAndExec', function (&$return, string &$headers, string &$body) { + $return = false; }); $client->on('curlStuff', function (&$return) { $return = [ @@ -543,21 +545,26 @@ protected function curlStuff($curlHandle): array } /** - * Calls curl_exec. - * - * This method exists so it can easily be overridden and mocked. - * * @param resource $curlHandle */ - protected function curlExec($curlHandle): string + protected function curlExec($curlHandle, bool $returnString = true) { $return = null; - $this->emit('curlExec', [&$return]); + $headers = ''; + $body = ''; + $this->emit('curlExec', [&$return, &$headers, &$body]); // If nothing modified $return, we're using the default behavior. if (is_null($return)) { - return parent::curlExec($curlHandle); + return parent::curlExec($curlHandle, $returnString); } else { + $this->parseHeaders($curlHandle, $headers); + $stream = \fopen('php://memory', 'rw+'); + if (strlen($body) > 0) { + fwrite($stream, $body); + rewind($stream); + } + $this->responseResourcesMap[(int)$curlHandle] = $stream; return $return; } } From b03498b6abf35b8b09183ca94e6199d0cb71ba89 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 11 Jan 2021 00:59:32 +0300 Subject: [PATCH 3/7] Cleanup --- lib/Client.php | 78 +++++++++++++++++++-------------------- tests/HTTP/ClientTest.php | 12 +++--- 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/lib/Client.php b/lib/Client.php index 1b98a0f..e28d3db 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -26,7 +26,7 @@ * request before it's done, such as adding authentication headers. * * The afterRequest event will be emitted after the request is completed - * succesfully. + * successfully. * * If a HTTP error is returned (status code higher than 399) the error event is * triggered. It's possible using this event to retry the request, by setting @@ -53,7 +53,7 @@ class Client extends EventEmitter protected $curlSettings = []; /** - * Wether or not exceptions should be thrown when a HTTP error is returned. + * Whether or not exceptions should be thrown when a HTTP error is returned. * * @var bool */ @@ -66,6 +66,14 @@ class Client extends EventEmitter */ protected $maxRedirects = 5; + /** + * The maximum size of in-memory cache per request. If this value is exceeded, the cache is transferred to a + * temporary file. + * + * @var int + */ + protected $maxMemorySize = 2 * 1024 * 1024; + protected $headerLinesMap = []; protected $responseResourcesMap = []; @@ -199,31 +207,19 @@ public function poll(): bool } do { - $r = curl_multi_exec( - $this->curlMultiHandle, - $stillRunning - ); + $r = curl_multi_exec($this->curlMultiHandle, $stillRunning); } while (CURLM_CALL_MULTI_PERFORM === $r); $messagesInQueue = 0; do { - messageQueue: - - $status = curl_multi_info_read( - $this->curlMultiHandle, - $messagesInQueue - ); + $status = curl_multi_info_read($this->curlMultiHandle, $messagesInQueue); if ($status && CURLMSG_DONE === $status['msg']) { - $resourceId = (int) $status['handle']; - list( - $request, - $successCallback, - $errorCallback, - $retryCount) = $this->curlMultiMap[$resourceId]; + $curlHandle = $status['handle']; + $resourceId = (int) $curlHandle; + [$request, $successCallback, $errorCallback, $retryCount] = $this->curlMultiMap[$resourceId]; unset($this->curlMultiMap[$resourceId]); - $curlHandle = $status['handle']; $curlResult = $this->parseCurlResource($curlHandle); $retry = false; @@ -234,7 +230,7 @@ public function poll(): bool if ($retry) { ++$retryCount; $this->sendAsyncInternal($request, $successCallback, $errorCallback, $retryCount); - goto messageQueue; + continue; } $curlResult['request'] = $request; @@ -249,7 +245,7 @@ public function poll(): bool if ($retry) { ++$retryCount; $this->sendAsyncInternal($request, $successCallback, $errorCallback, $retryCount); - goto messageQueue; + continue; } $curlResult['request'] = $request; @@ -336,7 +332,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface * By keeping this resource around for the lifetime of this object, things * like persistent connections are possible. * - * @var resource + * @var resource|\CurlHandle */ private $curlHandle; @@ -345,7 +341,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface * * The first time sendAsync is used, this will be created. * - * @var resource + * @var resource|\CurlMultiHandle */ private $curlMultiHandle; @@ -430,7 +426,7 @@ protected function createCurlSettingsArray(RequestInterface $request): array * * http_code - HTTP status code, as an int. Only set if Only set if * status is STATUS_SUCCESS, or STATUS_HTTPERROR * - * @param resource $curlHandle + * @param resource|\CurlHandle $curlHandle */ protected function parseCurlResource($curlHandle): array { @@ -465,7 +461,6 @@ protected function parseCurlResource($curlHandle): array ]; } - /** * Parses the result of a curl call in a format that's a bit more * convenient to work with. @@ -483,7 +478,7 @@ protected function parseCurlResource($curlHandle): array * * @deprecated Use parseCurlResource instead * - * @param resource $curlHandle + * @param resource|\CurlHandle $curlHandle */ protected function parseCurlResponse(array $headerLines, string $body, $curlHandle): array { @@ -538,7 +533,7 @@ protected function parseCurlResponse(array $headerLines, string $body, $curlHand * * @deprecated Use parseCurlResource instead * - * @param resource $curlHandle + * @param resource|\CurlHandle $curlHandle */ protected function parseCurlResult(string $response, $curlHandle): array { @@ -594,7 +589,6 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes curl_multi_add_handle($this->curlMultiHandle, $curl); $resourceId = (int) $curl; - $this->headerLinesMap[$resourceId] = []; $this->curlMultiMap[$resourceId] = [ $request, $success, @@ -604,19 +598,25 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes } /** - * @param resource $curlHandle + * @param resource|\CurlHandle $curlHandle */ - private function prepareCurl(RequestInterface $request, $curlHandle): void + protected function prepareCurl(RequestInterface $request, $curlHandle): void { $handleId = (int) $curlHandle; $this->headerLinesMap[$handleId] = []; - $this->responseResourcesMap[$handleId] = \fopen('php://temp', 'rw+'); - $options = $this->createCurlSettingsArray($request); + if (array_key_exists(CURLOPT_NOBODY, $options) && !array_key_exists(CURLOPT_WRITEFUNCTION, $options)) { + $this->responseResourcesMap[$handleId] = \fopen( + "php://temp/maxmemory:{$this->maxMemorySize}", + 'rw+b' + ); + // $this->responseResourcesMap[$handleId] = tmpfile(); + $options[CURLOPT_FILE] = $this->responseResourcesMap[$handleId]; + } + $options[CURLOPT_RETURNTRANSFER] = false; $options[CURLOPT_HEADER] = false; - $options[CURLOPT_FILE] = $this->responseResourcesMap[$handleId]; $userHeaderFunction = $this->curlSettings[CURLOPT_HEADERFUNCTION] ?? null; $options[CURLOPT_HEADERFUNCTION] = function ($curlHandle, $str) use ($userHeaderFunction) { @@ -625,16 +625,16 @@ private function prepareCurl(RequestInterface $request, $curlHandle): void $userHeaderFunction($curlHandle, $str); } - return $this->parseHeaders($curlHandle, $str); + return $this->parseHeadersBlock($curlHandle, $str); }; curl_setopt_array($curlHandle, $options); } /** - * @param resource $curlHandle + * @param resource|\CurlHandle $curlHandle */ - protected function parseHeaders($curlHandle, string $str): int + protected function parseHeadersBlock($curlHandle, string $str): int { // Header parsing $headerBlob = explode("\r\n\r\n", trim($str, "\r\n")); @@ -654,7 +654,7 @@ protected function parseHeaders($curlHandle, string $str): int * * This method exists so it can easily be overridden and mocked. * - * @param resource $curlHandle + * @param resource|\CurlHandle $curlHandle * @param bool $returnString If true then returns response content string * * @return bool|string @@ -675,7 +675,7 @@ protected function curlExec($curlHandle, bool $returnString = true) } return false === $result || !$streamExists ? '' - : fread($this->responseResourcesMap[$handleId], PHP_INT_MAX); + : stream_get_contents($this->responseResourcesMap[$handleId]); } /** @@ -683,7 +683,7 @@ protected function curlExec($curlHandle, bool $returnString = true) * * This method exists so it can easily be overridden and mocked. * - * @param resource $curlHandle + * @param resource|\CurlHandle $curlHandle */ protected function curlStuff($curlHandle): array { diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index c485816..72a3e89 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -229,7 +229,7 @@ public function testSendAsync() $request = new Request('GET', $url); $client->sendAsync($request, function (ResponseInterface $response) { - $this->assertEquals("foo\n", $response->getBody()); + $this->assertEquals("foo\n", stream_get_contents($response->getBody())); $this->assertEquals(200, $response->getStatus()); $this->assertEquals(4, $response->getHeader('Content-Length')); }, function ($error) use ($request) { @@ -243,7 +243,7 @@ public function testSendAsync() /** * @group ci */ - public function testSendAsynConsecutively() + public function testSendAsyncConsecutively() { $url = $this->getAbsoluteUrl('/foo'); if (!$url) { @@ -254,7 +254,7 @@ public function testSendAsynConsecutively() $request = new Request('GET', $url); $client->sendAsync($request, function (ResponseInterface $response) { - $this->assertEquals("foo\n", $response->getBody()); + $this->assertEquals("foo\n", stream_get_contents($response->getBody())); $this->assertEquals(200, $response->getStatus()); $this->assertEquals(4, $response->getHeader('Content-Length')); }, function ($error) use ($request) { @@ -265,7 +265,7 @@ public function testSendAsynConsecutively() $url = $this->getAbsoluteUrl('/bar.php'); $request = new Request('GET', $url); $client->sendAsync($request, function (ResponseInterface $response) { - $this->assertEquals("bar\n", $response->getBody()); + $this->assertEquals("bar\n", stream_get_contents($response->getBody())); $this->assertEquals(200, $response->getStatus()); $this->assertEquals('Bar', $response->getHeader('X-Test')); }, function ($error) use ($request) { @@ -459,7 +459,7 @@ public function testDoRequestCurlError() { $client = new ClientMock(); $request = new Request('GET', 'http://example.org/'); - $client->on('curlPrepareAndExec', function (&$return, string &$headers, string &$body) { + $client->on('curlExec', function (&$return, string &$headers, string &$body) { $return = false; }); $client->on('curlStuff', function (&$return) { @@ -558,7 +558,7 @@ protected function curlExec($curlHandle, bool $returnString = true) if (is_null($return)) { return parent::curlExec($curlHandle, $returnString); } else { - $this->parseHeaders($curlHandle, $headers); + $this->parseHeadersBlock($curlHandle, $headers); $stream = \fopen('php://memory', 'rw+'); if (strlen($body) > 0) { fwrite($stream, $body); From 6c0e1c9d304981cfa4fe4099353a1d05106d8816 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 11 Jan 2021 14:51:45 +0300 Subject: [PATCH 4/7] Optimize --- lib/Client.php | 163 ++++++++++++++++---------------------- tests/HTTP/ClientTest.php | 24 +++--- 2 files changed, 81 insertions(+), 106 deletions(-) diff --git a/lib/Client.php b/lib/Client.php index e28d3db..d95d539 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -45,12 +45,19 @@ */ class Client extends EventEmitter { + const STATUS_SUCCESS = 0; + const STATUS_CURLERROR = 1; + const STATUS_HTTPERROR = 2; + /** * List of curl settings. * * @var array */ - protected $curlSettings = []; + protected $curlSettings = [ + CURLOPT_NOBODY => false, + CURLOPT_USERAGENT => 'sabre-http/'.Version::VERSION.' (http://sabre.io/)', + ]; /** * Whether or not exceptions should be thrown when a HTTP error is returned. @@ -79,27 +86,33 @@ class Client extends EventEmitter protected $responseResourcesMap = []; /** - * Initializes the client. + * Cached curl handle. + * + * By keeping this resource around for the lifetime of this object, things + * like persistent connections are possible. + * + * @var resource|\CurlHandle */ - public function __construct() - { - // See https://github.com/sabre-io/http/pull/115#discussion_r241292068 - // Preserve compatibility for sub-classes that implements their own method `parseCurlResult` - $separatedHeaders = __CLASS__ === get_class($this); - - $this->curlSettings = [ - CURLOPT_RETURNTRANSFER => true, - CURLOPT_NOBODY => false, - CURLOPT_USERAGENT => 'sabre-http/'.Version::VERSION.' (http://sabre.io/)', - ]; - if ($separatedHeaders) { - $this->curlSettings[CURLOPT_HEADERFUNCTION] = [$this, 'receiveCurlHeader']; - } else { - $this->curlSettings[CURLOPT_HEADER] = true; - } - } + private $curlHandle; + + /** + * Handler for curl_multi requests. + * + * The first time sendAsync is used, this will be created. + * + * @var resource|\CurlMultiHandle + */ + private $curlMultiHandle; + + /** + * Has a list of curl handles, as well as their associated success and + * error callbacks. + * + * @var array + */ + private $curlMultiMap = []; - protected function receiveCurlHeader($curlHandle, $headerLine) + protected function receiveCurlHeader($curlHandle, $headerLine): int { $this->headerLinesMap[(int) $curlHandle][] = $headerLine; @@ -214,13 +227,22 @@ public function poll(): bool do { $status = curl_multi_info_read($this->curlMultiHandle, $messagesInQueue); - if ($status && CURLMSG_DONE === $status['msg']) { + if ($status !== false && CURLMSG_DONE === $status['msg']) { $curlHandle = $status['handle']; - $resourceId = (int) $curlHandle; - [$request, $successCallback, $errorCallback, $retryCount] = $this->curlMultiMap[$resourceId]; - unset($this->curlMultiMap[$resourceId]); + $handleId = (int) $curlHandle; + [$request, $successCallback, $errorCallback, $retryCount] = $this->curlMultiMap[$handleId]; $curlResult = $this->parseCurlResource($curlHandle); + + // Cleanup + curl_multi_remove_handle($this->curlMultiHandle, $curlHandle); + curl_close($curlHandle); + unset( + $this->curlMultiMap[$handleId], + $this->responseResourcesMap[$handleId], + $this->headerLinesMap[$handleId] + ); + $retry = false; if (self::STATUS_CURLERROR === $curlResult['status']) { @@ -326,33 +348,6 @@ protected function doRequest(RequestInterface $request): ResponseInterface return $response['response']; } - /** - * Cached curl handle. - * - * By keeping this resource around for the lifetime of this object, things - * like persistent connections are possible. - * - * @var resource|\CurlHandle - */ - private $curlHandle; - - /** - * Handler for curl_multi requests. - * - * The first time sendAsync is used, this will be created. - * - * @var resource|\CurlMultiHandle - */ - private $curlMultiHandle; - - /** - * Has a list of curl handles, as well as their associated success and - * error callbacks. - * - * @var array - */ - private $curlMultiMap = []; - /** * Turns a RequestInterface object into an array with settings that can be * fed to curl_setopt. @@ -407,10 +402,6 @@ protected function createCurlSettingsArray(RequestInterface $request): array return $settings; } - const STATUS_SUCCESS = 0; - const STATUS_CURLERROR = 1; - const STATUS_HTTPERROR = 2; - /** * Parses the result of a curl call in a format that's a bit more * convenient to work with. @@ -439,12 +430,17 @@ protected function parseCurlResource($curlHandle): array 'curl_errmsg' => $curlErrMsg, ]; } + $handleId = (int) $curlHandle; $response = new Response(); $response->setStatus($curlInfo['http_code']); - $response->setBody($this->responseResourcesMap[(int) $curlHandle] ?? ''); - $headerLines = $this->headerLinesMap[(int) $curlHandle] ?? []; + if (isset($this->responseResourcesMap[$handleId])) { + rewind($this->responseResourcesMap[$handleId]); + $response->setBody($this->responseResourcesMap[$handleId]); + } + + $headerLines = $this->headerLinesMap[$handleId] ?? []; foreach ($headerLines as $header) { $parts = explode(':', $header, 2); if (2 === count($parts)) { @@ -606,47 +602,30 @@ protected function prepareCurl(RequestInterface $request, $curlHandle): void $this->headerLinesMap[$handleId] = []; $options = $this->createCurlSettingsArray($request); - if (array_key_exists(CURLOPT_NOBODY, $options) && !array_key_exists(CURLOPT_WRITEFUNCTION, $options)) { - $this->responseResourcesMap[$handleId] = \fopen( - "php://temp/maxmemory:{$this->maxMemorySize}", - 'rw+b' - ); - // $this->responseResourcesMap[$handleId] = tmpfile(); - $options[CURLOPT_FILE] = $this->responseResourcesMap[$handleId]; - } - $options[CURLOPT_RETURNTRANSFER] = false; $options[CURLOPT_HEADER] = false; + if (!($options[CURLOPT_NOBODY] ?? false) && !array_key_exists(CURLOPT_WRITEFUNCTION, $options)) { + $options[CURLOPT_FILE] = $this->responseResourcesMap[$handleId] = $options[CURLOPT_FILE] ?? \fopen( + "php://temp/maxmemory:{$this->maxMemorySize}", + 'rw+b' + ); + } else { + $this->responseResourcesMap[$handleId] = null; + } + $userHeaderFunction = $this->curlSettings[CURLOPT_HEADERFUNCTION] ?? null; $options[CURLOPT_HEADERFUNCTION] = function ($curlHandle, $str) use ($userHeaderFunction) { // Call user func - if ($userHeaderFunction !== null) { + if (is_callable($userHeaderFunction)) { $userHeaderFunction($curlHandle, $str); } - - return $this->parseHeadersBlock($curlHandle, $str); + return $this->receiveCurlHeader($curlHandle, $str); }; curl_setopt_array($curlHandle, $options); } - /** - * @param resource|\CurlHandle $curlHandle - */ - protected function parseHeadersBlock($curlHandle, string $str): int - { - // Header parsing - $headerBlob = explode("\r\n\r\n", trim($str, "\r\n")); - // We only care about the last set of headers - $headerBlob = end($headerBlob); - $headerBlob = explode("\r\n", $headerBlob); - foreach ($headerBlob as $headerLine) { - $this->receiveCurlHeader($curlHandle, $headerLine); - } - return strlen($str); - } - // @codeCoverageIgnoreStart /** @@ -665,17 +644,15 @@ protected function curlExec($curlHandle, bool $returnString = true) $handleId = (int) $curlHandle; - $streamExists = isset($this->responseResourcesMap[$handleId]); - if ($streamExists) { - rewind($this->responseResourcesMap[$handleId]); - } - if (!$returnString) { return $result; } - return false === $result || !$streamExists - ? '' - : stream_get_contents($this->responseResourcesMap[$handleId]); + + if (!$result || !isset($this->responseResourcesMap[$handleId])) { + return ''; + } + rewind($this->responseResourcesMap[$handleId]); + return stream_get_contents($this->responseResourcesMap[$handleId]); } /** diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index 72a3e89..271dcdc 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -14,8 +14,6 @@ public function testCreateCurlSettingsArrayGET() $request = new Request('GET', 'http://example.org/', ['X-Foo' => 'bar']); $settings = [ - CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, CURLOPT_POSTREDIR => 0, CURLOPT_HTTPHEADER => ['X-Foo: bar'], CURLOPT_NOBODY => false, @@ -40,8 +38,6 @@ public function testCreateCurlSettingsArrayHEAD() $request = new Request('HEAD', 'http://example.org/', ['X-Foo' => 'bar']); $settings = [ - CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, CURLOPT_NOBODY => true, CURLOPT_CUSTOMREQUEST => 'HEAD', CURLOPT_HTTPHEADER => ['X-Foo: bar'], @@ -74,8 +70,6 @@ public function testCreateCurlSettingsArrayGETAfterHEAD() $settings = [ CURLOPT_CUSTOMREQUEST => 'GET', - CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, CURLOPT_HTTPHEADER => ['X-Foo: bar'], CURLOPT_NOBODY => false, CURLOPT_URL => 'http://example.org/', @@ -101,8 +95,6 @@ public function testCreateCurlSettingsArrayPUTStream() $request = new Request('PUT', 'http://example.org/', ['X-Foo' => 'bar'], $h); $settings = [ - CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, CURLOPT_PUT => true, CURLOPT_INFILE => $h, CURLOPT_NOBODY => false, @@ -128,8 +120,6 @@ public function testCreateCurlSettingsArrayPUTString() $request = new Request('PUT', 'http://example.org/', ['X-Foo' => 'bar'], 'boo'); $settings = [ - CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, CURLOPT_NOBODY => false, CURLOPT_POSTFIELDS => 'boo', CURLOPT_CUSTOMREQUEST => 'PUT', @@ -254,7 +244,7 @@ public function testSendAsyncConsecutively() $request = new Request('GET', $url); $client->sendAsync($request, function (ResponseInterface $response) { - $this->assertEquals("foo\n", stream_get_contents($response->getBody())); + $this->assertEquals("foo\n", $response->getBodyAsString()); $this->assertEquals(200, $response->getStatus()); $this->assertEquals(4, $response->getHeader('Content-Length')); }, function ($error) use ($request) { @@ -265,7 +255,7 @@ public function testSendAsyncConsecutively() $url = $this->getAbsoluteUrl('/bar.php'); $request = new Request('GET', $url); $client->sendAsync($request, function (ResponseInterface $response) { - $this->assertEquals("bar\n", stream_get_contents($response->getBody())); + $this->assertEquals("bar\n", $response->getBodyAsString()); $this->assertEquals(200, $response->getStatus()); $this->assertEquals('Bar', $response->getHeader('X-Test')); }, function ($error) use ($request) { @@ -487,7 +477,7 @@ class ClientMock extends Client /** * Making this method public. */ - public function receiveCurlHeader($curlHandle, $headerLine) + public function receiveCurlHeader($curlHandle, $headerLine): int { return parent::receiveCurlHeader($curlHandle, $headerLine); } @@ -568,4 +558,12 @@ protected function curlExec($curlHandle, bool $returnString = true) return $return; } } + + protected function parseHeadersBlock($curlHandle, string $str): void + { + $headerBlob = explode("\r\n", trim($str, "\r\n")); + foreach ($headerBlob as $headerLine) { + $this->receiveCurlHeader($curlHandle, $headerLine); + } + } } From 63b8b73888fcac0833d7e4db8d1de8427ac234b7 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 11 Jan 2021 15:15:16 +0300 Subject: [PATCH 5/7] CS Fixer --- lib/Client.php | 18 ++++++------------ tests/HTTP/ClientTest.php | 3 ++- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/Client.php b/lib/Client.php index d95d539..2075a94 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -227,7 +227,7 @@ public function poll(): bool do { $status = curl_multi_info_read($this->curlMultiHandle, $messagesInQueue); - if ($status !== false && CURLMSG_DONE === $status['msg']) { + if (false !== $status && CURLMSG_DONE === $status['msg']) { $curlHandle = $status['handle']; $handleId = (int) $curlHandle; [$request, $successCallback, $errorCallback, $retryCount] = $this->curlMultiMap[$handleId]; @@ -478,11 +478,7 @@ protected function parseCurlResource($curlHandle): array */ protected function parseCurlResponse(array $headerLines, string $body, $curlHandle): array { - list( - $curlInfo, - $curlErrNo, - $curlErrMsg - ) = $this->curlStuff($curlHandle); + [$curlInfo, $curlErrNo, $curlErrMsg] = $this->curlStuff($curlHandle); if ($curlErrNo) { return [ @@ -533,11 +529,7 @@ protected function parseCurlResponse(array $headerLines, string $body, $curlHand */ protected function parseCurlResult(string $response, $curlHandle): array { - list( - $curlInfo, - $curlErrNo, - $curlErrMsg - ) = $this->curlStuff($curlHandle); + [$curlInfo, $curlErrNo, $curlErrMsg] = $this->curlStuff($curlHandle); if ($curlErrNo) { return [ @@ -620,6 +612,7 @@ protected function prepareCurl(RequestInterface $request, $curlHandle): void if (is_callable($userHeaderFunction)) { $userHeaderFunction($curlHandle, $str); } + return $this->receiveCurlHeader($curlHandle, $str); }; @@ -634,7 +627,7 @@ protected function prepareCurl(RequestInterface $request, $curlHandle): void * This method exists so it can easily be overridden and mocked. * * @param resource|\CurlHandle $curlHandle - * @param bool $returnString If true then returns response content string + * @param bool $returnString If true then returns response content string * * @return bool|string */ @@ -652,6 +645,7 @@ protected function curlExec($curlHandle, bool $returnString = true) return ''; } rewind($this->responseResourcesMap[$handleId]); + return stream_get_contents($this->responseResourcesMap[$handleId]); } diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index 271dcdc..bddcc1b 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -554,7 +554,8 @@ protected function curlExec($curlHandle, bool $returnString = true) fwrite($stream, $body); rewind($stream); } - $this->responseResourcesMap[(int)$curlHandle] = $stream; + $this->responseResourcesMap[(int) $curlHandle] = $stream; + return $return; } } From c3c7c910acd4b2ba7736660d171362e46d74e484 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 11 Jan 2021 16:07:25 +0300 Subject: [PATCH 6/7] Fix BC, add test for a big file downloading --- lib/Client.php | 34 +++++++++++++++++++------- tests/HTTP/ClientTest.php | 51 +++++++++++++++++++++++++++++++++++---- tests/www/anysize.php | 19 +++++++++++++++ 3 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 tests/www/anysize.php diff --git a/lib/Client.php b/lib/Client.php index 2075a94..7c01ac5 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -112,6 +112,13 @@ class Client extends EventEmitter */ private $curlMultiMap = []; + /** + * Reserved for backward compatibility. + */ + public function __construct() + { + } + protected function receiveCurlHeader($curlHandle, $headerLine): int { $this->headerLinesMap[(int) $curlHandle][] = $headerLine; @@ -339,7 +346,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface } $this->prepareCurl($request, $this->curlHandle); - $this->curlExec($this->curlHandle, false); + $this->curlExecInternal($this->curlHandle); $response = $this->parseCurlResource($this->curlHandle); if (self::STATUS_CURLERROR === $response['status']) { throw new ClientException($response['curl_errmsg'], $response['curl_errno']); @@ -438,6 +445,8 @@ protected function parseCurlResource($curlHandle): array if (isset($this->responseResourcesMap[$handleId])) { rewind($this->responseResourcesMap[$handleId]); $response->setBody($this->responseResourcesMap[$handleId]); + } else { + $response->setBody(''); } $headerLines = $this->headerLinesMap[$handleId] ?? []; @@ -627,20 +636,27 @@ protected function prepareCurl(RequestInterface $request, $curlHandle): void * This method exists so it can easily be overridden and mocked. * * @param resource|\CurlHandle $curlHandle - * @param bool $returnString If true then returns response content string + */ + protected function curlExecInternal($curlHandle): bool + { + return curl_exec($curlHandle); + } + + /** + * Calls curl_exec and returns content string with headers. + * + * This method exists so it can easily be overridden and mocked. * - * @return bool|string + * @param resource|\CurlHandle $curlHandle + * + * @deprecated */ - protected function curlExec($curlHandle, bool $returnString = true) + protected function curlExec($curlHandle): string { - $result = curl_exec($curlHandle); + $result = $this->curlExecInternal($curlHandle); $handleId = (int) $curlHandle; - if (!$returnString) { - return $result; - } - if (!$result || !isset($this->responseResourcesMap[$handleId])) { return ''; } diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index bddcc1b..7cc99d3 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -205,6 +205,25 @@ public function testSendToGetLargeContent() $this->assertLessThan(60 * pow(1024, 2), memory_get_peak_usage()); } + /** + * @group ci + */ + public function testSendToGetVeryLargeContent() + { + $memoryLimit = max(64, ceil($this->getMemoryLimit() / 1024 / 1024)); + $url = $this->getAbsoluteUrl("/anysize.php?size={$memoryLimit}"); + if (!$url) { + $this->markTestSkipped('Set an environment value BASEURL to continue'); + } + + $request = new Request('GET', $url); + $client = new Client(); + $response = $client->send($request); + + $this->assertEquals(200, $response->getStatus()); + $this->assertLessThan(60 * pow(1024, 2), memory_get_peak_usage()); + } + /** * @group ci */ @@ -424,7 +443,7 @@ public function testDoRequest() { $client = new ClientMock(); $request = new Request('GET', 'http://example.org/'); - $client->on('curlExec', function (&$return, string &$headers, string &$body) { + $client->on('curlExecInternal', function (&$return, string &$headers, string &$body) { $return = true; $body = 'Foo'; $headers = "HTTP/1.1 200 OK\r\nHeader1:Val1\r\n\r\n"; @@ -449,7 +468,7 @@ public function testDoRequestCurlError() { $client = new ClientMock(); $request = new Request('GET', 'http://example.org/'); - $client->on('curlExec', function (&$return, string &$headers, string &$body) { + $client->on('curlExecInternal', function (&$return, string &$headers, string &$body) { $return = false; }); $client->on('curlStuff', function (&$return) { @@ -468,6 +487,28 @@ public function testDoRequestCurlError() $this->assertEquals('Curl error', $e->getMessage()); } } + + private function getMemoryLimit(): int + { + $val = trim(ini_get('memory_limit')); + $last = strtolower($val[strlen($val) - 1]); + $val = substr($val, 0, -1); + if (!is_numeric($val) || is_numeric($last)) { + return PHP_INT_MAX; + } + switch ($last) { + case 'g': + $val *= 1024; + // no break + case 'm': + $val *= 1024; + // no break + case 'k': + $val *= 1024; + } + + return $val; + } } class ClientMock extends Client @@ -537,16 +578,16 @@ protected function curlStuff($curlHandle): array /** * @param resource $curlHandle */ - protected function curlExec($curlHandle, bool $returnString = true) + protected function curlExecInternal($curlHandle): bool { $return = null; $headers = ''; $body = ''; - $this->emit('curlExec', [&$return, &$headers, &$body]); + $this->emit('curlExecInternal', [&$return, &$headers, &$body]); // If nothing modified $return, we're using the default behavior. if (is_null($return)) { - return parent::curlExec($curlHandle, $returnString); + return parent::curlExecInternal($curlHandle); } else { $this->parseHeadersBlock($curlHandle, $headers); $stream = \fopen('php://memory', 'rw+'); diff --git a/tests/www/anysize.php b/tests/www/anysize.php new file mode 100644 index 0000000..4a16508 --- /dev/null +++ b/tests/www/anysize.php @@ -0,0 +1,19 @@ + $chunk; + } +}; + +foreach ($generator(512 * 1024, $megabytes * 2) as $value) { + echo $value; +} From 2703040a5c7f24adb1ddb18e213cd678d17d3633 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Tue, 19 Jan 2021 12:13:19 +0300 Subject: [PATCH 7/7] Add empty body test --- tests/HTTP/ClientTest.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index 7cc99d3..844995a 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -205,6 +205,24 @@ public function testSendToGetLargeContent() $this->assertLessThan(60 * pow(1024, 2), memory_get_peak_usage()); } + /** + * @group ci + */ + public function testSendHeadHeader() + { + $url = $this->getAbsoluteUrl('/large.php'); + if (!$url) { + $this->markTestSkipped('Set an environment value BASEURL to continue'); + } + + $request = new Request('HEAD', $url); + $client = new Client(); + $response = $client->send($request); + + $this->assertEquals(200, $response->getStatus()); + $this->assertEmpty($response->getBodyAsString()); + } + /** * @group ci */