ZF-11663: Zend_Oauth_Client ignores GET parameters that are set via setParameterGet

Description

Adding the following line (in file Zend/Oauth/Client.php in function prepareOauth()):

$params = array_merge($params, $this->paramsGet);

just before:

if (!empty($this->paramsPost)) {

fixed the bug for me.

Comments

The issue here is that {{Zend_Oauth_Client}} takes the GET parameters directly from the URI provided:


## Zend_Oauth_Client, Lines 265-273
$query = $this->getUri()->getQuery();
if ($query) {
    $queryParts = explode('&', $this->getUri()->getQuery());
    foreach ($queryParts as $queryPart) {
        $kvTuple = explode('=', $queryPart);
        $params[urldecode($kvTuple[0])] =
            (array_key_exists(1, $kvTuple) ? urldecode($kvTuple[1]) : null);
    }
}

So, we have two sources of GET parameters. That begs the question: Which takes precedence, the ones set in the URI or the ones set directly in the client?

After talking with [~padraic] on IRC, it was decided that URI parameters should take precedence over those set by the client using {{setParameterGet}}.

Reproducing test case:


Index: tests/Zend/Oauth/OauthTest.php
===================================================================
--- tests/Zend/Oauth/OauthTest.php      (revision 24413)
+++ tests/Zend/Oauth/OauthTest.php      (working copy)
@@ -183,4 +183,43 @@
         $this->assertNotContains('realm=""',$client->getHeader('Authorization'));
         $this->assertContains('realm="someRealm"',$client->getHeader('Authorization'));
     }
+
+    /**
+     * @group ZF-11663
+     */
+    public function testOauthClientAcceptsGetParametersThroughSetter()
+    {
+        require_once "Zend/Oauth/Token/Access.php";
+        $token = new Zend_Oauth_Token_Access();
+
+        $options = array(
+            'requestMethod' => 'GET',
+            'requestScheme' => Zend_Oauth::REQUEST_SCHEME_QUERYSTRING,
+            'realm'                    => 'someRealm'
+        );
+
+        require_once 'Zend/Oauth/Client.php';
+        $client = new Zend_Oauth_Client($options);
+        $client->setToken($token);
+        $client->setUri('http://www.example.com/?test=FooBar');
+        $queryString = $client->getUri()->getQuery();
+
+        // Check that query string was set properly
+        $this->assertSame('test=FooBar', $queryString);
+
+        // Change the GET parameters
+        $client->setParameterGet('test', 'FooBaz');
+        $client->setParameterGet('second', 'TestTest');
+
+        // Prepare the OAuth request
+        $client->prepareOauth();
+        $queryString = $client->getUri()->getQuery();
+
+        // Ensure that parameter 'test' is unchanged, as URI parameters
+        // should take precedence over ones set with setParameterGet
+        $this->assertContains('test=FooBar', $queryString);
+
+        // Ensure that new parameter was added
+        $this->assertContains('second=TestTest', $queryString);
+    }
 }

Fix:


Index: library/Zend/Oauth/Client.php
===================================================================
--- library/Zend/Oauth/Client.php       (revision 24413)
+++ library/Zend/Oauth/Client.php       (working copy)
@@ -261,7 +261,7 @@
             $this->setRawData($raw, 'application/x-www-form-urlencoded');
             $this->paramsPost = array();
         } elseif ($requestScheme == Zend_Oauth::REQUEST_SCHEME_QUERYSTRING) {
-            $params = array();
+            $params = $this->paramsGet;
             $query = $this->getUri()->getQuery();
             if ($query) {
                 $queryParts = explode('&', $this->getUri()->getQuery());

After applying fix, reproducing test case passes.

Fixed in trunk r24414 Merged to release-1.11 in r24415

GitHub pull request issued against {{padraic/zf2}} branch {{zend-oauth-cleanup}} https://github.com/padraic/zf2/pull/1