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 12, 2018
1 parent 0e55cc6 commit 24fbf06
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 31 deletions.
40 changes: 17 additions & 23 deletions lib/Client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,30 @@ class Client extends EventEmitter
*/
protected $maxRedirects = 5;

protected $headerLines = [];

/**
* 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)
{
if ($this->curlHandle !== $curlHandle) {
return;
}
$this->headerLines[] = $headerLine;
return strlen($headerLine);
}

/**
* Sends a request to a HTTP server, and returns a response.
*/
Expand Down Expand Up @@ -414,7 +425,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 +441,18 @@ 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) {
foreach ($this->headerLines as $header) {
$parts = explode(':', $header, 2);
if (2 === count($parts)) {
$response->addHeader(trim($parts[0]), trim($parts[1]));
}
}
$this->headerLines = [];

$response->setBody($responseBody);
$response->setBody($body);

$httpCode = $response->getStatus();

Expand Down Expand Up @@ -506,6 +499,7 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes
*/
protected function curlExec($curlHandle): string
{
$this->headerLines = [];
return curl_exec($curlHandle);
}

Expand Down
27 changes: 19 additions & 8 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 @@ -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']);
Expand Down Expand Up @@ -320,8 +321,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";
$return = "Foo";
});
$client->receiveCurlHeader(null, "HTTP/1.1 200 OK\r\n");
$client->receiveCurlHeader(null, "Header1:Val1\r\n");
$client->on('curlStuff', function (&$return) {
$return = [
[
Expand Down Expand Up @@ -367,6 +370,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 24fbf06

Please sign in to comment.