RichardSisonFerrer / oauth

Automatically exported from code.google.com/p/oauth
0 stars 0 forks source link

invalid sort order when two or more parameters share the same name #164

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
[this bug is in reference to the PHP library]

The specification describes that they are sorted by their value, if two or 
more parameters share the same name.
OAuth.php returns invalid result by natsort.

1. set query parameters like below.

id=1&id=21&id=124&id=123

2. natsort() returns below.

id=1&id=21&id=123&id=124

What is the expected output? What do you see instead?

id=1&id=123&id=124&id=21

What version of the product are you using? On what operating system?

trunk (r1171)

Please provide any additional information below.

This is a patch for this problem.

--- OAuth.php.org       2010-05-17 17:25:38.000000000 +0900
+++ OAuth.php   2010-05-17 17:27:12.000000000 +0900
@@ -857,7 +857,7 @@
       if (is_array($value)) {
         // If two or more parameters share the same name, they are sorted 
by their value
         // Ref: Spec: 9.1.1 (1)
-        natsort($value);
+        sort($value, SORT_STRING);
         foreach ($value as $duplicate_value) {
           $pairs[] = $parameter . '=' . $duplicate_value;
         }

Original issue reported on code.google.com by hidet...@gmail.com on 17 May 2010 at 8:50

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hi,
Have you verified that your change doesn't break any of the existing unit 
tests? 
And could you provide a unit test that fails with the current code, but passes 
with your patch, with a ref. to why 
the results should be as follows.. (The method you want to add your test to is 
probably 
OAuthRequestTest::testNormalizeParameters)
-Morten

Original comment by morten.f...@gmail.com on 28 May 2010 at 7:09

GoogleCodeExporter commented 9 years ago
Thanks for your reply.

The sort order must be "lexicographical byte value ordering".
Please see the below discussion.
http://groups.google.com/group/oauth/browse_thread/thread/7c698004be0d536/dced7b
6c82b917b2?lnk=gst&q=sort#

I added a test for this.

--- OAuthUtilTest.php   (revision 1227)
+++ OAuthUtilTest.php   (working copy)
@@ -99,6 +99,10 @@
                        'a=1&c=hi%20there&f=25&f=50&f=a&z=p&z=t',
                        OAuthUtil::build_http_query(array('a'=>'1', 'c' =>'hi there', 'f'=>array(25, 50, 'a'), 'z'=>array('p','t')))
                );
+               $this->assertEquals(
+                       'x=200&x=25&y=B&y=a',
+                       OAuthUtil::build_http_query(array('x'=>array(25, 200), 
'y'=>array('a', 'B')))
+               );
        }

Original comment by hidet...@gmail.com on 9 Jun 2010 at 9:28

GoogleCodeExporter commented 9 years ago
Patches added as of revision 1230. Thanks for reporting and creating patches.

Original comment by morten.f...@gmail.com on 12 Jun 2010 at 8:09