Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance, reuse server params for same connection #467

Merged
merged 1 commit into from Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 51 additions & 33 deletions src/Io/RequestHeaderParser.php
Expand Up @@ -27,6 +27,9 @@ class RequestHeaderParser extends EventEmitter
/** @var Clock */
private $clock;

/** @var array<string|int,array<string,string>> */
private $connectionParams = array();

public function __construct(Clock $clock)
{
$this->clock = $clock;
Expand Down Expand Up @@ -66,8 +69,7 @@ public function handle(ConnectionInterface $conn)
try {
$request = $that->parseRequest(
(string)\substr($buffer, 0, $endOfHeader + 2),
$conn->getRemoteAddress(),
$conn->getLocalAddress()
$conn
);
} catch (Exception $exception) {
$buffer = '';
Expand Down Expand Up @@ -119,13 +121,12 @@ public function handle(ConnectionInterface $conn)

/**
* @param string $headers buffer string containing request headers only
* @param ?string $remoteSocketUri
* @param ?string $localSocketUri
* @param ConnectionInterface $connection
* @return ServerRequestInterface
* @throws \InvalidArgumentException
* @internal
*/
public function parseRequest($headers, $remoteSocketUri, $localSocketUri)
public function parseRequest($headers, ConnectionInterface $connection)
{
// additional, stricter safe-guard for request line
// because request parser doesn't properly cope with invalid ones
Expand Down Expand Up @@ -160,26 +161,59 @@ public function parseRequest($headers, $remoteSocketUri, $localSocketUri)
}
}

// reuse same connection params for all server params for this connection
$cid = \PHP_VERSION_ID < 70200 ? \spl_object_hash($connection) : \spl_object_id($connection);
if (isset($this->connectionParams[$cid])) {
$serverParams = $this->connectionParams[$cid];
} else {
// assign new server params for new connection
$serverParams = array();

// scheme is `http` unless TLS is used
$localSocketUri = $connection->getLocalAddress();
$localParts = $localSocketUri === null ? array() : \parse_url($localSocketUri);
if (isset($localParts['scheme']) && $localParts['scheme'] === 'tls') {
$serverParams['HTTPS'] = 'on';
}

// apply SERVER_ADDR and SERVER_PORT if server address is known
// address should always be known, even for Unix domain sockets (UDS)
// but skip UDS as it doesn't have a concept of host/port.
if ($localSocketUri !== null && isset($localParts['host'], $localParts['port'])) {
$serverParams['SERVER_ADDR'] = $localParts['host'];
$serverParams['SERVER_PORT'] = $localParts['port'];
}

// apply REMOTE_ADDR and REMOTE_PORT if source address is known
// address should always be known, unless this is over Unix domain sockets (UDS)
$remoteSocketUri = $connection->getRemoteAddress();
if ($remoteSocketUri !== null) {
$remoteAddress = \parse_url($remoteSocketUri);
$serverParams['REMOTE_ADDR'] = $remoteAddress['host'];
$serverParams['REMOTE_PORT'] = $remoteAddress['port'];
}

// remember server params for all requests from this connection, reset on connection close
$this->connectionParams[$cid] = $serverParams;
$params =& $this->connectionParams;
$connection->on('close', function () use (&$params, $cid) {
assert(\is_array($params));
unset($params[$cid]);
});
}

// create new obj implementing ServerRequestInterface by preserving all
// previous properties and restoring original request-target
$serverParams = array(
'REQUEST_TIME' => (int) ($now = $this->clock->now()),
'REQUEST_TIME_FLOAT' => $now
);
$serverParams['REQUEST_TIME'] = (int) ($now = $this->clock->now());
$serverParams['REQUEST_TIME_FLOAT'] = $now;

// scheme is `http` unless TLS is used
$localParts = $localSocketUri === null ? array() : \parse_url($localSocketUri);
if (isset($localParts['scheme']) && $localParts['scheme'] === 'tls') {
$scheme = 'https://';
$serverParams['HTTPS'] = 'on';
} else {
$scheme = 'http://';
}
$scheme = isset($serverParams['HTTPS']) ? 'https://' : 'http://';

// default host if unset comes from local socket address or defaults to localhost
$hasHost = $host !== null;
if ($host === null) {
$host = isset($localParts['host'], $localParts['port']) ? $localParts['host'] . ':' . $localParts['port'] : '127.0.0.1';
$host = isset($serverParams['SERVER_ADDR'], $serverParams['SERVER_PORT']) ? $serverParams['SERVER_ADDR'] . ':' . $serverParams['SERVER_PORT'] : '127.0.0.1';
}

if ($start['method'] === 'OPTIONS' && $start['target'] === '*') {
Expand Down Expand Up @@ -210,22 +244,6 @@ public function parseRequest($headers, $remoteSocketUri, $localSocketUri)
}
}

