dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.44k stars 10.02k forks source link

Encoding plus sign makes no difference #2683

Closed aspnet-hello closed 6 years ago

aspnet-hello commented 6 years ago

From @PawelTroka on Sunday, November 12, 2017 5:11:23 AM

Basically this issue is kind of similar to #912, except it's about plus sign ('+') and this time it does not only affects TestHost but shows up also when hosted.

It does not work correctly on TestHost or when hosted and invoked through web browser or any web client for that matter.

It is failing my integration tests where I am using Microsoft.AspNetCore.TestHost v2.0.0 and Microsoft.AspNetCore.WebUtilities v2.0.0. Please notice that my WebApi and WebApi.IntegrationTests are AspNetCore 2.0 but are targeting net461 (due to some of my libraries not yet ported).

Repro: Unit test: https://github.com/PawelTroka/Computator.NET/blob/97b666341bf8a9b21fb9dc59f94528ccf830a931/Computator.NET.WebApi.IntegrationTests/CalculateApiTests.cs#L42 Goes through route: real/{equation}/{x} from method: https://github.com/PawelTroka/Computator.NET/blob/97b666341bf8a9b21fb9dc59f94528ccf830a931/Computator.NET.WebApi/Controllers/CalculateController.cs#L34 But because encoding plus sign makes no difference we have equation with unencoded plus sign

In images:

When run through browser: We type encoded url image What reaches our controller however is already decoded image So when we are decoding it in another overload of Get(..) we will attempt to decode already decoded expression, which will make us replace plus sign ('+') with space (' ').

The same happens in test: image image

Also note that this exists at least from AspNetCore v1.1

Copied from original issue: aspnet/HttpAbstractions#964

aspnet-hello commented 6 years ago

From @Tratcher on Sunday, November 12, 2017 8:22:23 PM

This is intentional, everything in the path is unescaped so you can operate on it. Forward slash is the only character that can't be unescaped without changing the meaning of the path. Why are you converting plus to space? That's not an expected form of un-escaping for request paths. That only applies to form urlencoded content.

aspnet-hello commented 6 years ago

From @PawelTroka on Monday, November 13, 2017 12:00:49 PM

It being intentional indeed solves plus sign issues, however then why I'm getting spaces encoded as pluses when not decoding it myself?

test: https://github.com/PawelTroka/Computator.NET/blob/6a5e9976a32cca3084f35d09ebfbcbb0a7a16e58/Computator.NET.WebApi.IntegrationTests/CalculateApiTests.cs#L116 image

debug: https://github.com/PawelTroka/Computator.NET/blob/6a5e9976a32cca3084f35d09ebfbcbb0a7a16e58/Computator.NET.WebApi/Controllers/CalculateController.cs#L32 image

aspnet-hello commented 6 years ago

From @Tratcher on Monday, November 13, 2017 12:03:41 PM

Try Uri.EscapeDataString instead.

aspnet-hello commented 6 years ago

From @PawelTroka on Monday, November 13, 2017 12:13:56 PM

Yes! It works now! Why is it so different than WebUtility.UrlEncode and how can I be sure that clients (written in totally different languages like JavaScript) will correctly escape it?

aspnet-hello commented 6 years ago

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.