Merge pull request #13 from clue-labs/invalid-uris

Fix rejecting invalid URIs and unexpected URI schemes

Author: clue

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

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)

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');
 

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();