Issues

ZF-3189: Zend_Http_Client_Adapter_Proxy::connectHandshake ignores useragent set in headers

Issue Type: Bug Created: 2008-05-02T12:09:51.000+0000 Last Updated: 2012-05-28T19:07:33.000+0000 Status: Resolved Fix version(s): - 1.12.0 (27/Aug/12)

Reporter: Chris Abernethy (brownoxford) Assignee: Adam Lundrigan (adamlundrigan) Tags: - Zend_Gdata

  • FixForZF1.12
  • zf-caretaker-adamlundrigan
  • zf-crteam-padraic
  • zf-crteam-priority

Related issues: Attachments: - ZF-3189.patch

Description

If the user-agent is specified in the headers, as opposed to via setConfig(), the default UA of Zend_Http_Client is used for CONNECT requests by the proxy adapter.

I'm not too familiar with proxy servers, but there could be issues if the CONNECT request uses one UA and the actual request uses another. It seems like setting the UA via setHeaders should overwrite the useragent config item.

Comments

Posted by Wil Sinclair (wil) on 2008-06-09T13:32:44.000+0000

Please evaluate and fix/categorize as necessary.

Posted by Ralph Schindler (ralph) on 2011-02-17T15:10:19.000+0000

Do you have any code to demonstrate this issue?

Posted by Adam Lundrigan (adamlundrigan) on 2011-06-07T02:34:34.000+0000

In tracing through the code, I noticed the following: Zend_Http_Client::request() calls Zend_Http_Client::_prepareHeaders(), which returns a numerically-indexed array of headers in 'name: value' format. That array is then passed to the adapter via an argument to it's connect() method. However, Zend_Http_Client_Adapter_Proxy::connect() treats $headers as an associative array of HeaderName=>HeaderValue pairs.

Did I miss something there, or is that actually the case? If it is, then Zend_Http_Client_Adapter_Proxy needs to be reworked to reflect that, as it uses $headers as an associative array in many places.

Posted by Adam Lundrigan (adamlundrigan) on 2011-06-07T03:06:07.000+0000

It looks to me that the culprit is here:

<pre class="highlight">
// Add the user-agent header
if (isset($this->config['useragent'])) {
    $request .= "User-agent: " . $this->config['useragent'] . "\r\n";
}

The config key 'useragent' always exists, as it has a default defined in Zend_Http_Client. In the connectHandshake() snippet above, the User-agent header will be set up with that default value (Zend_Http_Client) regardless of what might be set in $headers, as none of those headers are sent during the CONNECT handshake. As the original poster says, this may cause the CONNECT handshake and the subsequent request to have different UAs.

Posted by Adam Lundrigan (adamlundrigan) on 2011-10-04T15:31:23.000+0000

Test to reproduce the issue:

<pre class="highlight">
Index: tests/Zend/Http/Client/ProxyAdapterTest.php
===================================================================
--- tests/Zend/Http/Client/ProxyAdapterTest.php (revision 24490)
+++ tests/Zend/Http/Client/ProxyAdapterTest.php (working copy)
@@ -118,4 +118,73 @@
          * the TRACE response
          */
     }
+
+    /**
+     * @group ZF-3189
+     */
+    public function testConnectHandshakeSendsCustomUserAgentHeader()
+    {
+        // Change the adapter
+        $this->config['adapter'] = 'ZF3189_ProxyAdapter';
+        $this->config['useragent'] = 'ZendTest';
+        parent::setUp();
+
+        $base = preg_replace("/^http:/", "https:", $this->baseuri);
+        $this->client->setUri($base . 'testSimpleRequests.php');
+
+        // Ensure we're proxying a HTTPS request
+        $this->assertEquals('https', $this->client->getUri()->getScheme());
+
+        // Perform the request
+        $this->client->request();
+
+        $this->assertContains(
+            "User-agent: {$this->config['useragent']}",
+            $this->client->getAdapter()->getLastConnectHandshakeRequest()
+        );
+    }
+
+    /**
+     * @group ZF-3189
+     */
+    public function testConnectHandshakeSendsCustomUserAgentHeaderWhenSetInHeaders()
+    {
+        // Change the adapter
+        $this->config['adapter'] = 'ZF3189_ProxyAdapter';
+        parent::setUp();
+
+        $base = preg_replace("/^http:/", "https:", $this->baseuri);
+        $this->client->setUri($base . 'testSimpleRequests.php');
+        $this->client->setHeaders('User-Agent', 'ZendTest');
+
+        // Ensure we're proxying a HTTPS request
+        $this->assertEquals('https', $this->client->getUri()->getScheme());
+
+        // Perform the request
+        $this->client->request();
+        print_r($this->client->getAdapter()->getLastConnectHandshakeRequest());
+        $this->assertContains(
+            "User-agent: ZendTest",
+            $this->client->getAdapter()->getLastConnectHandshakeRequest()
+        );
+    }
+
 }
+
+/**
+ * Exposes internal variable connectHandshakeRequest for test purposes
+ * @see ZF-3189
+ */
+class ZF3189_ProxyAdapter extends Zend_Http_Client_Adapter_Proxy
+{
+
+    /**
+     * Retrieve the request data from last CONNECT handshake
+     * @return string
+     */
+    public function getLastConnectHandshakeRequest()
+    {
+        return $this->connectHandshakeRequest;
+    }
+
+}
\ No newline at end of file
Index: library/Zend/Http/Client/Adapter/Proxy.php
===================================================================
--- library/Zend/Http/Client/Adapter/Proxy.php  (revision 24490)
+++ library/Zend/Http/Client/Adapter/Proxy.php  (working copy)
@@ -75,6 +75,13 @@
      * @var boolean
      */
     protected $negotiated = false;
