Zend Framework

Zend_Oauth_Client ignores GET parameters that are set via setParameterGet

Details

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.

Activity

Hide
Adam Lundrigan added a comment -

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?

Show
Adam Lundrigan added a comment - 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?
Hide
Adam Lundrigan added a comment -

After talking with Pádraic Brady 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.

Show
Adam Lundrigan added a comment - After talking with Pádraic Brady 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.
Hide
Adam Lundrigan added a comment -

Fixed in trunk r24414
Merged to release-1.11 in r24415

Show
Adam Lundrigan added a comment - Fixed in trunk r24414 Merged to release-1.11 in r24415
Hide
Adam Lundrigan added a comment -

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

Show
Adam Lundrigan added a comment - GitHub pull request issued against padraic/zf2 branch zend-oauth-cleanup https://github.com/padraic/zf2/pull/1

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: