Skip to content

Fix invalid references in exception stack trace #284

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.

Sign up for GitHub

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

Merged
WyriHaximus merged 2 commits into from
Feb 11, 2022
Merged

Fix invalid references in exception stack trace #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a96220
Fix invalid references in exception stack trace
clue Feb 10, 2022
e01f93d
Clean up unneeded references in test suite
clue Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/DnsConnector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ function ($resolve, $reject) use (&$promise, &$resolved, $uri, $connector, $host

// Exception trace arguments are not available on some PHP 7.4 installs
// @codeCoverageIgnoreStart
foreach ($trace as &$one) {
foreach ($trace as $ti => $one) {
if (isset($one['args'])) {
foreach ($one['args'] as &$arg) {
foreach ($one['args'] as $ai => $arg) {
if ($arg instanceof \Closure) {
$arg = 'Object(' . \get_class($arg) . ')';
$trace[$ti]['args'][$ai] = 'Object(' . \get_class($arg) . ')';
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/SecureConnector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public function connect($uri)
$context = $this->context;
$encryption = $this->streamEncryption;
$connected = false;
/** @var \React\Promise\PromiseInterface $promise */
$promise = $this->connector->connect(
\str_replace('tls://', '', $uri)
)->then(function (ConnectionInterface $connection) use ($context, $encryption, $uri, &$promise, &$connected) {
Expand Down Expand Up @@ -86,11 +87,11 @@ public function connect($uri)

// Exception trace arguments are not available on some PHP 7.4 installs
// @codeCoverageIgnoreStart
foreach ($trace as &$one) {
foreach ($trace as $ti => $one) {
if (isset($one['args'])) {
foreach ($one['args'] as &$arg) {
foreach ($one['args'] as $ai => $arg) {
if ($arg instanceof \Closure) {
$arg = 'Object(' . \get_class($arg) . ')';
$trace[$ti]['args'][$ai] = 'Object(' . \get_class($arg) . ')';
}
}
}
Expand Down
113 changes: 77 additions & 36 deletions tests/DnsConnectorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,18 @@ public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithRuntimeExce
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80')->willReturn($promise);

$promise = $this->connector->connect('1.2.3.4:80');
$promise->cancel();

$this->setExpectedException('RuntimeException', 'Connection to tcp://1.2.3.4:80 failed: Connection failed', 42);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tcp://1.2.3.4:80 failed: Connection failed', $exception->getMessage());
$this->assertEquals(42, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithInvalidArgumentException()
Expand All @@ -105,10 +113,18 @@ public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithInvalidArgu
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80')->willReturn($promise);

$promise = $this->connector->connect('1.2.3.4:80');
$promise->cancel();

$this->setExpectedException('InvalidArgumentException', 'Invalid', 42);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \InvalidArgumentException);
$this->assertInstanceOf('InvalidArgumentException', $exception);
$this->assertEquals('Invalid', $exception->getMessage());
$this->assertEquals(42, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testConnectRejectsWithOriginalHostnameInMessageAfterResolvingIfTcpConnectorRejectsWithRuntimeException()
Expand All @@ -118,10 +134,18 @@ public function testConnectRejectsWithOriginalHostnameInMessageAfterResolvingIfT
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80?hostname=example.com')->willReturn($promise);

$promise = $this->connector->connect('example.com:80');
$promise->cancel();

$this->setExpectedException('RuntimeException', 'Connection to tcp://example.com:80 failed: Connection to tcp://1.2.3.4:80 failed: Connection failed', 42);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tcp://example.com:80 failed: Connection to tcp://1.2.3.4:80 failed: Connection failed', $exception->getMessage());
$this->assertEquals(42, $exception->getCode());
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testConnectRejectsWithOriginalExceptionAfterResolvingIfTcpConnectorRejectsWithInvalidArgumentException()
Expand All @@ -131,10 +155,18 @@ public function testConnectRejectsWithOriginalExceptionAfterResolvingIfTcpConnec
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80?hostname=example.com')->willReturn($promise);

$promise = $this->connector->connect('example.com:80');
$promise->cancel();

$this->setExpectedException('InvalidArgumentException', 'Invalid', 42);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \InvalidArgumentException);
$this->assertInstanceOf('InvalidArgumentException', $exception);
$this->assertEquals('Invalid', $exception->getMessage());
$this->assertEquals(42, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testSkipConnectionIfDnsFails()
Expand All @@ -145,8 +177,17 @@ public function testSkipConnectionIfDnsFails()

$promise = $this->connector->connect('example.invalid:80');

$this->setExpectedException('RuntimeException', 'Connection to tcp://example.invalid:80 failed during DNS lookup: DNS error');
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tcp://example.invalid:80 failed during DNS lookup: DNS error', $exception->getMessage());
$this->assertEquals(0, $exception->getCode());
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testRejectionExceptionUsesPreviousExceptionIfDnsFails()
Expand All @@ -171,12 +212,17 @@ public function testCancelDuringDnsCancelsDnsAndDoesNotStartTcpConnection()
$promise = $this->connector->connect('example.com:80');
$promise->cancel();

$this->setExpectedException(
'RuntimeException',
'Connection to tcp://example.com:80 cancelled during DNS lookup (ECONNABORTED)',
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tcp://example.com:80 cancelled during DNS lookup (ECONNABORTED)', $exception->getMessage());
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testCancelDuringTcpConnectionCancelsTcpConnectionIfGivenIp()
Expand Down Expand Up @@ -216,12 +262,17 @@ public function testCancelDuringTcpConnectionCancelsTcpConnectionWithTcpRejectio

$promise->cancel();

$this->setExpectedException(
'RuntimeException',
'Connection cancelled',
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tcp://example.com:80 failed: Connection cancelled', $exception->getMessage());
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testRejectionDuringDnsLookupShouldNotCreateAnyGarbageReferences()
Expand Down Expand Up @@ -336,14 +387,4 @@ public function testCancelDuringTcpConnectionShouldNotCreateAnyGarbageReferences

$this->assertEquals(0, gc_collect_cycles());
}

private function throwRejection($promise)
{
$ex = null;
$promise->then(null, function ($e) use (&$ex) {
$ex = $e;
});

throw $ex;
}
}
115 changes: 67 additions & 48 deletions tests/SecureConnectorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,18 @@ public function testConnectWillRejectWithTlsUriWhenUnderlyingConnectorRejects()
)));

$promise = $this->connector->connect('example.com:80');
$promise->cancel();

$this->setExpectedException(
'RuntimeException',
'Connection to tls://example.com:80 failed: Connection refused (ECONNREFUSED)',
defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111
);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tls://example.com:80 failed: Connection refused (ECONNREFUSED)', $exception->getMessage());
$this->assertEquals(defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111, $exception->getCode());
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testConnectWillRejectWithOriginalMessageWhenUnderlyingConnectorRejectsWithInvalidArgumentException()
Expand All @@ -98,14 +102,18 @@ public function testConnectWillRejectWithOriginalMessageWhenUnderlyingConnectorR
)));

$promise = $this->connector->connect('example.com:80');
$promise->cancel();

$this->setExpectedException(
'InvalidArgumentException',
'Invalid',
42
);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \InvalidArgumentException);
$this->assertInstanceOf('InvalidArgumentException', $exception);
$this->assertEquals('Invalid', $exception->getMessage());
$this->assertEquals(42, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testCancelDuringTcpConnectionCancelsTcpConnection()
Expand All @@ -128,12 +136,17 @@ public function testCancelDuringTcpConnectionCancelsTcpConnectionAndRejectsWithT
$promise = $this->connector->connect('example.com:80');
$promise->cancel();

$this->setExpectedException(
'RuntimeException',
'Connection to tls://example.com:80 cancelled (ECONNABORTED)',
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tls://example.com:80 cancelled (ECONNABORTED)', $exception->getMessage());
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testConnectionWillBeClosedAndRejectedIfConnectionIsNoStream()
Expand All @@ -145,8 +158,17 @@ public function testConnectionWillBeClosedAndRejectedIfConnectionIsNoStream()

$promise = $this->connector->connect('example.com:80');

$this->setExpectedException('UnexpectedValueException', 'Base connector does not use internal Connection class exposing stream resource');
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \UnexpectedValueException);
$this->assertInstanceOf('UnexpectedValueException', $exception);
$this->assertEquals('Base connector does not use internal Connection class exposing stream resource', $exception->getMessage());
$this->assertEquals(0, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testStreamEncryptionWillBeEnabledAfterConnecting()
Expand All @@ -160,10 +182,9 @@ public function testStreamEncryptionWillBeEnabledAfterConnecting()
$ref->setAccessible(true);
$ref->setValue($this->connector, $encryption);

$pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); });
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));

$promise = $this->connector->connect('example.com:80');
$this->connector->connect('example.com:80');
}

public function testConnectionWillBeRejectedIfStreamEncryptionFailsAndClosesConnection()
Expand All @@ -178,18 +199,21 @@ public function testConnectionWillBeRejectedIfStreamEncryptionFailsAndClosesConn
$ref->setAccessible(true);
$ref->setValue($this->connector, $encryption);

$pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); });
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));

