clue · clue · Nov 27, 2016 · Nov 17, 2016 · Nov 27, 2016
diff --git a/README.md b/README.md
@@ -73,6 +73,7 @@ $dnsResolverFactory = new DnsFactory();
 $resolver = $dnsResolverFactory->createCached('127.0.0.1', $loop);
 
 // outgoing connections to SOCKS server via interface 192.168.10.1
+// this is not to be confused with local DNS resolution (see further below)
 $connector = new DnsConnector(
     new TcpConnector($loop, array('bindto' => '192.168.10.1:0')),
     $resolver
@@ -220,6 +221,10 @@ You can also explicitly set the protocol version later:
 $client->setProtocolVersion('4a');
 ```
 
+As seen above, both SOCKS5 and SOCKS4a support remote and local DNS resolution.
+If you've explicitly set this to SOCKS4, then you may want to check the following
+chapter about local DNS resolution or you may only connect to IPv4 addresses.
+
 In order to reset the protocol version to its default (i.e. automatic detection),
 use `null` as protocol version.
 
@@ -229,32 +234,58 @@ $client->setProtocolVersion(null);
 
 #### DNS resolution
 
-By default, the `Client` uses local DNS resolving to resolve target hostnames
-into IP addresses and only transmits the resulting target IP to the socks server.
+By default, the `Client` does not perform any DNS resolution at all and simply
+forwards any hostname you're trying to connect to to the SOCKS server.
+The remote SOCKS server is thus responsible for looking up any hostnames via DNS
+(this default mode is thus called *remote DNS resolution*).
+As seen above, this mode is supported by the SOCKS5 and SOCKS4a protocols, but
+not the SOCKS4 protocol, as the protocol lacks a way to communicate hostnames.
+
+On the other hand, all SOCKS protocol versions support sending destination IP
+addresses to the SOCKS server.
+In this mode you either have to stick to using IPs only (which is ofen unfeasable)
+or perform any DNS lookups locally and only transmit the resolved destination IPs
+(this mode is thus called *local DNS resolution*).
+
+The default *remote DNS resolution* is useful if your local `Client` either can
+not resolve target hostnames because it has no direct access to the internet or
+if it should not resolve target hostnames because its outgoing DNS traffic might
+be intercepted (in particular when using the
+[Tor network](#using-the-tor-anonymity-network-to-tunnel-socks-connections)).
 
-Resolving locally usually results in better performance as for each outgoing
-request both resolving the hostname and initializing the connection to the
-SOCKS server can be done simultanously. So by the time the SOCKS connection is
-established (requires a TCP handshake for each connection), the target hostname
-will likely already be resolved ( _usually_ either already cached or requires a
-simple DNS query via UDP).
+If you want to explicitly use *local DNS resolution* (such as when explicitly
+using SOCKS4), you can use the following code:
 
-You may want to switch to remote DNS resolving if your local `Client` either can not
-resolve target hostnames because it has no direct access to the internet or if
-it should not resolve target hostnames because its outgoing DNS traffic might
-be intercepted (in particular when using the
-[Tor network](#using-the-tor-anonymity-network-to-tunnel-socks-connections)). 
+```php
+// usual client setup
+$client = new Client($uri, $loop);
 
-Local DNS resolving is available in all SOCKS protocol versions.
-Remote DNS resolving is only available for SOCKS4a and SOCKS5
-(i.e. it is NOT available for SOCKS4).
+// set up DNS server to use (Google's public DNS here)
+$factory = new React\Dns\Resolver\Factory();
+$resolver = $factory->createCached('8.8.8.8', $loop);
 
-Valid values are boolean `true`(default) or `false`.
+// resolve hostnames via DNS before forwarding resulting IP trough SOCKS server
+$dns = new React\SocketClient\DnsConnector($client->createConnector(), $resolver);
 
-```PHP
-$client->setResolveLocal(false);
+// secure TLS via the DNS connector
+$ssl = new React\SocketClient\SecureConnector($dns, $loop);
+
+$ssl->create('www.google.com', 443)->then(function ($stream) {
+    // …
+});
 ```
 
+See also the [fourth example](examples).
+
+> Also note how local DNS resolution is in fact entirely handled outside of this
+SOCKS client implementation.
+
+If you've explicitly set the client to SOCKS4 and stick to the default
+*remote DNS resolution*, then you may only connect to IPv4 addresses because
+the protocol lacks a way to communicate hostnames.
+If you try to connect to a hostname despite, the resulting promise will be
+rejected right away.
+
 #### Authentication
 
 This library supports username/password authentication for SOCKS5 servers as

diff --git a/examples/01-http.php b/examples/01-http.php
@@ -11,7 +11,6 @@
 
 $client = new Client('127.0.0.1:' . $port, $loop);
 $client->setTimeout(3.0);
-$client->setResolveLocal(false);
 
 echo 'Demo SOCKS client connecting to SOCKS server 127.0.0.1:' . $port . PHP_EOL;
 echo 'Not already running a SOCKS server? Try this: ssh -D ' . $port . ' localhost' . PHP_EOL;

diff --git a/examples/02-https.php b/examples/02-https.php
@@ -11,7 +11,6 @@
 
 $client = new Client('127.0.0.1:' . $port, $loop);
 $client->setTimeout(3.0);
-$client->setResolveLocal(false);
 
 echo 'Demo SOCKS client connecting to SOCKS server 127.0.0.1:' . $port . PHP_EOL;
 echo 'Not already running a SOCKS server? Try this: ssh -D ' . $port . ' localhost' . PHP_EOL;

diff --git a/examples/04-local-dns.php b/examples/04-local-dns.php
@@ -0,0 +1,38 @@
+<?php
+
+use React\Stream\Stream;
+use Clue\React\Socks\Client;
+use React\Dns\Resolver\Factory;
+use React\SocketClient\DnsConnector;
+use React\SocketClient\SecureConnector;
+
+include_once __DIR__.'/../vendor/autoload.php';
+
+$port = isset($argv[1]) ? $argv[1] : 9050;
+
+$loop = React\EventLoop\Factory::create();
+
+$client = new Client('127.0.0.1:' . $port, $loop);
+$client->setTimeout(3.0);
+
+// set up DNS server to use (Google's public DNS)
+$factory = new Factory();
+$resolver = $factory->createCached('8.8.8.8', $loop);
+
+// resolve hostnames via DNS before forwarding resulting IP trough SOCKS server
+$dns = new DnsConnector($client->createConnector(), $resolver);
+
+echo 'Demo SOCKS client connecting to SOCKS server 127.0.0.1:' . $port . PHP_EOL;
+echo 'Not already running a SOCKS server? Try this: ssh -D ' . $port . ' localhost' . PHP_EOL;
+
+$ssl = new SecureConnector($dns, $loop);
+
+$ssl->create('www.google.com', 443)->then(function (Stream $stream) {
+    echo 'connected' . PHP_EOL;
+    $stream->write("GET / HTTP/1.0\r\n\r\n");
+    $stream->on('data', function ($data) {
+        echo $data;
+    });
+}, 'printf');
+
+$loop->run();
diff --git a/src/Client.php b/src/Client.php
@@ -27,12 +27,6 @@ class Client
      */
     private $connector;
 
-    /**
-     *
-     * @var Resolver
-     */
-    private $resolver;
-
     private $socksHost;
 
     private $socksPort;
@@ -44,8 +38,6 @@ class Client
      */
     protected $loop;
 
-    private $resolveLocal = true;
-
     private $protocolVersion = null;
 
     protected $auth = null;
@@ -90,7 +82,6 @@ public function __construct($socksUri, LoopInterface $loop, ConnectorInterface $
         $this->socksHost = $parts['host'];
         $this->socksPort = $parts['port'];
         $this->connector = $connector;
-        $this->resolver = $resolver;
 
         $this->timeout = ini_get("default_socket_timeout");
     }
@@ -100,14 +91,6 @@ public function setTimeout($timeout)
         $this->timeout = $timeout;
     }
 
-    public function setResolveLocal($resolveLocal)
-    {
-        if ($this->protocolVersion === '4' && !$resolveLocal) {
-            throw new UnexpectedValueException('SOCKS4 requires resolving locally. Consider using another protocol version or resolving locally');
-        }
-        $this->resolveLocal = $resolveLocal;
-    }
-
     public function setProtocolVersion($version)
     {
         if ($version !== null) {
@@ -118,9 +101,6 @@ public function setProtocolVersion($version)
             if ($version !== '5' && $this->auth){
                 throw new UnexpectedValueException('Unable to change protocol version to anything but SOCKS5 while authentication is used. Consider removing authentication info or sticking to SOCKS5');
             }
-            if ($version === '4' && !$this->resolveLocal) {
-                throw new UnexpectedValueException('Unable to change to SOCKS4 while resolving locally is turned off. Consider using another protocol version or resolving locally');
-            }
         }
         $this->protocolVersion = $version;
     }
@@ -192,6 +172,10 @@ public function createSecureConnector(array $sslContext = array())
      */
     public function createConnection($host, $port)
     {
+        if ($this->protocolVersion === '4' && false === filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
+            return Promise\reject(new InvalidArgumentException('Requires an IPv4 address for SOCKS4'));
+        }
+
         if (strlen($host) > 255 || $port > 65535 || $port < 0) {
             $deferred = new Deferred();
             $deferred->reject(new InvalidArgumentException('Invalid target specified'));
@@ -221,37 +205,29 @@ public function createConnection($host, $port)
 
         $that = $this;
 
-        // simultaneously start TCP connection and DNS resolution
+        // start TCP/IP connection to SOCKS server
         $connecting = $this->connect($this->socksHost, $this->socksPort);
-        $resolving = $this->resolve($host);
 
+        // handle SOCKS protocol once connection is ready
         $handling = $connecting->then(
-            function (Stream $stream) use ($that, $resolving, $loop, $timerTimeout, $protocolVersion, $auth, $timestampTimeout, $port) {
-                // connection established, wait for DNS resolver
-                return $resolving->then(
-                    function ($host) use ($stream, $port, $timestampTimeout, $that, $loop, $timerTimeout, $protocolVersion, $auth) {
-                        // DNS resolver completed => cancel timeout
-                        $loop->cancelTimer($timerTimeout);
-
-                        $timeout = max($timestampTimeout - microtime(true), 0.1);
-                        return $that->handleConnectedSocks($stream, $host, $port, $timeout, $protocolVersion, $auth);
-                    },
-                    function (Exception $error) {
-                        throw new Exception('Unable to resolve remote hostname', 0, $error);
-                    }
-                );
+            function (Stream $stream) use ($that, $loop, $timerTimeout, $protocolVersion, $auth, $timestampTimeout, $host, $port) {
+                // connection established => cancel timeout
+                $loop->cancelTimer($timerTimeout);
+
+                $timeout = max($timestampTimeout - microtime(true), 0.1);
+                return $that->handleConnectedSocks($stream, $host, $port, $timeout, $protocolVersion, $auth);
             },
             function (Exception $error) {
                 throw new Exception('Unable to connect to socks server', 0, $error);
             }
         );
 
+        // resolve plain connection once SOCKS protocol is completed
         $handling->then(array($deferred, 'resolve'), array($deferred, 'reject'));
 
-        return $deferred->promise()->then(null, function (Exception $error) use ($connecting, $resolving, $handling, $loop, $timerTimeout) {
-            // cancel pending connection, DNS lookup, SOCKS handling and timeout timer
+        return $deferred->promise()->then(null, function (Exception $error) use ($connecting, $handling, $loop, $timerTimeout) {
+            // cancel pending connection, SOCKS handling and timeout timer
             $connecting->cancel();
-            $resolving->cancel();
             $handling->cancel();
             $loop->cancelTimer($timerTimeout);
 
@@ -290,34 +266,6 @@ function ($_, $reject) use ($promise) {
         );
     }
 
-    private function resolve($host)
-    {
-        // return if it's already an IP or we want to resolve remotely (socks 4 only supports resolving locally)
-        if (false !== filter_var($host, FILTER_VALIDATE_IP) || ($this->protocolVersion !== '4' && !$this->resolveLocal)) {
-            $deferred = new Deferred();
-            $deferred->resolve($host);
-            return $deferred->promise();
-        }
-
-        $promise = $this->resolver->resolve($host);
-
-        return new Promise\Promise(
-            function ($resolve, $reject) use ($promise) {
-                // resolve/reject with result of DNS lookup
-                $promise->then($resolve, $reject);
-            },
-            function ($_, $reject) use ($promise) {
-                // cancellation should reject DNS resolution
-                $reject(new RuntimeException('Connection attempt cancelled during DNS lookup'));
-
-                // (try to) cancel pending DNS connection
-                if ($promise instanceof CancellablePromiseInterface) {
-                    $promise->cancel();
-                }
-            }
-        );
-    }
-
     /**
      * Internal helper used to handle the communication with the SOCKS server
      *