// apply REMOTE_ADDR and REMOTE_PORT if source address is known
// address should always be known, unless this is over Unix domain sockets (UDS)
if ($remoteSocketUri !== null) {
$remoteAddress = \parse_url($remoteSocketUri);
$serverParams['REMOTE_ADDR'] = $remoteAddress['host'];
$serverParams['REMOTE_PORT'] = $remoteAddress['port'];
}

// apply SERVER_ADDR and SERVER_PORT if server address is known
// address should always be known, even for Unix domain sockets (UDS)
// but skip UDS as it doesn't have a concept of host/port.
if ($localSocketUri !== null && isset($localParts['host'], $localParts['port'])) {
$serverParams['SERVER_ADDR'] = $localParts['host'];
$serverParams['SERVER_PORT'] = $localParts['port'];
}

$request = new ServerRequest(
$start['method'],
$uri,
Expand Down
58 changes: 57 additions & 1 deletion tests/Io/RequestHeaderParserTest.php
Expand Up @@ -2,9 +2,9 @@

namespace React\Tests\Http\Io;

use Psr\Http\Message\ServerRequestInterface;
use React\Http\Io\RequestHeaderParser;
use React\Tests\Http\TestCase;
use Psr\Http\Message\ServerRequestInterface;

class RequestHeaderParserTest extends TestCase
{
Expand Down Expand Up @@ -808,6 +808,62 @@ public function testServerParamsWontBeSetOnMissingUrls()
$this->assertArrayNotHasKey('REMOTE_PORT', $serverParams);
}

public function testServerParamsWillBeReusedForMultipleRequestsFromSameConnection()
{
$clock = $this->getMockBuilder('React\Http\Io\Clock')->disableOriginalConstructor()->getMock();
$clock->expects($this->exactly(2))->method('now')->willReturn(1652972091.3958);

$parser = new RequestHeaderParser($clock);

$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('getLocalAddress', 'getRemoteAddress'))->getMock();
$connection->expects($this->once())->method('getLocalAddress')->willReturn('tcp://127.1.1.1:8000');
$connection->expects($this->once())->method('getRemoteAddress')->willReturn('tcp://192.168.1.1:8001');

$parser->handle($connection);
$connection->emit('data', array("GET /foo HTTP/1.0\r\nHost: example.com\r\n\r\n"));

$request = null;
$parser->on('headers', function ($parsedRequest) use (&$request) {
$request = $parsedRequest;
});

$parser->handle($connection);
$connection->emit('data', array("GET /foo HTTP/1.0\r\nHost: example.com\r\n\r\n"));

assert($request instanceof ServerRequestInterface);
$serverParams = $request->getServerParams();

$this->assertArrayNotHasKey('HTTPS', $serverParams);
$this->assertEquals(1652972091, $serverParams['REQUEST_TIME']);
$this->assertEquals(1652972091.3958, $serverParams['REQUEST_TIME_FLOAT']);

$this->assertEquals('127.1.1.1', $serverParams['SERVER_ADDR']);
$this->assertEquals('8000', $serverParams['SERVER_PORT']);

$this->assertEquals('192.168.1.1', $serverParams['REMOTE_ADDR']);
$this->assertEquals('8001', $serverParams['REMOTE_PORT']);
}

public function testServerParamsWillBeRememberedUntilConnectionIsClosed()
{
$clock = $this->getMockBuilder('React\Http\Io\Clock')->disableOriginalConstructor()->getMock();

$parser = new RequestHeaderParser($clock);

$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('getLocalAddress', 'getRemoteAddress'))->getMock();

$parser->handle($connection);
$connection->emit('data', array("GET /foo HTTP/1.0\r\nHost: example.com\r\n\r\n"));

$ref = new \ReflectionProperty($parser, 'connectionParams');
$ref->setAccessible(true);

$this->assertCount(1, $ref->getValue($parser));

$connection->emit('close');
$this->assertEquals(array(), $ref->getValue($parser));
}

public function testQueryParmetersWillBeSet()
{
$request = null;
Expand Down