$promise = $this->connector->connect('example.com:80');

try {
$this->throwRejection($promise);
} catch (\RuntimeException $e) {
$this->assertEquals('Connection to tls://example.com:80 failed during TLS handshake: TLS error', $e->getMessage());
$this->assertEquals(123, $e->getCode());
$this->assertNull($e->getPrevious());
}
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tls://example.com:80 failed during TLS handshake: TLS error', $exception->getMessage());
$this->assertEquals(123, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testCancelDuringStreamEncryptionCancelsEncryptionAndClosesConnection()
Expand All @@ -212,12 +236,17 @@ public function testCancelDuringStreamEncryptionCancelsEncryptionAndClosesConnec
$promise = $this->connector->connect('example.com:80');
$promise->cancel();

$this->setExpectedException(
'RuntimeException',
'Connection to tls://example.com:80 cancelled during TLS handshake (ECONNABORTED)',
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
);
$this->throwRejection($promise);
$exception = null;
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

assert($exception instanceof \RuntimeException);
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tls://example.com:80 cancelled during TLS handshake (ECONNABORTED)', $exception->getMessage());
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
$this->assertNull($exception->getPrevious());
$this->assertNotEquals('', $exception->getTraceAsString());
}

public function testRejectionDuringConnectionShouldNotCreateAnyGarbageReferences()
Expand Down Expand Up @@ -267,14 +296,4 @@ public function testRejectionDuringTlsHandshakeShouldNotCreateAnyGarbageReferenc

$this->assertEquals(0, gc_collect_cycles());
}

private function throwRejection($promise)
{
$ex = null;
$promise->then(null, function ($e) use (&$ex) {
$ex = $e;
});

throw $ex;
}
}