Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use array to sort params instead of object #15

Closed
wants to merge 1 commit into from

Conversation

osminushkin
Copy link
Contributor

Sorting of params changed to use array instead of object according to explanation in issue #11, point 2

@osminushkin
Copy link
Contributor Author

Hi @paymentwall!
Could you take a look please?

@liufanhhh
Copy link
Contributor

Hi Osminushkin,

We will look this with our colleague.

Thank you so much for your help :)

@liufanhhh
Copy link
Contributor

Hi Osminushkin,

Could you please help to advise in what case the Sort Function will broken?

As we tested, it worked well.

Thanks,
Fan

@osminushkin
Copy link
Contributor Author

Hi!

According to the standard there is no guarantee of iteration order for objects.

var ns = {};
ns['b'] = 'b';
ns['a'] = 'a';
ns['2'] = '2';
ns['1'] = '1';
for (k in ns) {
  console.log(k);
}

The script above will print
1 2 b a

Of course, this is an abstract example just to show the case :)

@liufanhhh
Copy link
Contributor

Hi,

But in the sort object code we use this:

    for (key in o) {
        if (o.hasOwnProperty(key)) {
            a.push(key);
        }
    }

    a.sort();

In this case, we will sort the keys no matter if they are in order.

Are you referring this part?

@osminushkin
Copy link
Contributor Author

in sortObject you defined sorted to store sorted key-value and array a to store the keys from object and sort them.

You pushed the keys to the array and sorted it:

for (key in o) {
    if (o.hasOwnProperty(key)) {
        a.push(key);
    }
}

a.sort();

but next you converted this array back to an object

for (key = 0; key < a.length; key++) {
  sorted[a[key]] = o[a[key]];
}

return sorted;

in pingback.js when calculating signature:

params = this.sortObject(params);
...
for (key in params) {
    ...
}

The thing is that you are sorting the array, but then iterating over the object and there is no guarantee of order of iterations, which you are trying to get by sorting

@liufanhhh
Copy link
Contributor

Understand , thank you so much for helping!

We will fix this issue in next version :)

@liufanhhh
Copy link
Contributor

Please refer to issue 11.

@liufanhhh liufanhhh closed this Dec 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants