From ef22bcff4fa9a9f33775d05bae492bb966ac2f27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 9 Jun 2017 14:08:10 +0200 Subject: [PATCH] Fix rejecting invalid URIs and unexpected URI schemes --- README.md | 19 ++++++++------- src/ProxyConnector.php | 7 ++++-- tests/FunctionalTest.php | 2 +- tests/ProxyConnectorTest.php | 46 +++++++++++++++++++++++++++++++++--- 4 files changed, 59 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 75a0d7c..a194055 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,7 @@ to the proxy server address: ```php $connector = new Connector($loop); -$proxy = new ProxyConnector('127.0.0.1:8080', $connector); +$proxy = new ProxyConnector('http://127.0.0.1:8080', $connector); ``` The proxy URL may or may not contain a scheme and port definition. The default @@ -119,7 +119,7 @@ higher-level component: ```diff - $client = new SomeClient($connector); -+ $proxy = new ProxyConnector('127.0.0.1:8080', $connector); ++ $proxy = new ProxyConnector('http://127.0.0.1:8080', $connector); + $client = new SomeClient($proxy); ``` @@ -133,7 +133,7 @@ The `ProxyConnector` implements the [`ConnectorInterface`](#connectorinterface) hence provides a single public method, the [`connect()`](#connect) method. ```php -$proxy = new ProxyConnector('127.0.0.1:8080', $connector); +$proxy = new ProxyConnector('http://127.0.0.1:8080', $connector); $proxy->connect('tcp://smtp.googlemail.com:587')->then(function (ConnectionInterface $stream) { $stream->write("EHLO local\r\n"); @@ -171,7 +171,7 @@ your destination, you may want to wrap this connector in React's [`SecureConnector`](https://github.com/reactphp/socket#secureconnector): ```php -$proxy = new ProxyConnector('127.0.0.1:8080', $connector); +$proxy = new ProxyConnector('http://127.0.0.1:8080', $connector); $connector = new Connector($loop, array( 'tcp' => $proxy, 'dns' => false @@ -274,16 +274,17 @@ unencrypted, plain TCP/IP HTTP connection. Note that this is the most common setup, because you can still establish a TLS connection between you and the destination host as above. -If you want to connect to a (rather rare) HTTPS proxy, you may want use its -HTTPS port (443) and use a +If you want to connect to a (rather rare) HTTPS proxy, you may want use the +`https://` scheme (HTTPS default port 443) and use React's +[`Connector`](https://github.com/reactphp/socket#connector) or the low-level [`SecureConnector`](https://github.com/reactphp/socket#secureconnector) instance to create a secure connection to the proxy: ```php -$ssl = new SecureConnector($connector, $loop); -$proxy = new ProxyConnector('127.0.0.1:443', $ssl); +$connector = new Connector($loop); +$proxy = new ProxyConnector('https://127.0.0.1:443', $connector); -$proxy->connect('smtp.googlemail.com:587'); +$proxy->connect('tcp://smtp.googlemail.com:587'); ``` ## Install diff --git a/src/ProxyConnector.php b/src/ProxyConnector.php index f8e27be..d99e265 100644 --- a/src/ProxyConnector.php +++ b/src/ProxyConnector.php @@ -7,6 +7,7 @@ use InvalidArgumentException; use RuntimeException; use RingCentral\Psr7; +use React\Promise; use React\Promise\Deferred; use React\Socket\ConnectionInterface; @@ -60,16 +61,18 @@ public function __construct($proxyUrl, ConnectorInterface $connector) } $parts = parse_url($proxyUrl); - if (!$parts || !isset($parts['scheme'], $parts['host'])) { + if (!$parts || !isset($parts['scheme'], $parts['host']) || ($parts['scheme'] !== 'http' && $parts['scheme'] !== 'https')) { throw new InvalidArgumentException('Invalid proxy URL'); } + // apply default port and TCP/TLS transport for given scheme if (!isset($parts['port'])) { $parts['port'] = $parts['scheme'] === 'https' ? 443 : 80; } + $parts['scheme'] = $parts['scheme'] === 'https' ? 'tls' : 'tcp'; $this->connector = $connector; - $this->proxyUri = $parts['host'] . ':' . $parts['port']; + $this->proxyUri = $parts['scheme'] . '://' . $parts['host'] . ':' . $parts['port']; } public function connect($uri) diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php index 65b0a88..e7e8ae6 100644 --- a/tests/FunctionalTest.php +++ b/tests/FunctionalTest.php @@ -44,7 +44,7 @@ public function testSecureGoogleDoesNotAcceptConnectMethod() } $secure = new SecureConnector($this->dnsConnector, $this->loop); - $proxy = new ProxyConnector('google.com:443', $secure); + $proxy = new ProxyConnector('https://google.com:443', $secure); $promise = $proxy->connect('google.com:80'); diff --git a/tests/ProxyConnectorTest.php b/tests/ProxyConnectorTest.php index a97e3f6..7e45d0f 100644 --- a/tests/ProxyConnectorTest.php +++ b/tests/ProxyConnectorTest.php @@ -23,20 +23,38 @@ public function testInvalidProxy() new ProxyConnector('///', $this->connector); } + /** + * @expectedException InvalidArgumentException + */ + public function testInvalidProxyScheme() + { + new ProxyConnector('ftp://example.com', $this->connector); + } + public function testCreatesConnectionToHttpPort() { $promise = new Promise(function () { }); - $this->connector->expects($this->once())->method('connect')->with('proxy.example.com:80?hostname=google.com')->willReturn($promise); + $this->connector->expects($this->once())->method('connect')->with('tcp://proxy.example.com:80?hostname=google.com')->willReturn($promise); $proxy = new ProxyConnector('proxy.example.com', $this->connector); $proxy->connect('google.com:80'); } + public function testCreatesConnectionToHttpPortAndPassesThroughUriComponents() + { + $promise = new Promise(function () { }); + $this->connector->expects($this->once())->method('connect')->with('tcp://proxy.example.com:80/path?foo=bar&hostname=google.com#segment')->willReturn($promise); + + $proxy = new ProxyConnector('proxy.example.com', $this->connector); + + $proxy->connect('google.com:80/path?foo=bar#segment'); + } + public function testCreatesConnectionToHttpPortAndObeysExplicitHostname() { $promise = new Promise(function () { }); - $this->connector->expects($this->once())->method('connect')->with('proxy.example.com:80?hostname=www.google.com')->willReturn($promise); + $this->connector->expects($this->once())->method('connect')->with('tcp://proxy.example.com:80?hostname=www.google.com')->willReturn($promise); $proxy = new ProxyConnector('proxy.example.com', $this->connector); @@ -46,7 +64,7 @@ public function testCreatesConnectionToHttpPortAndObeysExplicitHostname() public function testCreatesConnectionToHttpsPort() { $promise = new Promise(function () { }); - $this->connector->expects($this->once())->method('connect')->with('proxy.example.com:443?hostname=google.com')->willReturn($promise); + $this->connector->expects($this->once())->method('connect')->with('tls://proxy.example.com:443?hostname=google.com')->willReturn($promise); $proxy = new ProxyConnector('https://proxy.example.com', $this->connector); @@ -80,6 +98,28 @@ public function testWillWriteToOpenConnection() $proxy->connect('google.com:80'); } + public function testRejectsInvalidUri() + { + $this->connector->expects($this->never())->method('connect'); + + $proxy = new ProxyConnector('proxy.example.com', $this->connector); + + $promise = $proxy->connect('///'); + + $promise->then(null, $this->expectCallableOnce()); + } + + public function testRejectsUriWithNonTcpScheme() + { + $this->connector->expects($this->never())->method('connect'); + + $proxy = new ProxyConnector('proxy.example.com', $this->connector); + + $promise = $proxy->connect('tls://google.com:80'); + + $promise->then(null, $this->expectCallableOnce()); + } + public function testRejectsAndClosesIfStreamWritesNonHttp() { $stream = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('close', 'write'))->getMock();