From 4a96220cc040a76c07f77783db68ac2958cfff88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Thu, 10 Feb 2022 10:59:36 +0100 Subject: [PATCH 1/2] Fix invalid references in exception stack trace --- src/DnsConnector.php | 6 +++--- src/SecureConnector.php | 6 +++--- tests/DnsConnectorTest.php | 31 ++++++++++++++++++++++--------- tests/SecureConnectorTest.php | 35 ++++++++++++++++++++++------------- 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/DnsConnector.php b/src/DnsConnector.php index 27fc8f8b..29523ebf 100644 --- a/src/DnsConnector.php +++ b/src/DnsConnector.php @@ -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) . ')'; } } } diff --git a/src/SecureConnector.php b/src/SecureConnector.php index 03c6e361..eba8c9d6 100644 --- a/src/SecureConnector.php +++ b/src/SecureConnector.php @@ -86,11 +86,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) . ')'; } } } diff --git a/tests/DnsConnectorTest.php b/tests/DnsConnectorTest.php index 2dbc4020..63b2efa5 100644 --- a/tests/DnsConnectorTest.php +++ b/tests/DnsConnectorTest.php @@ -118,10 +118,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() @@ -216,12 +224,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() diff --git a/tests/SecureConnectorTest.php b/tests/SecureConnectorTest.php index af3a6f58..39d82913 100644 --- a/tests/SecureConnectorTest.php +++ b/tests/SecureConnectorTest.php @@ -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() @@ -128,12 +132,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() From e01f93dd70626ad7ca3773cf90f1873d0e54d574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Tue, 8 Feb 2022 09:49:58 +0100 Subject: [PATCH 2/2] Clean up unneeded references in test suite --- src/SecureConnector.php | 1 + tests/DnsConnectorTest.php | 82 +++++++++++++++++++++++------------ tests/SecureConnectorTest.php | 80 +++++++++++++++++++--------------- 3 files changed, 101 insertions(+), 62 deletions(-) diff --git a/src/SecureConnector.php b/src/SecureConnector.php index eba8c9d6..a5087ca0 100644 --- a/src/SecureConnector.php +++ b/src/SecureConnector.php @@ -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) { diff --git a/tests/DnsConnectorTest.php b/tests/DnsConnectorTest.php index 63b2efa5..5d37a237 100644 --- a/tests/DnsConnectorTest.php +++ b/tests/DnsConnectorTest.php @@ -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() @@ -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() @@ -139,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() @@ -153,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() @@ -179,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() @@ -349,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; - } } diff --git a/tests/SecureConnectorTest.php b/tests/SecureConnectorTest.php index 39d82913..591012e5 100644 --- a/tests/SecureConnectorTest.php +++ b/tests/SecureConnectorTest.php @@ -102,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() @@ -154,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() @@ -169,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() @@ -187,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() @@ -221,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() @@ -276,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; - } }