+
+    /**
+     * Stores the last CONNECT handshake request
+     *
+     * @var string
+     */
+    protected $connectHandshakeRequest;

     /**
      * Connect to the remote server
@@ -217,6 +224,9 @@
         }

         $request .= "\r\n";
+
+        // @see ZF-3189
+        $this->connectHandshakeRequest = $request;

         // Send the request
         if (! @fwrite($this->socket, $request)) {

The first test (testConnectHandshakeSendsCustomUserAgentHeader) will pass, as setting the user agent in the client configuration works as expected. However, the second test (testConnectHandshakeSendsCustomUserAgentHeaderWhenSetInHeaders) currently fails for the reason the OP suggested: connectHandshake does not check to see if $headers} defines a user agent header.

My reproduction test required a slight modification to Zend_Http_Client_Adapter_Proxy so that it would record the request headers sent during the connectHandshake method call. Rather than change the public API of Zend_Http_Client_Adapter_Proxy, I added a protected variable to store the request, then created a test-specific adapter class (ZF3189_ProxyAdapter) to expose it.

Posted by Adam Lundrigan (adamlundrigan) on 2011-10-04T15:56:14.000+0000

Suggested fix:

<pre class="highlight">
Index: library/Zend/Http/Client/Adapter/Proxy.php
===================================================================
--- library/Zend/Http/Client/Adapter/Proxy.php  (revision 24490)
+++ library/Zend/Http/Client/Adapter/Proxy.php  (working copy)
@@ -136,10 +136,20 @@
         }

         // Add Proxy-Authorization header
-        if ($this->config['proxy_user'] && ! isset($headers['proxy-authorization'])) {
-            $headers['proxy-authorization'] = Zend_Http_Client::encodeAuthHeader(
-                $this->config['proxy_user'], $this->config['proxy_pass'], $this->config['proxy_auth']
-            );
+        if ($this->config['proxy_user']) {
+            // Check to see if one already exists
+            $hasProxyAuthHeader = false;
+            foreach ($headers as $k => $v) {
+                if ($k == 'proxy-authorization' || preg_match("/^proxy-authorization:/i", $v) ) {
+                    $hasProxyAuthHeader = true;
+                    break;
+                }
+            }
+            if (!$hasProxyAuthHeader) {
+                $headers[] = 'Proxy-authorization: ' . Zend_Http_Client::encodeAuthHeader(
+                    $this->config['proxy_user'], $this->config['proxy_pass'], $this->config['proxy_auth']
+                );
+            }
         }

         // if we are proxying HTTPS, preform CONNECT handshake with the proxy
@@ -204,18 +214,18 @@
         $request = "CONNECT $host:$port HTTP/$http_ver\r\n" .
                    "Host: " . $this->config['proxy_host'] . "\r\n";

-        // Add the user-agent header
-        if (isset($this->config['useragent'])) {
-            $request .= "User-agent: " . $this->config['useragent'] . "\r\n";
+        // Process provided headers, including important ones to CONNECT request
+        foreach ( $headers as $k=>$v ) {
+            switch ( strtolower(substr($v,0,strpos($v,':'))) ) {
+                case 'proxy-authorization':
+                    // break intentionally omitted
+                case 'user-agent':
+                    $request .= $v . "\r\n";
+                    break;
+                default:
+                    break;
+            }
         }
-
-        // If the proxy-authorization header is set, send it to proxy but remove
-        // it from headers sent to target host
-        if (isset($headers['proxy-authorization'])) {
-            $request .= "Proxy-authorization: " . $headers['proxy-authorization'] . "\r\n";
-            unset($headers['proxy-authorization']);
-        }
-
         $request .= "\r\n";

         // Send the request

Two things here: * Rejig the authorization header processing Zend_Http_Client_Adapter_Proxy::write to properly detect an existing Proxy-Authorization header (old method assumed that $headers was a header=>value associative array, which it isn't. * Update Zend_Http_Client_Adapter_Proxy::connectHandshake to pass User-Agent and Proxy-Authorization headers through to CONNECT request.

I've also added a third test to enforce that Zend_Http_Client_Adapter_Proxy::write does indeed properly detect a preexisting Proxy-Authorization header:

<pre class="highlight">
    /**
     * @group ZF-3189
     */
    public function testProxyAdapterDoesNotOverwriteExistingProxyAuthorizationHeader()
    {      
        // Change the adapter
        $this->config['adapter'] = 'ZF3189_ProxyAdapter';
        parent::setUp();
        
        $base = preg_replace("/^http:/", "https:", $this->baseuri);
        $this->client->setUri($base . 'testSimpleRequests.php');
        $this->client->setHeaders('Proxy-Authorization', 'FooBarBaz');

        // Ensure we're proxying a HTTPS request
        $this->assertEquals('https', $this->client->getUri()->getScheme());
        
        // Perform the request
        $this->client->request();
        print_r($this->client->getAdapter()->getLastConnectHandshakeRequest());
        
        $resp = $this->client->getAdapter()->getLastConnectHandshakeRequest();
        $this->assertEquals(1, preg_match_all('/\r\nProxy-Authorization: ([^\r\n]+)\r\n/i', $resp, $matches));
        $this->assertEquals('FooBarBaz', $matches[1][0]);
    }

Thoughts?

Posted by Adam Lundrigan (adamlundrigan) on 2012-05-12T19:40:31.000+0000

I would like to have this fixed in ZF 1.12.0. Could someone with a proxy setup already in place test out this patch to ensure it works as expected?

Posted by Rob Allen (rob) on 2012-05-28T18:50:04.000+0000

Fixed in svn r24818.

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts