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

QueryString key is not unescaped when value omitted #33394

Closed
bart-degreed opened this issue Jun 9, 2021 · 1 comment · Fixed by #33401
Closed

QueryString key is not unescaped when value omitted #33394

bart-degreed opened this issue Jun 9, 2021 · 1 comment · Fixed by #33401
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Milestone

Comments

@bart-degreed
Copy link

An incoming request with a query string that contains an escaped key without a value is not properly unescaped.

Example:

GET http://localhost/api/demo?fields%5BtodoItems%5D HTTP/1.1
string keys = string.Join(' ', new HttpContextAccessor().HttpContext.Request.Query.Keys);
// keys: %5BtodoItems%5D

In contrast, when the query string does contain a value, it gets unescaped properly.

GET http://localhost/api/demo?fields%5BtodoItems%5D=1 HTTP/1.1
string keys = string.Join(' ', new HttpContextAccessor().HttpContext.Request.Query.Keys);
// keys: [todoItems]

This bug applies to ASP.NET Core version: 3.1, 5.0 and the master branch.

The problem is caused by the next line:

accumulator.Append(queryString.Substring(scanIndex, delimiterIndex - scanIndex), string.Empty);

which does not unescape. To fix, replace this line with:

string name = queryString.Substring(scanIndex, delimiterIndex - scanIndex);
accumulator.Append(Uri.UnescapeDataString(name.Replace('+', ' ')), string.Empty);

When this gets fixed, it would be great to also backport it to .NET Core 3.1 and 5.0.

@bart-degreed
Copy link
Author

I should note this problem does not repro when using WebApplicationFactory from an integration test, presumable because things are short-circuited in that case.

@Tratcher Tratcher added bug This issue describes a behavior which is not expected - a bug. good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue labels Jun 9, 2021
@BrennanConroy BrennanConroy added this to the 6.0-preview6 milestone Jun 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 18, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants