Improve handling of curl proxification. (#15)

- Set all curl proxy options in setCurlProxyOptions. - Remove setting CURLOPT_FOLLOWLOCATION based on whether SOCKS 5 proxy is enabled (can still be set with curloptions in defaults as necessary). - Add support for SOCK5_HOSTNAME proxy.
vufind-org · Jul 1, 2020 · 1813d6d · 1813d6d
1 parent a29cd54
commit 1813d6d
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 9 deletions.
diff --git a/src/VuFindHttp/HttpService.php b/src/VuFindHttp/HttpService.php
@@ -112,6 +112,15 @@ protected function setCurlProxyOptions($adapter)
             $adapter
                 ->setCurlOption(CURLOPT_PROXYPORT, $this->proxyConfig['proxy_port']);
         }
+        // HTTP is default, so handle only the SOCKS 5 proxy types
+        switch ($this->proxyConfig['proxy_type'] ?? '') {
+        case 'socks5':
+            $adapter->setCurlOption(CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
+            break;
+        case 'socks5_hostname':
+            $adapter->setCurlOption(CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5_HOSTNAME);
+            break;
+        }
     }
 
     /**
@@ -132,7 +141,7 @@ protected function hasCurlAdapterAsDefault()
      * Returns the client given as argument with appropriate proxy setup.
      *
      * @param \Laminas\Http\Client $client  HTTP client
-     * @param array                $options ZF2 ProxyAdapter options
+     * @param array                $options Laminas ProxyAdapter options
      *
      * @return \Laminas\Http\Client
      */
@@ -141,15 +150,12 @@ public function proxify(\Laminas\Http\Client $client, array $options = [])
         if ($this->proxyConfig) {
             $host = $client->getUri()->getHost();
             if (!$this->isLocal($host)) {
-                $proxyType =  $this->proxyConfig['proxy_type'] ?? 'default';
+                $proxyType = $this->proxyConfig['proxy_type'] ?? 'default';
 
-                if ($proxyType == 'socks5') {
+                if (in_array($proxyType, ['socks5', 'socks5_hostname'])) {
                     $adapter = new \Laminas\Http\Client\Adapter\Curl();
-                    // Apply standard proxy options for Curl adapter:
+                    // Apply proxy options for Curl adapter:
                     $this->setCurlProxyOptions($adapter);
-                    // Add SOCKS5 settings:
-                    $adapter->setCurlOption(CURLOPT_FOLLOWLOCATION, true);
-                    $adapter->setCurlOption(CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
                     $client->setAdapter($adapter);
                 } elseif ($proxyType == 'default') {
                     // If the user has manually configured a Curl adapter,

diff --git a/tests/unit-tests/src/VuFindTest/HttpServiceTest.php b/tests/unit-tests/src/VuFindTest/HttpServiceTest.php
@@ -159,7 +159,7 @@ public function testGetAppendsHeaders()
                 ),
                 $this->equalTo('1.1'),
                 $this->equalTo(
-                    ['Host' => 'example.tld', 'Connection' => 'close', 'Accept-Encoding' => 'gzip, deflate','User-Agent' => 'Laminas\Http\Client', 'Content-Type' => 'application/json', 'Accept' => 'application/json']
+                    ['Host' => 'example.tld', 'Connection' => 'close', 'Accept-Encoding' => 'gzip, deflate','User-Agent' => 'Laminas_Http_Client', 'Content-Type' => 'application/json', 'Accept' => 'application/json']
                 )
             );
         $service->setDefaultAdapter($adapter);
@@ -185,7 +185,7 @@ public function testPostAppendsHeaders()
                 ),
                 $this->equalTo('1.1'),
                 $this->equalTo(
-                    ['Host' => 'example.tld', 'Connection' => 'close', 'Accept-Encoding' => 'gzip, deflate','User-Agent' => 'Laminas\Http\Client', 'Content-Type' => 'application/json', 'Accept' => 'application/json', 'Content-Length' => '5']
+                    ['Host' => 'example.tld', 'Connection' => 'close', 'Accept-Encoding' => 'gzip, deflate','User-Agent' => 'Laminas_Http_Client', 'Content-Type' => 'application/json', 'Accept' => 'application/json', 'Content-Length' => '5']
                 )
             );
         $service->setDefaultAdapter($adapter);
@@ -305,6 +305,7 @@ public function testProxifySocks5()
         $this->assertEquals(
             CURLPROXY_SOCKS5, $config['curloptions'][CURLOPT_PROXYTYPE]
         );
+        $this->assertNotContains(CURLOPT_FOLLOWLOCATION, $config['curloptions']);
     }
 
     /**
@@ -444,6 +445,49 @@ public function testTimeout()
         $this->assertEquals($clientConfig['timeout'], 67);
     }
 
+    /**
+     * Test defaults with Curl adapter.
+     *
+     * @return void
+     */
+    public function testCurlAdapterDefaults()
+    {
+        $service = new Service(
+            [],
+            ['adapter' => '\Laminas\Http\Client\Adapter\Curl']
+        );
+        $client = $service->createClient('http://example.tld:8080');
+        $adapter = $client->getAdapter();
+        $this->assertInstanceOf('Laminas\Http\Client\Adapter\Curl', $adapter);
+        $config = $adapter->getConfig();
+        $this->assertTrue(empty($config['curloptions']));
+    }
+
+    /**
+     * Test defaults with Curl adapter.
+     *
+     * @return void
+     */
+    public function testCurlAdapterFollowLocation()
+    {
+        $service = new Service(
+            [],
+            [
+                'adapter' => '\Laminas\Http\Client\Adapter\Curl',
+                'curloptions' => [
+                    52 => '1'
+                ]
+            ]
+        );
+        $client = $service->createClient('http://example.tld:8080');
+        $adapter = $client->getAdapter();
+        $this->assertInstanceOf('Laminas\Http\Client\Adapter\Curl', $adapter);
+        $config = $adapter->getConfig();
+        $this->assertEquals(
+            '1', $config['curloptions'][CURLOPT_FOLLOWLOCATION] ?? null
+        );
+    }
+
     /**
      * Return protected or private property.
      *