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

don't re-order multiple values for the same key with post_form #125

Open
neilb opened this issue Jul 15, 2019 · 3 comments
Open

don't re-order multiple values for the same key with post_form #125

neilb opened this issue Jul 15, 2019 · 3 comments
Labels

Comments

@neilb
Copy link

neilb commented Jul 15, 2019

I'm using an API that can take multiple strings, and returns a result for each string, in the same order as the strings were provided.

Initially I was doing this:

my $form_data = { 'q[]' => \@strings };
my $response  = $ua->post_form($url, $form_data, $headers);

But I found that depending on the strings, I wasn't getting the responses back in the expected order.

I just realised that the strings are being sorted, because of this line in www_form_urlencode():

return join("&", (ref $data eq 'ARRAY') ? (@terms) : (sort @terms) );

So for my case, I can fix it by doing this:

my $form_data = [ 'q[]' => \@strings ];

But I think that for the law of least surprise, I think a better behaviour would be to sort on the keys, but within a key to preserve the order.

@neilb
Copy link
Author

neilb commented Jul 16, 2019

Here's one way to achieve that. In www_form_urlencode():

Change this line:

my @params = ref $data eq 'HASH' ? %$data : @$data;

To:

my @params = ref $data eq 'HASH'
           ? map { ($_, $data->{$_}) } sort keys %$data
           : @$data;

And then the final line becomes:

return join("&", @terms);

@neilb
Copy link
Author

neilb commented Jul 16, 2019

I'm happy to submit a PR for this, if you're ok with this?

@xdg xdg added the Wishlist label Jul 20, 2021
@xdg
Copy link
Collaborator

xdg commented Jul 20, 2021

Sorry to take so long to get to this. A PR with tests would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants