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

Enumerable serialization in query strings failing #432

Closed
eduardocampano opened this issue Feb 2, 2018 · 6 comments · Fixed by #433
Closed

Enumerable serialization in query strings failing #432

eduardocampano opened this issue Feb 2, 2018 · 6 comments · Fixed by #433
Milestone

Comments

@eduardocampano
Copy link

eduardocampano commented Feb 2, 2018

We run into an issue when updating from 4.0.1 to 4.2.0. We relied on a custom IUrlParameterFormatter to serialize IEnumerable into comma separated values. After updating to 4.2.0 we started having issues with query string parameters and it looks like IEnumerable is being serialized with BuildQueryMap probably because of the DoNotConvertToQueryMap being incorrect in this scenario.

Here is an example of our interface:

private interface ISomeApi
{
    [Get("/companies")]
    Task GetCompanies(IEnumerable<Guid> ids);
}

When calling apiClient.GetCompanies(new[] {Guid.NewGuid(), Guid.NewGuid()}) we get a query string like

?Length=2&LongLength=2&Rank=1&SyncRoot=de802b69-a204-4df4-b5e0-7d658777d056&SyncRoot=547f8167-1e48-4204-8081-c132c13c558a&IsReadOnly=False&IsFixedSize=True&IsSynchronized=False
@clairernovotny
Copy link
Member

@eduardocampano I have a fix for this, but it'll only do part of what you want -- it'll do the commas, but the commas will be urlencoded, so the result looks like this:

numbers=1%2C2%2C3 where the input was an enumerable array of 1, 2, 3. The server should be decoding the urls, would this work for you?

@eduardocampano
Copy link
Author

Wow, that was fast, I guess it's the way it's working in 4.0.1, will check

@clairernovotny
Copy link
Member

@eduardocampano there were a couple of UrlEncoding fixes in 4.2 as well, so it may not be 100% the same.

@clairernovotny
Copy link
Member

@eduardocampano if you can confirm this works for you, I can merge it in. Thanks.

@eduardocampano
Copy link
Author

eduardocampano commented Feb 3, 2018

@onovotny confirmed it works, thanks for the quick fix

@clairernovotny
Copy link
Member

Will try to get a 4.2.1 to nuget soon.

@clairernovotny clairernovotny added this to the 4.2.1 milestone Feb 4, 2018
@clairernovotny clairernovotny modified the milestones: 4.2.1, v4.3 Feb 16, 2018
@lock lock bot added the outdated label Jun 25, 2019
@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants