Merge pull request #74 from clue-labs/socks5-default

Clients defaults to SOCKS5 instead of SOCKS4a

README.md

@@ -304,17 +304,15 @@ application protocols to work through it.
 Note, this is __not__ a full SOCKS5 implementation due to missing GSSAPI
 authentication (but it's unlikely you're going to miss it anyway).
 
-By default, the `Client` communicates via SOCKS4a with the SOCKS server
-– unless you enable [authentication](#authentication), in which case it will
-default to SOCKS5.
-This is done because SOCKS4a incurs less overhead than SOCKS5 (see above) and
-is equivalent with SOCKS4 if you use [local DNS resolution](#dns-resolution).
+By default, the `Client` communicates via SOCKS5 with the SOCKS server.
+This is done because SOCKS5 is the latest version from the SOCKS protocol family
+and generally has best support across other vendors.
 
 If want to explicitly set the protocol version, use the supported values URI
-schemes `socks4`, `socks4a` or `socks5` as part of the SOCKS URI:
+schemes `socks4://` or `socks4a://`as part of the SOCKS URI:
 
 ```php
-$client = new Client('socks5://127.0.0.1', $connector);
+$client = new Client('socks4a://127.0.0.1', $connector);
 ```
 
 As seen above, both SOCKS5 and SOCKS4a support remote and local DNS resolution.

src/Client.php

@@ -22,7 +22,7 @@ class Client implements ConnectorInterface
 
     private $socksUri;
 
-    private $protocolVersion = null;
+    private $protocolVersion = '5';
 
     private $auth = null;
 
@@ -59,10 +59,7 @@ public function __construct($socksUri, ConnectorInterface $connector)
 
         // user or password in URI => SOCKS5 authentication
         if (isset($parts['user']) || isset($parts['pass'])) {
-            if ($parts['scheme'] === 'socks') {
-                // default to using SOCKS5 if not given explicitly
-                $parts['scheme'] = 'socks5';
-            } elseif ($parts['scheme'] !== 'socks5') {
+            if ($parts['scheme'] !== 'socks' && $parts['scheme'] !== 'socks5') {
                 // fail if any other protocol version given explicitly
                 throw new InvalidArgumentException('Authentication requires SOCKS5. Consider using protocol version 5 or waive authentication');
             }
@@ -79,10 +76,10 @@ public function __construct($socksUri, ConnectorInterface $connector)
 
     private function setProtocolVersionFromScheme($scheme)
     {
-        if ($scheme === 'socks' || $scheme === 'socks4a') {
-            $this->protocolVersion = '4a';
-        } elseif ($scheme === 'socks5') {
+        if ($scheme === 'socks' || $scheme === 'socks5') {
             $this->protocolVersion = '5';
+        } elseif ($scheme === 'socks4a') {
+            $this->protocolVersion = '4a';
         } elseif ($scheme === 'socks4') {
             $this->protocolVersion = '4';
         } else {

tests/FunctionalTest.php

@@ -95,6 +95,14 @@ public function testConnectionSocks5()
         $this->assertResolveStream($this->client->connect('www.google.com:80'));
     }
 
+    /** @group internet */
+    public function testConnectionDefaultsToSocks5()
+    {
+        $this->server->setProtocolVersion(5);
+
+        $this->assertResolveStream($this->client->connect('www.google.com:80'));
+    }
+
     /** @group internet */
     public function testConnectionSocksOverTls()
     {