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

[Bug]: Query parameters in path are reversed #1732

Closed
TimothyMakkison opened this issue Jun 23, 2024 · 1 comment · Fixed by #1737
Closed

[Bug]: Query parameters in path are reversed #1732

TimothyMakkison opened this issue Jun 23, 2024 · 1 comment · Fixed by #1737
Labels

Comments

@TimothyMakkison
Copy link
Contributor

TimothyMakkison commented Jun 23, 2024

Describe the bug 🐞

Query parameters defined in the path are reversed in the resulting request.

Step to reproduce

See RequestBuilderTests.ParametersShouldBePutAsExplicitQueryString.

 [Get("/query?q1={param1}&q2={param2}")]
 Task QueryWithExplicitParameters(string param1, string param2);

Becomes /query?q2=value2&q1=value1

Reproduction repository

https://github.com/reactiveui/refit

Expected behavior

Expected /query?q1=value1&q2=value2

Cause

The code that causes this is found in RequestBuilderImplementation, path queries are readded to queryParamsToAdd to the start using insert. We can prevent this by either iterating query.AllKeys in reverse or by keeping count of how many items have been prepended and using this number as an insert index.

var query = HttpUtility.ParseQueryString(uri.Query ?? "");
foreach (var key in query.AllKeys)
{
    if (!string.IsNullOrWhiteSpace(key))
    {
        queryParamsToAdd.Insert(
            0,
            new KeyValuePair<string, string?>(key, query[key])
        );
    }
                }
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant