From 48020a299805fe7abff98d5c247338d20e641c27 Mon Sep 17 00:00:00 2001 From: Gasol Wu Date: Wed, 12 Dec 2018 16:32:54 +0800 Subject: [PATCH] Reduce memory footprint when parsing HTTP result In order to resolve #82, Remove the substr function in `parseCurlResult()` and register receiveCurlHeader as callback function by using CURLOPT_HEADERFUNCTION instead, To avoid additional memory copy of response body (from $response to $responseBody). This changes is not intended to resolve issue #15 --- lib/Client.php | 53 +++++++++++++++++++-------------------- tests/HTTP/ClientTest.php | 31 ++++++++++++++--------- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/lib/Client.php b/lib/Client.php index 5940662..2527bcf 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -66,6 +66,8 @@ class Client extends EventEmitter */ protected $maxRedirects = 5; + protected $headerLinesMap = []; + /** * Initializes the client. */ @@ -73,12 +75,19 @@ public function __construct() { $this->curlSettings = [ CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, + CURLOPT_HEADERFUNCTION => [$this, 'receiveCurlHeader'], CURLOPT_NOBODY => false, CURLOPT_USERAGENT => 'sabre-http/'.Version::VERSION.' (http://sabre.io/)', ]; } + protected function receiveCurlHeader($curlHandle, $headerLine) + { + $this->headerLinesMap[(int) $curlHandle][] = $headerLine; + + return strlen($headerLine); + } + /** * Sends a request to a HTTP server, and returns a response. */ @@ -414,7 +423,7 @@ protected function createCurlSettingsArray(RequestInterface $request): array * * @param resource $curlHandle */ - protected function parseCurlResult(string $response, $curlHandle): array + protected function parseCurlResult(string $body, $curlHandle): array { list( $curlInfo, @@ -430,36 +439,21 @@ protected function parseCurlResult(string $response, $curlHandle): array ]; } - $headerBlob = substr($response, 0, $curlInfo['header_size']); - // In the case of 204 No Content, strlen($response) == $curlInfo['header_size]. - // This will cause substr($response, $curlInfo['header_size']) return FALSE instead of NULL - // An exception will be thrown when calling getBodyAsString then - $responseBody = substr($response, $curlInfo['header_size']) ?: null; - - unset($response); - - // In the case of 100 Continue, or redirects we'll have multiple lists - // of headers for each separate HTTP response. We can easily split this - // because they are separated by \r\n\r\n - $headerBlob = explode("\r\n\r\n", trim($headerBlob, "\r\n")); - - // We only care about the last set of headers - $headerBlob = $headerBlob[count($headerBlob) - 1]; - - // Splitting headers - $headerBlob = explode("\r\n", $headerBlob); - $response = new Response(); $response->setStatus($curlInfo['http_code']); - foreach ($headerBlob as $header) { - $parts = explode(':', $header, 2); - if (2 === count($parts)) { - $response->addHeader(trim($parts[0]), trim($parts[1])); + $resourceId = (int) $curlHandle; + if (isset($this->headerLinesMap[$resourceId])) { + foreach ($this->headerLinesMap[$resourceId] as $header) { + $parts = explode(':', $header, 2); + if (2 === count($parts)) { + $response->addHeader(trim($parts[0]), trim($parts[1])); + } } + $this->headerLinesMap[$resourceId] = []; } - $response->setBody($responseBody); + $response->setBody($body); $httpCode = $response->getStatus(); @@ -487,7 +481,10 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes $this->createCurlSettingsArray($request) ); curl_multi_add_handle($this->curlMultiHandle, $curl); - $this->curlMultiMap[(int) $curl] = [ + + $resourceId = (int) $curl; + $this->headerLinesMap[$resourceId] = []; + $this->curlMultiMap[$resourceId] = [ $request, $success, $error, @@ -506,6 +503,8 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes */ protected function curlExec($curlHandle): string { + $this->headerLinesMap[(int) $curlHandle] = []; + return curl_exec($curlHandle); } diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index b960423..938f5ed 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -15,7 +15,7 @@ public function testCreateCurlSettingsArrayGET() $settings = [ CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, + CURLOPT_HEADERFUNCTION => [$client, 'receiveCurlHeader'], CURLOPT_POSTREDIR => 0, CURLOPT_HTTPHEADER => ['X-Foo: bar'], CURLOPT_NOBODY => false, @@ -41,7 +41,7 @@ public function testCreateCurlSettingsArrayHEAD() $settings = [ CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, + CURLOPT_HEADERFUNCTION => [$client, 'receiveCurlHeader'], CURLOPT_NOBODY => true, CURLOPT_CUSTOMREQUEST => 'HEAD', CURLOPT_HTTPHEADER => ['X-Foo: bar'], @@ -75,7 +75,7 @@ public function testCreateCurlSettingsArrayGETAfterHEAD() $settings = [ CURLOPT_CUSTOMREQUEST => 'GET', CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, + CURLOPT_HEADERFUNCTION => [$client, 'receiveCurlHeader'], CURLOPT_HTTPHEADER => ['X-Foo: bar'], CURLOPT_NOBODY => false, CURLOPT_URL => 'http://example.org/', @@ -102,7 +102,7 @@ public function testCreateCurlSettingsArrayPUTStream() $settings = [ CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, + CURLOPT_HEADERFUNCTION => [$client, 'receiveCurlHeader'], CURLOPT_PUT => true, CURLOPT_INFILE => $h, CURLOPT_NOBODY => false, @@ -129,7 +129,7 @@ public function testCreateCurlSettingsArrayPUTString() $settings = [ CURLOPT_RETURNTRANSFER => true, - CURLOPT_HEADER => true, + CURLOPT_HEADERFUNCTION => [$client, 'receiveCurlHeader'], CURLOPT_NOBODY => false, CURLOPT_POSTFIELDS => 'boo', CURLOPT_CUSTOMREQUEST => 'PUT', @@ -286,8 +286,9 @@ public function testParseCurlResult() ]; }); - $body = "HTTP/1.1 200 OK\r\nHeader1:Val1\r\n\r\nFoo"; - $result = $client->parseCurlResult($body, 'foobar'); + $client->receiveCurlHeader(null, "HTTP/1.1 200 OK\r\n"); + $client->receiveCurlHeader(null, "Header1: Val1\r\n"); + $result = $client->parseCurlResult('Foo', 'foobar'); $this->assertEquals(Client::STATUS_SUCCESS, $result['status']); $this->assertEquals(200, $result['http_code']); @@ -307,7 +308,7 @@ public function testParseCurlError() ]; }); - $body = "HTTP/1.1 200 OK\r\nHeader1:Val1\r\n\r\nFoo"; + $body = 'Foo'; $result = $client->parseCurlResult($body, 'foobar'); $this->assertEquals(Client::STATUS_CURLERROR, $result['status']); @@ -320,12 +321,11 @@ 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"; + $return = 'Foo'; }); $client->on('curlStuff', function (&$return) { $return = [ [ - 'header_size' => 33, 'http_code' => 200, ], 0, @@ -334,7 +334,6 @@ public function testDoRequest() }); $response = $client->doRequest($request); $this->assertEquals(200, $response->getStatus()); - $this->assertEquals(['Header1' => ['Val1']], $response->getHeaders()); $this->assertEquals('Foo', $response->getBodyAsString()); } @@ -367,6 +366,14 @@ class ClientMock extends Client { protected $persistedSettings = []; + /** + * Making this method public. + */ + public function receiveCurlHeader($curlHandle, $headerLine) + { + return parent::receiveCurlHeader($curlHandle, $headerLine); + } + /** * Making this method public. */ @@ -429,7 +436,7 @@ protected function curlStuff($curlHandle): array protected function curlExec($curlHandle): string { $return = null; - $this->emit('curlExec', [&$return]); + $this->emit('curlExec', [&$return, $curlHandle]); // If nothing modified $return, we're using the default behavior. if (is_null($return)) {