Skip to content

Commit

Permalink
Reduce memory footprint when parsing HTTP result
Browse files Browse the repository at this point in the history 
In order to resolve sabre-io#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 sabre-io#15
  • Loading branch information
Gasol Wu committed Dec 13, 2018
1 parent 5644080 commit f68b5d5
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 44 deletions.
53 changes: 26 additions & 27 deletions lib/Client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,28 @@ class Client extends EventEmitter
*/
protected $maxRedirects = 5;

protected $headerLinesMap = [];

/**
* Initializes the client.
*/
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.
*/
Expand Down Expand Up @@ -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,
Expand All @@ -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();

Expand Down Expand Up @@ -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,
Expand All @@ -506,6 +503,8 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes
*/
protected function curlExec($curlHandle): string
{
$this->headerLinesMap[(int) $curlHandle] = [];

return curl_exec($curlHandle);
}

Expand Down
41 changes: 24 additions & 17 deletions tests/HTTP/ClientTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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'],
Expand Down Expand Up @@ -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/',
Expand All @@ -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,
Expand All @@ -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',
Expand Down Expand Up @@ -336,8 +336,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']);
Expand All @@ -353,23 +354,23 @@ public function testParseCurlLargeResult()
{
ini_set('memory_limit', '70M');

$header = "HTTP/1.1 200 OK\r\nHeader1: Val1\r\n\r\n";

$client = new ClientMock();
$client->on('curlStuff', function (&$return) use ($header) {
$client->on('curlStuff', function (&$return) {
$return = [
[
'header_size' => strlen($header),
'header_size' => 34,
'http_code' => 200,
],
0,
'',
];
});

$client->receiveCurlHeader(null, "HTTP/1.1 200 OK\r\n");
$client->receiveCurlHeader(null, "Header1: Val1\r\n");

$body = str_repeat('x', 30 * pow(1024, 2));
$response = $header . $body;
$result = $client->parseCurlResult($response, 'foobar');
$result = $client->parseCurlResult($body, 'foobar');

$this->assertEquals(Client::STATUS_SUCCESS, $result['status']);
$this->assertEquals(200, $result['http_code']);
Expand All @@ -388,7 +389,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']);
Expand All @@ -401,12 +402,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,
Expand All @@ -415,7 +415,6 @@ public function testDoRequest()
});
$response = $client->doRequest($request);
$this->assertEquals(200, $response->getStatus());
$this->assertEquals(['Header1' => ['Val1']], $response->getHeaders());
$this->assertEquals('Foo', $response->getBodyAsString());
}

Expand Down Expand Up @@ -448,6 +447,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.
*/
Expand Down

0 comments on commit f68b5d5

Please sign in to comment.