From 2d471da92b00caf7ee2e2f58326882ce6a1e2214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 29 Aug 2021 18:04:26 +0200 Subject: [PATCH 1/6] Improve examples to use proper error handlers --- README.md | 4 ++++ examples/11-http-client.php | 4 +++- examples/12-https-client.php | 4 +++- examples/21-netcat-client.php | 8 ++++---- examples/22-http-client.php | 4 +++- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 4c980df0..3b6ec6d4 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,8 @@ $connector = new React\Socket\Connector(); $connector->connect('127.0.0.1:8080')->then(function (React\Socket\ConnectionInterface $connection) { $connection->pipe(new React\Stream\WritableResourceStream(STDOUT)); $connection->write("Hello World!\n"); +}, function (Exception $e) { + echo 'Error: ' . $e->getMessage() . PHP_EOL; }); ``` @@ -905,6 +907,8 @@ $connector = new React\Socket\Connector(); $connector->connect($uri)->then(function (React\Socket\ConnectionInterface $connection) { $connection->write('...'); $connection->end(); +}, function (Exception $e) { + echo 'Error: ' . $e->getMessage() . PHP_EOL; }); ``` diff --git a/examples/11-http-client.php b/examples/11-http-client.php index cdbd7eca..008cfc81 100644 --- a/examples/11-http-client.php +++ b/examples/11-http-client.php @@ -29,4 +29,6 @@ }); $connection->write("GET / HTTP/1.0\r\nHost: $host\r\n\r\n"); -}, 'printf'); +}, function (Exception $e) { + echo 'Error: ' . $e->getMessage() . PHP_EOL; +}); diff --git a/examples/12-https-client.php b/examples/12-https-client.php index d5878935..492dabae 100644 --- a/examples/12-https-client.php +++ b/examples/12-https-client.php @@ -29,4 +29,6 @@ }); $connection->write("GET / HTTP/1.0\r\nHost: $host\r\n\r\n"); -}, 'printf'); +}, function (Exception $e) { + echo 'Error: ' . $e->getMessage() . PHP_EOL; +}); diff --git a/examples/21-netcat-client.php b/examples/21-netcat-client.php index 6b0f9bf4..86014b21 100644 --- a/examples/21-netcat-client.php +++ b/examples/21-netcat-client.php @@ -48,8 +48,8 @@ $connection->pipe($stdout); // report errors to STDERR - $connection->on('error', function ($error) use ($stderr) { - $stderr->write('Stream ERROR: ' . $error . PHP_EOL); + $connection->on('error', function (Exception $e) use ($stderr) { + $stderr->write('Stream error: ' . $e->getMessage() . PHP_EOL); }); // report closing and stop reading from input @@ -59,6 +59,6 @@ }); $stderr->write('Connected' . PHP_EOL); -}, function ($error) use ($stderr) { - $stderr->write('Connection ERROR: ' . $error . PHP_EOL); +}, function (Exception $e) use ($stderr) { + $stderr->write('Connection error: ' . $e->getMessage() . PHP_EOL); }); diff --git a/examples/22-http-client.php b/examples/22-http-client.php index 4d0ddeb2..d7f77fcf 100644 --- a/examples/22-http-client.php +++ b/examples/22-http-client.php @@ -53,4 +53,6 @@ $connection->pipe($stdout); $connection->write("GET $resource HTTP/1.0\r\nHost: $host\r\n\r\n"); -}, 'printf'); +}, function (Exception $e) { + echo 'Error: ' . $e->getMessage() . PHP_EOL; +}); From 2c16a9cccffc29be9a620872e37c555ea488c0ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 29 Aug 2021 19:02:13 +0200 Subject: [PATCH 2/6] Refactor to reuse URI construction --- src/Connector.php | 57 ++++++++++++++++++++++++++ src/DnsConnector.php | 42 +------------------ src/HappyEyeBallsConnectionBuilder.php | 42 +------------------ 3 files changed, 59 insertions(+), 82 deletions(-) diff --git a/src/Connector.php b/src/Connector.php index b500eb3f..02c9561b 100644 --- a/src/Connector.php +++ b/src/Connector.php @@ -175,4 +175,61 @@ public function connect($uri) return $this->connectors[$scheme]->connect($uri); } + + + /** + * [internal] Builds on URI from the given URI parts and ip address with original hostname as query + * + * @param array $parts + * @param string $host + * @param string $ip + * @return string + * @internal + */ + public static function uri(array $parts, $host, $ip) + { + $uri = ''; + + // prepend original scheme if known + if (isset($parts['scheme'])) { + $uri .= $parts['scheme'] . '://'; + } + + if (\strpos($ip, ':') !== false) { + // enclose IPv6 addresses in square brackets before appending port + $uri .= '[' . $ip . ']'; + } else { + $uri .= $ip; + } + + // append original port if known + if (isset($parts['port'])) { + $uri .= ':' . $parts['port']; + } + + // append orignal path if known + if (isset($parts['path'])) { + $uri .= $parts['path']; + } + + // append original query if known + if (isset($parts['query'])) { + $uri .= '?' . $parts['query']; + } + + // append original hostname as query if resolved via DNS and if + // destination URI does not contain "hostname" query param already + $args = array(); + \parse_str(isset($parts['query']) ? $parts['query'] : '', $args); + if ($host !== $ip && !isset($args['hostname'])) { + $uri .= (isset($parts['query']) ? '&' : '?') . 'hostname=' . \rawurlencode($host); + } + + // append original fragment if known + if (isset($parts['fragment'])) { + $uri .= '#' . $parts['fragment']; + } + + return $uri; + } } diff --git a/src/DnsConnector.php b/src/DnsConnector.php index 9d0341b0..d14b12c8 100644 --- a/src/DnsConnector.php +++ b/src/DnsConnector.php @@ -46,47 +46,7 @@ function ($resolve, $reject) use (&$promise, &$resolved, $uri, $connector, $host // resolve/reject with result of DNS lookup $promise->then(function ($ip) use (&$promise, &$resolved, $connector, $host, $parts) { $resolved = $ip; - $uri = ''; - - // prepend original scheme if known - if (isset($parts['scheme'])) { - $uri .= $parts['scheme'] . '://'; - } - - if (\strpos($ip, ':') !== false) { - // enclose IPv6 addresses in square brackets before appending port - $uri .= '[' . $ip . ']'; - } else { - $uri .= $ip; - } - - // append original port if known - if (isset($parts['port'])) { - $uri .= ':' . $parts['port']; - } - - // append orignal path if known - if (isset($parts['path'])) { - $uri .= $parts['path']; - } - - // append original query if known - if (isset($parts['query'])) { - $uri .= '?' . $parts['query']; - } - - // append original hostname as query if resolved via DNS and if - // destination URI does not contain "hostname" query param already - $args = array(); - \parse_str(isset($parts['query']) ? $parts['query'] : '', $args); - if ($host !== $ip && !isset($args['hostname'])) { - $uri .= (isset($parts['query']) ? '&' : '?') . 'hostname=' . \rawurlencode($host); - } - - // append original fragment if known - if (isset($parts['fragment'])) { - $uri .= '#' . $parts['fragment']; - } + $uri = Connector::uri($parts, $host, $ip); return $promise = $connector->connect($uri); }, function ($e) use ($uri, $reject) { diff --git a/src/HappyEyeBallsConnectionBuilder.php b/src/HappyEyeBallsConnectionBuilder.php index 4ec671a3..3333b1c9 100644 --- a/src/HappyEyeBallsConnectionBuilder.php +++ b/src/HappyEyeBallsConnectionBuilder.php @@ -222,47 +222,7 @@ public function check($resolve, $reject) */ public function attemptConnection($ip) { - $uri = ''; - - // prepend original scheme if known - if (isset($this->parts['scheme'])) { - $uri .= $this->parts['scheme'] . '://'; - } - - if (\strpos($ip, ':') !== false) { - // enclose IPv6 addresses in square brackets before appending port - $uri .= '[' . $ip . ']'; - } else { - $uri .= $ip; - } - - // append original port if known - if (isset($this->parts['port'])) { - $uri .= ':' . $this->parts['port']; - } - - // append orignal path if known - if (isset($this->parts['path'])) { - $uri .= $this->parts['path']; - } - - // append original query if known - if (isset($this->parts['query'])) { - $uri .= '?' . $this->parts['query']; - } - - // append original hostname as query if resolved via DNS and if - // destination URI does not contain "hostname" query param already - $args = array(); - \parse_str(isset($this->parts['query']) ? $this->parts['query'] : '', $args); - if ($this->host !== $ip && !isset($args['hostname'])) { - $uri .= (isset($this->parts['query']) ? '&' : '?') . 'hostname=' . \rawurlencode($this->host); - } - - // append original fragment if known - if (isset($this->parts['fragment'])) { - $uri .= '#' . $this->parts['fragment']; - } + $uri = Connector::uri($this->parts, $this->host, $ip); return $this->connector->connect($uri); } From 4e70843091973bd47dcf1190e4200e2aad38b97d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Wed, 1 Sep 2021 18:16:33 +0200 Subject: [PATCH 3/6] Omit internal hostname argument from error messages --- src/DnsConnector.php | 18 ++++++-- src/HappyEyeBallsConnectionBuilder.php | 5 ++- tests/DnsConnectorTest.php | 44 ++++++++++++++++---- tests/HappyEyeBallsConnectionBuilderTest.php | 41 ++++++++++++++++++ 4 files changed, 94 insertions(+), 14 deletions(-) diff --git a/src/DnsConnector.php b/src/DnsConnector.php index d14b12c8..75f7f07c 100644 --- a/src/DnsConnector.php +++ b/src/DnsConnector.php @@ -44,11 +44,23 @@ public function connect($uri) return new Promise\Promise( function ($resolve, $reject) use (&$promise, &$resolved, $uri, $connector, $host, $parts) { // resolve/reject with result of DNS lookup - $promise->then(function ($ip) use (&$promise, &$resolved, $connector, $host, $parts) { + $promise->then(function ($ip) use (&$promise, &$resolved, $uri, $connector, $host, $parts) { $resolved = $ip; - $uri = Connector::uri($parts, $host, $ip); - return $promise = $connector->connect($uri); + return $promise = $connector->connect( + Connector::uri($parts, $host, $ip) + )->then(null, function (\Exception $e) use ($uri) { + if ($e instanceof \RuntimeException) { + $message = \preg_replace('/^(Connection to [^ ]+)[&?]hostname=[^ &]+/', '$1', $e->getMessage()); + $e = new \RuntimeException( + 'Connection to ' . $uri . ' failed: ' . $message, + $e->getCode(), + $e + ); + } + + throw $e; + }); }, function ($e) use ($uri, $reject) { $reject(new \RuntimeException('Connection to ' . $uri .' failed during DNS lookup: ' . $e->getMessage(), 0, $e)); })->then($resolve, $reject); diff --git a/src/HappyEyeBallsConnectionBuilder.php b/src/HappyEyeBallsConnectionBuilder.php index 3333b1c9..6183b177 100644 --- a/src/HappyEyeBallsConnectionBuilder.php +++ b/src/HappyEyeBallsConnectionBuilder.php @@ -175,11 +175,12 @@ public function check($resolve, $reject) $that->failureCount++; + $message = \preg_replace('/^(Connection to [^ ]+)[&?]hostname=[^ &]+/', '$1', $e->getMessage()); if (\strpos($ip, ':') === false) { - $that->lastError4 = $e->getMessage(); + $that->lastError4 = $message; $that->lastErrorFamily = 4; } else { - $that->lastError6 = $e->getMessage(); + $that->lastError6 = $message; $that->lastErrorFamily = 6; } diff --git a/tests/DnsConnectorTest.php b/tests/DnsConnectorTest.php index f6401931..8ef9559a 100644 --- a/tests/DnsConnectorTest.php +++ b/tests/DnsConnectorTest.php @@ -81,29 +81,55 @@ public function testRejectsImmediatelyIfUriIsInvalid() $promise->then($this->expectCallableNever(), $this->expectCallableOnce()); } - public function testRejectsWithTcpConnectorRejectionIfGivenIp() + public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithRuntimeException() { - $promise = Promise\reject(new \RuntimeException('Connection failed')); + $promise = Promise\reject(new \RuntimeException('Connection to tcp://1.2.3.4:80 failed: Connection failed', 42)); $this->resolver->expects($this->never())->method('resolve'); - $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80'))->willReturn($promise); + $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 failed'); + $this->setExpectedException('RuntimeException', 'Connection to tcp://1.2.3.4:80 failed: Connection failed', 42); $this->throwRejection($promise); } - public function testRejectsWithTcpConnectorRejectionAfterDnsIsResolved() + public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithInvalidArgumentException() { - $promise = Promise\reject(new \RuntimeException('Connection failed')); - $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn(Promise\resolve('1.2.3.4')); - $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($promise); + $promise = Promise\reject(new \InvalidArgumentException('Invalid', 42)); + $this->resolver->expects($this->never())->method('resolve'); + $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); + } + + public function testConnectRejectsWithOriginalHostnameInMessageAfterResolvingIfTcpConnectorRejectsWithRuntimeException() + { + $promise = Promise\reject(new \RuntimeException('Connection to tcp://1.2.3.4:80?hostname=example.com failed: Connection failed', 42)); + $this->resolver->expects($this->once())->method('resolve')->with('example.com')->willReturn(Promise\resolve('1.2.3.4')); + $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 example.com:80 failed: Connection to tcp://1.2.3.4:80 failed: Connection failed', 42); + $this->throwRejection($promise); + } + + public function testConnectRejectsWithOriginalExceptionAfterResolvingIfTcpConnectorRejectsWithInvalidArgumentException() + { + $promise = Promise\reject(new \InvalidArgumentException('Invalid', 42)); + $this->resolver->expects($this->once())->method('resolve')->with('example.com')->willReturn(Promise\resolve('1.2.3.4')); + $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 failed'); + $this->setExpectedException('InvalidArgumentException', 'Invalid', 42); $this->throwRejection($promise); } diff --git a/tests/HappyEyeBallsConnectionBuilderTest.php b/tests/HappyEyeBallsConnectionBuilderTest.php index d948b735..1d7f815e 100644 --- a/tests/HappyEyeBallsConnectionBuilderTest.php +++ b/tests/HappyEyeBallsConnectionBuilderTest.php @@ -553,6 +553,47 @@ public function testConnectWillRejectWhenAllConnectionsRejectAndCancelNextAttemp $this->assertEquals('Connection to tcp://reactphp.org:80 failed: Connection refused', $exception->getMessage()); } + public function testConnectWillRejectWithMessageWithoutHostnameWhenAllConnectionsRejectAndCancelNextAttemptTimerImmediately() + { + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(0.1, $this->anything())->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + + $deferred = new Deferred(); + $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); + $connector->expects($this->exactly(2))->method('connect')->willReturnOnConsecutiveCalls( + $deferred->promise(), + \React\Promise\reject(new \RuntimeException('Connection to tcp://127.0.0.1:80?hostname=localhost failed: Connection refused')) + ); + + $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock(); + $resolver->expects($this->exactly(2))->method('resolveAll')->withConsecutive( + array('localhost', Message::TYPE_AAAA), + array('localhost', Message::TYPE_A) + )->willReturnOnConsecutiveCalls( + \React\Promise\resolve(array('::1')), + \React\Promise\resolve(array('127.0.0.1')) + ); + + $uri = 'tcp://localhost:80'; + $host = 'localhost'; + $parts = parse_url($uri); + + $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts); + + $promise = $builder->connect(); + $deferred->reject(new \RuntimeException('Connection to tcp://[::1]:80?hostname=localhost failed: Connection refused')); + + $exception = null; + $promise->then(null, function ($e) use (&$exception) { + $exception = $e; + }); + + $this->assertInstanceOf('RuntimeException', $exception); + $this->assertEquals('Connection to tcp://localhost:80 failed: Last error for IPv4: Connection to tcp://127.0.0.1:80 failed: Connection refused. Previous error for IPv6: Connection to tcp://[::1]:80 failed: Connection refused', $exception->getMessage()); + } + public function testCancelConnectWillRejectPromiseAndCancelBothDnsLookups() { $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); From d77d54af13aef1f0b159b2121f1d376a0670819a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Wed, 1 Sep 2021 18:53:32 +0200 Subject: [PATCH 4/6] Prepend tcp:// scheme for URIs in error messages when missing --- src/DnsConnector.php | 8 +++++--- src/HappyEyeBallsConnector.php | 9 +++++---- tests/DnsConnectorTest.php | 6 +++--- tests/HappyEyeBallsConnectorTest.php | 4 ++-- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/DnsConnector.php b/src/DnsConnector.php index 75f7f07c..265b4aa9 100644 --- a/src/DnsConnector.php +++ b/src/DnsConnector.php @@ -19,15 +19,17 @@ public function __construct(ConnectorInterface $connector, ResolverInterface $re public function connect($uri) { + $original = $uri; if (\strpos($uri, '://') === false) { - $parts = \parse_url('tcp://' . $uri); + $uri = 'tcp://' . $uri; + $parts = \parse_url($uri); unset($parts['scheme']); } else { $parts = \parse_url($uri); } if (!$parts || !isset($parts['host'])) { - return Promise\reject(new \InvalidArgumentException('Given URI "' . $uri . '" is invalid')); + return Promise\reject(new \InvalidArgumentException('Given URI "' . $original . '" is invalid')); } $host = \trim($parts['host'], '[]'); @@ -35,7 +37,7 @@ public function connect($uri) // skip DNS lookup / URI manipulation if this URI already contains an IP if (false !== \filter_var($host, \FILTER_VALIDATE_IP)) { - return $connector->connect($uri); + return $connector->connect($original); } $promise = $this->resolver->resolve($host); diff --git a/src/HappyEyeBallsConnector.php b/src/HappyEyeBallsConnector.php index 677b7c76..f7ea0ecf 100644 --- a/src/HappyEyeBallsConnector.php +++ b/src/HappyEyeBallsConnector.php @@ -31,23 +31,24 @@ public function __construct(LoopInterface $loop = null, ConnectorInterface $conn public function connect($uri) { - + $original = $uri; if (\strpos($uri, '://') === false) { - $parts = \parse_url('tcp://' . $uri); + $uri = 'tcp://' . $uri; + $parts = \parse_url($uri); unset($parts['scheme']); } else { $parts = \parse_url($uri); } if (!$parts || !isset($parts['host'])) { - return Promise\reject(new \InvalidArgumentException('Given URI "' . $uri . '" is invalid')); + return Promise\reject(new \InvalidArgumentException('Given URI "' . $original . '" is invalid')); } $host = \trim($parts['host'], '[]'); // skip DNS lookup / URI manipulation if this URI already contains an IP if (false !== \filter_var($host, \FILTER_VALIDATE_IP)) { - return $this->connector->connect($uri); + return $this->connector->connect($original); } $builder = new HappyEyeBallsConnectionBuilder( diff --git a/tests/DnsConnectorTest.php b/tests/DnsConnectorTest.php index 8ef9559a..16e73fd6 100644 --- a/tests/DnsConnectorTest.php +++ b/tests/DnsConnectorTest.php @@ -116,7 +116,7 @@ public function testConnectRejectsWithOriginalHostnameInMessageAfterResolvingIfT $promise = $this->connector->connect('example.com:80'); $promise->cancel(); - $this->setExpectedException('RuntimeException', 'Connection to example.com:80 failed: Connection to tcp://1.2.3.4:80 failed: Connection failed', 42); + $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); } @@ -141,7 +141,7 @@ public function testSkipConnectionIfDnsFails() $promise = $this->connector->connect('example.invalid:80'); - $this->setExpectedException('RuntimeException', 'Connection to example.invalid:80 failed during DNS lookup: DNS error'); + $this->setExpectedException('RuntimeException', 'Connection to tcp://example.invalid:80 failed during DNS lookup: DNS error'); $this->throwRejection($promise); } @@ -167,7 +167,7 @@ public function testCancelDuringDnsCancelsDnsAndDoesNotStartTcpConnection() $promise = $this->connector->connect('example.com:80'); $promise->cancel(); - $this->setExpectedException('RuntimeException', 'Connection to example.com:80 cancelled during DNS lookup'); + $this->setExpectedException('RuntimeException', 'Connection to tcp://example.com:80 cancelled during DNS lookup'); $this->throwRejection($promise); } diff --git a/tests/HappyEyeBallsConnectorTest.php b/tests/HappyEyeBallsConnectorTest.php index b661d5b2..d44b8625 100644 --- a/tests/HappyEyeBallsConnectorTest.php +++ b/tests/HappyEyeBallsConnectorTest.php @@ -256,7 +256,7 @@ public function testSkipConnectionIfDnsFails() $that->throwRejection($promise); }); - $this->setExpectedException('RuntimeException', 'Connection to example.invalid:80 failed during DNS lookup: DNS error'); + $this->setExpectedException('RuntimeException', 'Connection to tcp://example.invalid:80 failed during DNS lookup: DNS error'); $this->loop->run(); } @@ -275,7 +275,7 @@ public function testCancelDuringDnsCancelsDnsAndDoesNotStartTcpConnection() $that->throwRejection($promise); }); - $this->setExpectedException('RuntimeException', 'Connection to example.com:80 cancelled during DNS lookup'); + $this->setExpectedException('RuntimeException', 'Connection to tcp://example.com:80 cancelled during DNS lookup'); $this->loop->run(); } From f561c4ed391933d7798463be6939476c803fc6db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Thu, 2 Sep 2021 12:05:37 +0200 Subject: [PATCH 5/6] Use tls:// scheme for URIs in error messages for secure connections --- src/SecureConnector.php | 17 ++++++++++++++--- tests/FunctionalConnectorTest.php | 4 ++-- tests/SecureConnectorTest.php | 30 ++++++++++++++++++++++++++---- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/SecureConnector.php b/src/SecureConnector.php index 0e6bd7c3..56e2c9aa 100644 --- a/src/SecureConnector.php +++ b/src/SecureConnector.php @@ -37,12 +37,12 @@ public function connect($uri) return Promise\reject(new \InvalidArgumentException('Given URI "' . $uri . '" is invalid')); } - $uri = \str_replace('tls://', '', $uri); $context = $this->context; - $encryption = $this->streamEncryption; $connected = false; - $promise = $this->connector->connect($uri)->then(function (ConnectionInterface $connection) use ($context, $encryption, $uri, &$promise, &$connected) { + $promise = $this->connector->connect( + \str_replace('tls://', '', $uri) + )->then(function (ConnectionInterface $connection) use ($context, $encryption, $uri, &$promise, &$connected) { // (unencrypted) TCP/IP connection succeeded $connected = true; @@ -66,6 +66,17 @@ public function connect($uri) $error->getCode() ); }); + }, function (\Exception $e) use ($uri) { + if ($e instanceof \RuntimeException) { + $message = \preg_replace('/^Connection to [^ ]+/', '', $e->getMessage()); + $e = new \RuntimeException( + 'Connection to ' . $uri . $message, + $e->getCode(), + $e + ); + } + + throw $e; }); return new \React\Promise\Promise( diff --git a/tests/FunctionalConnectorTest.php b/tests/FunctionalConnectorTest.php index 6b2f1cb6..f591295a 100644 --- a/tests/FunctionalConnectorTest.php +++ b/tests/FunctionalConnectorTest.php @@ -144,10 +144,10 @@ public function testCancelPendingTlsConnectionDuringTlsHandshakeShouldCloseTcpCo $loop = Factory::create(); $server = new TcpServer(0, $loop); - $uri = str_replace('tcp://', '', $server->getAddress()); + $uri = str_replace('tcp://', 'tls://', $server->getAddress()); $connector = new Connector(array(), $loop); - $promise = $connector->connect('tls://' . $uri); + $promise = $connector->connect($uri); $deferred = new Deferred(); $server->on('connection', function (ConnectionInterface $connection) use ($promise, $deferred, $loop) { diff --git a/tests/SecureConnectorTest.php b/tests/SecureConnectorTest.php index 66491696..1e0f4f87 100644 --- a/tests/SecureConnectorTest.php +++ b/tests/SecureConnectorTest.php @@ -68,6 +68,28 @@ public function testConnectionToInvalidSchemeWillReject() $promise->then(null, $this->expectCallableOnce()); } + public function testConnectWillRejectWithTlsUriWhenUnderlyingConnectorRejects() + { + $this->tcp->expects($this->once())->method('connect')->with('example.com:80')->willReturn(\React\Promise\reject(new \RuntimeException('Connection to tcp://example.com:80 failed: Connection refused', 42))); + + $promise = $this->connector->connect('example.com:80'); + $promise->cancel(); + + $this->setExpectedException('RuntimeException', 'Connection to tls://example.com:80 failed: Connection refused', 42); + $this->throwRejection($promise); + } + + public function testConnectWillRejectWithOriginalMessageWhenUnderlyingConnectorRejectsWithInvalidArgumentException() + { + $this->tcp->expects($this->once())->method('connect')->with('example.com:80')->willReturn(\React\Promise\reject(new \InvalidArgumentException('Invalid', 42))); + + $promise = $this->connector->connect('example.com:80'); + $promise->cancel(); + + $this->setExpectedException('InvalidArgumentException', 'Invalid', 42); + $this->throwRejection($promise); + } + public function testCancelDuringTcpConnectionCancelsTcpConnection() { $pending = new Promise\Promise(function () { }, $this->expectCallableOnce()); @@ -79,13 +101,13 @@ public function testCancelDuringTcpConnectionCancelsTcpConnection() public function testCancelDuringTcpConnectionCancelsTcpConnectionAndRejectsWithTcpRejection() { - $pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); }); + $pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection to tcp://example.com:80 cancelled', 42); }); $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->will($this->returnValue($pending)); $promise = $this->connector->connect('example.com:80'); $promise->cancel(); - $this->setExpectedException('RuntimeException', 'Connection cancelled'); + $this->setExpectedException('RuntimeException', 'Connection to tls://example.com:80 cancelled', 42); $this->throwRejection($promise); } @@ -139,7 +161,7 @@ public function testConnectionWillBeRejectedIfStreamEncryptionFailsAndClosesConn try { $this->throwRejection($promise); } catch (\RuntimeException $e) { - $this->assertEquals('Connection to example.com:80 failed during TLS handshake: TLS error', $e->getMessage()); + $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()); } @@ -165,7 +187,7 @@ public function testCancelDuringStreamEncryptionCancelsEncryptionAndClosesConnec $promise = $this->connector->connect('example.com:80'); $promise->cancel(); - $this->setExpectedException('RuntimeException', 'Connection to example.com:80 cancelled during TLS handshake'); + $this->setExpectedException('RuntimeException', 'Connection to tls://example.com:80 cancelled during TLS handshake'); $this->throwRejection($promise); } From 115097f9ce959852f8e1556fd055136aaa74b5cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 3 Sep 2021 09:00:34 +0200 Subject: [PATCH 6/6] Avoid garbage memory references on PHP < 7.4 --- src/DnsConnector.php | 20 ++++++++++++++++++++ src/SecureConnector.php | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/DnsConnector.php b/src/DnsConnector.php index 265b4aa9..b68d7ea6 100644 --- a/src/DnsConnector.php +++ b/src/DnsConnector.php @@ -59,6 +59,26 @@ function ($resolve, $reject) use (&$promise, &$resolved, $uri, $connector, $host $e->getCode(), $e ); + + // avoid garbage references by replacing all closures in call stack. + // what a lovely piece of code! + $r = new \ReflectionProperty('Exception', 'trace'); + $r->setAccessible(true); + $trace = $r->getValue($e); + + // Exception trace arguments are not available on some PHP 7.4 installs + // @codeCoverageIgnoreStart + foreach ($trace as &$one) { + if (isset($one['args'])) { + foreach ($one['args'] as &$arg) { + if ($arg instanceof \Closure) { + $arg = 'Object(' . \get_class($arg) . ')'; + } + } + } + } + // @codeCoverageIgnoreEnd + $r->setValue($e, $trace); } throw $e; diff --git a/src/SecureConnector.php b/src/SecureConnector.php index 56e2c9aa..e6e85c4d 100644 --- a/src/SecureConnector.php +++ b/src/SecureConnector.php @@ -74,6 +74,26 @@ public function connect($uri) $e->getCode(), $e ); + + // avoid garbage references by replacing all closures in call stack. + // what a lovely piece of code! + $r = new \ReflectionProperty('Exception', 'trace'); + $r->setAccessible(true); + $trace = $r->getValue($e); + + // Exception trace arguments are not available on some PHP 7.4 installs + // @codeCoverageIgnoreStart + foreach ($trace as &$one) { + if (isset($one['args'])) { + foreach ($one['args'] as &$arg) { + if ($arg instanceof \Closure) { + $arg = 'Object(' . \get_class($arg) . ')'; + } + } + } + } + // @codeCoverageIgnoreEnd + $r->setValue($e, $trace); } throw $e;